From d86524844f25c1865a8dbbee70e215f2c9f8b7e5 Mon Sep 17 00:00:00 2001 From: neutral-reconciler Date: Thu, 30 Jul 2026 07:33:00 +0000 Subject: [PATCH 1/3] Usage-policy gate folds path spellings a volume treats as one directory `realpath(2)` folds symlinks and nothing else. On a default macOS (APFS) volume the kernel accepts, stats, and chdirs to spellings the gate compared as different strings: `Cafe` composed (NFC) versus decomposed (NFD), and `Proj` versus `proj`. The machine-local list's membership test therefore returned `full` for a directory the user opted out of, because the `dir` a CLI declared and the `cwd` a client reported are produced by different processes at different times. List membership now compares a folded spelling of both sides: Unicode NFC unconditionally (total, no filesystem access, identity on an already-composed path), and case only behind a per-volume probe that is inert off darwin. Producing a folded spelling is necessary but not sufficient. Nearest-governs is an argmax over match depth, and an argmax discards verdicts rather than merging them, so a carve-out that gains reach only by folding could displace a broader restrictive entry and start a private directory forwarding. The rule is therefore evaluated twice, once over the spellings exactly as declared and once folded, and the more restrictive answer wins. The resolved class is max(pre_fold, folded) by construction. The per-cwd memo stays keyed on the lexical path and consulted before any folding, so the per-exchange hot path is unchanged. Fixes #483 Co-Authored-By: Claude --- llp/0049-hypignore-usage-policy.spec.md | 8 + ...50-ignore-enforced-in-adapters.decision.md | 103 +++++ src/core/usage-policy/fold.js | 227 ++++++++++ src/core/usage-policy/index.js | 5 + src/core/usage-policy/matcher.js | 187 +++++++- src/core/usage-policy/types.d.ts | 13 + test/core/usage-policy-fold.test.js | 404 ++++++++++++++++++ 7 files changed, 929 insertions(+), 18 deletions(-) create mode 100644 src/core/usage-policy/fold.js create mode 100644 test/core/usage-policy-fold.test.js diff --git a/llp/0049-hypignore-usage-policy.spec.md b/llp/0049-hypignore-usage-policy.spec.md index 7e28dc26..59ce2fef 100644 --- a/llp/0049-hypignore-usage-policy.spec.md +++ b/llp/0049-hypignore-usage-policy.spec.md @@ -62,6 +62,14 @@ governs. Because V1 has only `ignore` and no "un-ignore" directive mechanism is not repo-bound: a `.hypignore` anywhere in the ancestor chain (including outside any git repo) governs its subtree. +**Extended-by: +[LLP 0050 §normalization](./0050-ignore-enforced-in-adapters.decision.md#normalization)** +the ancestor walk itself is unaffected (it `stat`s each candidate rather than +comparing two strings), but the machine-local list's membership test compares a +`cwd` a client reported against a `dir` a CLI declared. A filesystem can spell +one directory several ways, so that comparison is over a **folded** spelling of +both sides, and the fold can only ever make the verdict more restrictive. + ## Classes {#classes} | Class | V1 | Meaning | diff --git a/llp/0050-ignore-enforced-in-adapters.decision.md b/llp/0050-ignore-enforced-in-adapters.decision.md index 617406f2..e1b129c1 100644 --- a/llp/0050-ignore-enforced-in-adapters.decision.md +++ b/llp/0050-ignore-enforced-in-adapters.decision.md @@ -97,6 +97,109 @@ Two copies of a privacy-critical matcher drift apart. A single core module with one test suite is the safer home; sibling-to-sibling plugin imports would be worse coupling than both importing core. +## The set of spellings that denote a directory is volume-dependent {#normalization} + +The shared matcher compares **strings**. A filesystem hands one directory +several strings, and which mechanisms apply is a property of the **volume**, +not of the path and not of the platform: + +| mechanism | folded by | volume-dependent? | +|---|---|---| +| symlinked components | `realpath(2)` | no | +| Unicode normalization (NFC vs NFD) | nothing in `node:fs` | yes, but folding to NFC is safe everywhere | +| case | nothing in `node:fs` | yes, and folding is **unsafe** where it does not apply | + +`realpath(2)` resolves symlinks and does nothing else. On a default macOS +(APFS) volume the kernel accepts, `stat`s, and `chdir`s to spellings it will not +fold: `Proj` and `proj` are one directory, and `Café` spelled NFC (U+00E9) and +NFD (`e` + U+0301) are one directory. So the gate could be handed a `cwd` whose +spelling differs from the spelling a machine-local entry was declared with and +return `full` for a directory the user opted out of. That is not an exotic +case: macOS frameworks and Finder-derived paths emit NFD while typed and +JSON-transported paths are usually NFC, and the two paths this gate compares are +produced by **different processes at different times** (a CLI resolving a mark, +versus a client reporting a `cwd`). + +**Decision: list membership compares a folded spelling of both sides.** The fold +is `src/core/usage-policy/fold.js`: + +1. **NFC unconditionally.** It is a total function of the string, needs no + filesystem access, cannot fail, and is the identity on a path that is already + composed. There is no volume on which folding NFC and NFD together is wrong, + because no filesystem this codebase targets lets two paths that differ only + by normalization name two different directories. +2. **Case only behind a per-volume probe.** Case-sensitivity is a property of + the mounted volume: an APFS volume can be formatted case-sensitive and every + ext4 volume is. Folding it unconditionally would merge two genuinely + different directories, which is a correctness bug in the other direction. The + probe compares the `dev`/`ino` of a path against a case-flipped spelling of + its last segment, memoizes by `dev`, and is inert (constant `false`, no + syscall) off darwin. An undetermined probe resolves to "case-sensitive", + which is the pre-fold behaviour, so a failed probe can only fail to *add* + reach. + +The fold must **distribute over the path separator**, since its only consumer is +a segment-aware prefix test: `fold(a + '/' + b) === fold(a) + '/' + fold(b)`. +Both halves do (`/` is a starter that participates in no canonical composition, +and `toLowerCase` maps it to itself), and the property is asserted rather than +assumed. + +### A widened spelling must only ever add restriction + +Producing a folded spelling is necessary but **not sufficient**. The +machine-local list's nearest-governs step is an **argmax over match depth**, and +an argmax discards verdicts instead of merging them. A less restrictive entry +that gains reach through its folded spelling can become the deepest match and +displace a broader restrictive entry that already governed: a `--sync` carve-out +spelled NFC would punch a hole in a private tree spelled NFD, and the directory +would **start recording and forwarding**. Nothing about "compare folded +spellings" prevents that on its own. + +So nearest-governs is evaluated **twice**, once over the spellings exactly as +declared (which reproduces the pre-fold verdict) and once folded, and the **more +restrictive of the two answers wins**, the declared one breaking a class tie +because it is the spelling the user typed. The resolved class is therefore +`max(pre_fold, folded)` on the restrictiveness lattice by construction, which +makes "folding never opens the gate" structural rather than a property someone +has to remember +([LLP 0049 §fail-safe](./0049-hypignore-usage-policy.spec.md#fail-safe)). + +The visible cost is that a **nested loosening does not cross spellings**: a +carve-out has to be declared in the same spelling as the entry it carves out of. +`hyp policy show` reports the class actually in force, so it is diagnosable. + +Specificity is measured on the **folded** spelling, not the declared one. NFD is +longer in code units than NFC for the same name, so a declared-string depth can +rank a decomposed ancestor above a composed descendant and invert +nearest-governs. + +### Cost + +The per-`cwd` memo is keyed on the **lexical** path and consulted **before** any +folding, so a cache hit costs exactly what it did before. Entry spellings and +the per-volume case verdict are computed once per list parse, inside the TTL +window LLP 0049 R6 already bounds. `String.prototype.normalize('NFC')` is +roughly 60 ns on a pure-ASCII path. + +### Relationship to the symlink class + +This is the same shape PR #482 (LLP 0049 issue #479) arrives at for symlink +canonicalization, for the same reason, and the two were found by the same +review. They are independent: `realpath` cannot fold case or normalization, and +folding cannot resolve a symlink. Whichever lands second should collapse the two +two-pass evaluations into **one** pass over one spelling set rather than keep +two, since running the argmax guard twice buys nothing. + +### Not covered + +The fold is applied at the **gate** (`resolve` / list membership). The one-shot +CLI membership sites (`hyp ignore --check`, `policy show`, `policy unset`) and +`hyp purge --subtree` still compare lexically, so on a case-insensitive or +NFD-carrying volume the CLI can still name a different governor than the gate +used. Those sites are exactly what #482 reroutes through a single shared +spelling-aware predicate; they should adopt the fold there, once, rather than +grow a second copy of the rule. + ## Consequences - Code that lands this carries `@ref LLP 0050 [implements]` on the adapter diff --git a/src/core/usage-policy/fold.js b/src/core/usage-policy/fold.js new file mode 100644 index 00000000..e7a7044a --- /dev/null +++ b/src/core/usage-policy/fold.js @@ -0,0 +1,227 @@ +// @ts-check + +import { createHash } from 'node:crypto' +import nodeFs from 'node:fs' +import path from 'node:path' + +import { Attr } from '../observability/attrs.js' +import { getLogger } from '../observability/logger.js' + +/** + * `error_kind` for a per-volume case-sensitivity probe that could not reach a + * definite answer. Never fatal: an undetermined volume is treated as + * case-sensitive, which is exactly the pre-fold behaviour, so a failed probe + * can only lose reach the fold would have added. + */ +export const PATH_CASE_PROBE_ERROR_KIND = 'path_case_probe_failed' + +/** + * Short one-way digest of a path, so a fold decision or a skipped probe is + * diagnosable (which path, how often, which errno) without dev telemetry ever + * carrying a raw local path. Same discipline as the `usage_policy.export_drop` + * aggregate in `src/core/cache/storage.js`. + * + * @param {string} p + * @returns {string} + */ +export function hashPath(p) { + return createHash('sha256').update(p).digest('hex').slice(0, 16) +} + +/** + * The `errno` code of a filesystem error, as a lowercase token suitable for a + * log attribute (`enoent`, `eacces`, `eperm`), or `unknown`. + * + * @param {unknown} err + * @returns {string} + */ +function errnoOf(err) { + const code = /** @type {{ code?: unknown }} */ (err)?.code + return typeof code === 'string' && code !== '' ? code.toLowerCase() : 'unknown' +} + +/** + * Fold a path into the form two spellings of the *same* directory share. + * + * `realpath(2)` folds symlinks and nothing else. A filesystem can give one + * directory several spellings by two further mechanisms, and neither is + * reachable through `realpath`: + * + * 1. **Unicode normalization.** macOS frameworks and Finder-derived paths emit + * NFD (`e` + U+0301) while typed and JSON-transported paths are usually NFC + * (U+00E9). On a default APFS volume both `stat` and `chdir` to the same + * directory. NFC is applied **unconditionally** here: it is a total function + * that needs no filesystem access and cannot fail, and on a path that is + * already NFC (every path on a Linux box that was never typed on a Mac) it + * is the identity, so folding costs a comparison and changes nothing. + * 2. **Case.** On a case-insensitive volume `Proj` and `proj` are one + * directory. This is a property of the **volume**, not of the platform: an + * APFS volume can be formatted case-sensitive, and every ext4 volume is. So + * case is folded only when the caller passes a verdict for the volume the + * path lives on. Folding it unconditionally would merge two genuinely + * different directories on a case-sensitive volume, which is a correctness + * bug in the other direction. + * + * **Separator-preserving, and that is load-bearing.** The only consumer is a + * path-segment prefix test, so the fold has to distribute over `/`: + * `fold(a + '/' + b) === fold(a) + '/' + fold(b)`. It does. `/` is a starter + * with combining class 0 that participates in no canonical composition, so NFC + * is computed independently on each side of it, and `toLowerCase` maps `/` to + * itself. A fold that did not distribute could turn a non-descendant into a + * descendant across a segment boundary, so the property is asserted in the + * test suite rather than left to inspection. + * + * @ref LLP 0050#normalization [implements]: the fold that makes two spellings of one directory compare equal + * @param {string} p absolute path (already `path.resolve`d) + * @param {{ caseInsensitive?: boolean }} [opts] + * @returns {string} + */ +export function foldPath(p, { caseInsensitive = false } = {}) { + const nfc = p.normalize('NFC') + return caseInsensitive ? nfc.toLowerCase() : nfc +} + +/** + * A spelling of `p` whose **last segment** has the case of every cased + * character flipped, or `null` when that segment has no cased character (so no + * probe is possible from this path). + * + * Only the last segment is flipped, because the parent directories have to stay + * traversable under their exact spelling for the probe to be a statement about + * the volume `p` sits on rather than about every volume between it and the + * root. Flipping *every* cased character of that segment rather than one is + * deliberate: it is the spelling least likely to collide with a genuinely + * different sibling on a case-sensitive volume, and even a collision is caught, + * because what decides the verdict is the `dev`/`ino` identity of the two + * spellings, not whether the flipped name resolves. + * + * @param {string} p + * @returns {string | null} + */ +function flipCase(p) { + const cut = p.lastIndexOf(path.sep) + const head = cut < 0 ? '' : p.slice(0, cut + 1) + const tail = cut < 0 ? p : p.slice(cut + 1) + let flipped = '' + let anyFlipped = false + for (const ch of tail) { + const lower = ch.toLowerCase() + const upper = ch.toUpperCase() + if (lower !== upper) { + flipped += ch === lower ? upper : lower + anyFlipped = true + } else { + flipped += ch + } + } + return anyFlipped ? head + flipped : null +} + +/** + * Create a memoized per-volume case-sensitivity probe. + * + * The verdict is a property of the mounted volume, so it is memoized by the + * volume's `dev` number rather than by path: one pair of `stat` calls per + * distinct volume for the life of the resolver, not one per directory and not + * one per TTL window. That is strictly under the per-`cwd`-per-window bound + * LLP 0049 R6 already sets for the ancestor walk. A volume cannot change its + * case-sensitivity without being unmounted and reformatted, at which point its + * `dev` changes too, so there is nothing for a TTL to refresh. + * + * **Inert off darwin.** On any other platform the probe returns `false` + * immediately and issues **no syscall at all**, because no shipping + * Linux/Windows filesystem this codebase targets presents the macOS + * case-insensitive-by-default behaviour that motivates the fold. That also + * means the whole probe is dead code on a Linux host, and therefore that its + * darwin behaviour cannot be executed, let alone verified, there. + * + * **Undetermined resolves to `false`**, which is the pre-fold behaviour: a + * directory that does not exist, a `stat` that is refused, or a path with no + * cased character all fold NFC only. A failed probe can therefore only fail to + * *add* reach; it can never remove a verdict some spelling already produced. + * + * @ref LLP 0050#normalization [implements]: case folding is per-volume and probed, never a platform constant + * @ref LLP 0049#fail-safe [constrained-by]: an undetermined probe resolves to the pre-fold behaviour, never to a looser gate + * @param {object} [deps] + * @param {string} [deps.platform] defaults to `process.platform` + * @param {(p: string) => { dev: number, ino: number }} [deps.statSync] + * @param {(name: string, fields?: Record) => void} [deps.logSkip] + * @returns {(dir: string) => boolean} + */ +export function createVolumeCaseProbe({ + platform = process.platform, + statSync = nodeFs.statSync, + logSkip, +} = {}) { + if (platform !== 'darwin') return () => false + + /** @type {Map} */ + const byDev = new Map() + + /** + * @param {string} dir + * @param {string} reason + * @param {string} errno + */ + function skip(dir, reason, errno) { + const emit = logSkip ?? defaultLogSkip + emit('usage_policy.case_probe_skipped', { + [Attr.COMPONENT]: 'usage-policy', + [Attr.OPERATION]: 'case_probe', + [Attr.STATUS]: 'skipped', + [Attr.ERROR_KIND]: PATH_CASE_PROBE_ERROR_KIND, + reason, + errno, + path_hash: hashPath(dir), + }) + } + + return function probe(dir) { + /** @type {{ dev: number, ino: number }} */ + let st + try { + st = statSync(dir) + } catch (err) { + skip(dir, 'stat_failed', errnoOf(err)) + return false + } + const memoized = byDev.get(st.dev) + if (memoized !== undefined) return memoized + + const flipped = flipCase(dir) + if (flipped === null) { + // Not memoized: another path on this same volume may well have a cased + // character, and would then reach a definite answer. + skip(dir, 'no_cased_character', 'none') + return false + } + let verdict + try { + const other = statSync(flipped) + verdict = other.dev === st.dev && other.ino === st.ino + } catch (err) { + // `ENOENT` here is the *informative* outcome: the flipped spelling does + // not resolve, so the volume is case-sensitive. Any other errno is a + // genuinely undetermined probe and is not memoized. + const errno = errnoOf(err) + if (errno !== 'enoent') { + skip(dir, 'stat_failed', errno) + return false + } + verdict = false + } + byDev.set(st.dev, verdict) + return verdict + } +} + +/** + * @param {string} name + * @param {Record} [fields] + * @returns {void} + */ +function defaultLogSkip(name, fields) { + // A directory that does not exist is routine at this seam (a deleted `cwd`, + // a mark for a not-yet-created directory), so this is never a warning. + getLogger('usage-policy').debug(name, fields) +} diff --git a/src/core/usage-policy/index.js b/src/core/usage-policy/index.js index 3f562752..438c0c5d 100644 --- a/src/core/usage-policy/index.js +++ b/src/core/usage-policy/index.js @@ -5,6 +5,11 @@ // import it exactly as they import `src/core/observability`. export { parseHypignore } from './format.js' export { CLASS_RANK, createUsagePolicyResolver, isEqualOrDescendant } from './matcher.js' +// The spelling fold the gate compares through (LLP 0050 #normalization): +// Unicode-NFC always, case only on a volume probed case-insensitive. Exported +// so a caller that has to agree with the gate's verdict folds by the same rule +// instead of growing a second one. +export { createVolumeCaseProbe, foldPath, PATH_CASE_PROBE_ERROR_KIND } from './fold.js' // The terminal capture-seam drop sentinel (LLP 0050): an adapter projector // returns it for an `.hypignore`-ignored exchange, and the gateway dispatcher // stops on it (never falls through to a later projector) and logs it as a drop. diff --git a/src/core/usage-policy/matcher.js b/src/core/usage-policy/matcher.js index 12e1c375..4dd3c093 100644 --- a/src/core/usage-policy/matcher.js +++ b/src/core/usage-policy/matcher.js @@ -3,11 +3,15 @@ import nodeFs from 'node:fs' import path from 'node:path' +import { Attr } from '../observability/attrs.js' +import { getLogger } from '../observability/logger.js' + +import { createVolumeCaseProbe, foldPath, hashPath } from './fold.js' import { parseHypignore } from './format.js' import { LocalOnlyListUnreadableError } from './local_only.js' /** - * @import { LocalOnlyEntry, ResolveResult, UsagePolicyResolver } from '../../../src/core/usage-policy/types.js' + * @import { ListScope, LocalOnlyEntry, ResolveResult, UsageClass, UsagePolicyResolver } from '../../../src/core/usage-policy/types.js' */ const HYPIGNORE_FILENAME = '.hypignore' @@ -79,6 +83,7 @@ const LOCAL_ONLY_LIST_VERSION_V2 = 2 * @ref LLP 0052#matcher [implements]: bounded-TTL staleness so a mid-run .hypignore is honored without a daemon restart * @ref LLP 0070#resolver [implements]: one shared resolver, two sources, most-restrictive class wins * @ref LLP 0071 [implements]: the machine-local list is the second source + * @ref LLP 0050#normalization [implements]: list membership compares folded spellings, so an NFC/NFD divergence between two processes does not open the gate * @param {object} [deps] * @param {(path: string, encoding: 'utf8') => string} [deps.readFileSync] * @param {(path: string) => boolean} [deps.existsSync] @@ -87,6 +92,12 @@ const LOCAL_ONLY_LIST_VERSION_V2 = 2 * @param {string} [deps.localOnlyListPath] absolute path of the machine-local * `local-only` list (`localOnlyListPath(stateDir)`, LLP 0071); omitted => * the resolver behaves exactly as it did before the list existed + * @param {(dir: string) => boolean} [deps.caseInsensitiveVolume] per-volume + * case-sensitivity verdict for an entry's directory; defaults to + * {@link createVolumeCaseProbe}, which is inert (constant `false`, no + * syscall) off darwin. Injected so the folding logic can be exercised for a + * case-insensitive volume on a host that has none + * @param {(name: string, fields?: Record) => void} [deps.logEvent] * @returns {UsagePolicyResolver} */ export function createUsagePolicyResolver({ @@ -95,11 +106,15 @@ export function createUsagePolicyResolver({ now = Date.now, ttlMs = CACHE_TTL_MS, localOnlyListPath, + caseInsensitiveVolume, + logEvent, } = {}) { /** @type {Map} */ const cache = new Map() - /** @type {{ entries: LocalOnlyEntry[], expiresAt: number } | null} */ + /** @type {{ scopes: ListScope[], expiresAt: number } | null} */ let listCache = null + const emit = logEvent ?? emitDebug + const probeVolume = caseInsensitiveVolume ?? createVolumeCaseProbe({ logSkip: emit }) /** * @param {string} cwd @@ -109,6 +124,9 @@ export function createUsagePolicyResolver({ const key = path.resolve(cwd) const at = now() const cached = cache.get(key) + // The memo is keyed on the *lexical* path and consulted before any folding, + // so a cache hit costs exactly what it did before this existed. Folding is + // on the miss path only. if (cached && cached.expiresAt > at) return cached.result const dotfileResult = walk(key) const listResult = localOnlyListPath ? matchList(key, at) : null @@ -172,37 +190,69 @@ export function createUsagePolicyResolver({ * mirroring the `.hypignore` walk's nearest-governs rule; a tie is broken * by the more restrictive class. * + * An entry governs `cwd` when its declared `dir` equals-or-contains `cwd`, + * **or** when the two do once both are folded (Unicode-normalized, and + * case-folded on a volume probed case-insensitive). Widening an entry's + * reach that way must not *loosen* the list, which is what + * {@link selectGoverning} guarantees. + * * @ref LLP 0071 [implements]: segment-aware equal-or-descendant list membership, second resolver source + * @ref LLP 0050#normalization [implements]: an entry governs through any spelling the volume folds together * @ref LLP 0103 [implements]: the entry's own class governs, not a hardcoded `local-only` * @param {string} cwd absolute, already `path.resolve`d * @param {number} at current clock reading (ms) * @returns {ResolveResult | null} `null` when nothing in the list governs `cwd` */ function matchList(cwd, at) { - const entries = getListEntries(at) - const matches = entries.filter((entry) => isEqualOrDescendant(cwd, entry.dir)) - if (matches.length === 0) return null - const governing = matches.reduce((best, entry) => { - if (entry.dir.length > best.dir.length) return entry - if (entry.dir.length === best.dir.length && CLASS_RANK[entry.class] > CLASS_RANK[best.class]) return entry - return best - }) + const governing = selectGoverning(cwd, getListScopes(at), reportFold) + if (governing === null) return null return { - class: governing.class, + class: governing.entry.class, governedBy: /** @type {string} */ (localOnlyListPath), - declared: governing.class, + declared: governing.entry.class, } } /** + * Structured signal for the one interesting outcome: the folded pass reached + * a **more restrictive** verdict than the declared spellings did, i.e. a + * spelling divergence that would otherwise have opened the gate. Paths are + * hashed, never logged raw, the same discipline as the + * `usage_policy.export_drop` aggregate. + * + * @param {string} cwd + * @param {UsageClass | null} declaredClass + * @param {UsageClass} foldedClass + * @returns {void} + */ + function reportFold(cwd, declaredClass, foldedClass) { + emit('usage_policy.fold_tightened', { + [Attr.COMPONENT]: 'usage-policy', + [Attr.OPERATION]: 'match_list', + [Attr.STATUS]: 'ok', + declared_class: declaredClass ?? 'none', + folded_class: foldedClass, + cwd_hash: hashPath(cwd), + }) + } + + /** + * The list entries paired with the folded spelling of each entry's declared + * directory and the case verdict for the volume it lives on, computed once + * per TTL window along with the parse. Resolving many `cwd`s in one window + * therefore costs one fold per entry, not one per entry per `cwd`. + * * @param {number} at - * @returns {LocalOnlyEntry[]} + * @returns {ListScope[]} */ - function getListEntries(at) { - if (listCache && listCache.expiresAt > at) return listCache.entries - const entries = readListEntriesSync() - listCache = { entries, expiresAt: at + ttlMs } - return entries + function getListScopes(at) { + if (listCache && listCache.expiresAt > at) return listCache.scopes + const scopes = readListEntriesSync().map((entry) => { + const caseInsensitive = probeVolume(entry.dir) + return { entry, caseInsensitive, foldedDir: foldPath(entry.dir, { caseInsensitive }) } + }) + listCache = { scopes, expiresAt: at + ttlMs } + return scopes } /** @@ -271,6 +321,19 @@ export function createUsagePolicyResolver({ return { resolve, isIgnored } } +/** + * Default sink for the resolver's structured signals. Both of them (a fold that + * tightened a verdict, a case probe that could not reach an answer) are routine + * rather than faults, so neither is ever louder than `debug`. + * + * @param {string} name + * @param {Record} [fields] + * @returns {void} + */ +function emitDebug(name, fields) { + getLogger('usage-policy').debug(name, fields) +} + /** * True when `cwd` equals `dir`, or is a path-segment descendant of it. * Segment-aware: `/a/bc` is not a descendant of `/a/b` even though it shares @@ -293,6 +356,94 @@ export function isEqualOrDescendant(cwd, dir) { return cwd.startsWith(prefix) } +/** + * The nearest-governs winner over `scopes`: the entry whose matched directory + * spelling is the longest, ties broken by the more restrictive class. When + * `folded` is false this compares the spellings exactly as declared, which is + * bit-for-bit the rule the matcher applied before folding existed; when it is + * true both sides are folded first, so an entry reaches every spelling its + * volume treats as the same directory. + * + * @param {string} cwd absolute, already `path.resolve`d + * @param {readonly ListScope[]} scopes + * @param {boolean} folded + * @returns {{ entry: LocalOnlyEntry, depth: number } | null} + */ +function deepestMatch(cwd, scopes, folded) { + const foldedCwd = folded ? foldPath(cwd) : cwd + /** @type {string | null} */ + let loweredCwd = null + /** @type {{ entry: LocalOnlyEntry, depth: number } | null} */ + let best = null + for (const scope of scopes) { + let target = cwd + let dir = scope.entry.dir + if (folded) { + dir = scope.foldedDir + // `foldPath(cwd, { caseInsensitive: true })` is `foldPath(cwd)` lowered, + // so the two variants are computed at most once each per call rather than + // once per entry. + target = scope.caseInsensitive ? (loweredCwd ??= foldedCwd.toLowerCase()) : foldedCwd + } + if (!isEqualOrDescendant(target, dir)) continue + const depth = dir.length + if ( + best === null || + depth > best.depth || + (depth === best.depth && CLASS_RANK[scope.entry.class] > CLASS_RANK[best.entry.class]) + ) { + best = { entry: scope.entry, depth } + } + } + return best +} + +/** + * The machine-local entry that governs `cwd`, over precomputed folded scopes. + * + * Nearest-governs alone is **not** monotone in how many spellings an entry can + * reach, and that is the one place a fold could make the gate *less* + * restrictive than the string matcher it replaces. A less restrictive entry + * that gains reach through its folded spelling can become the deepest match and + * displace a broader restrictive entry that already governed: an explicit + * `full`/`sync` carve-out spelled NFC would punch a hole in a private tree + * spelled NFD, and the directory would start recording and forwarding. Nothing + * about "compare folded spellings" prevents that on its own, because the + * argmax-over-depth step in the middle discards verdicts instead of merging + * them. + * + * So the rule is run twice, once over the spellings exactly as declared (which + * reproduces the pre-fold verdict) and once folded, and the **more restrictive + * of the two answers wins**, the declared one breaking a class tie because it + * is the spelling the user typed. The resolved class is therefore + * `max(pre_fold, folded)` on the restrictiveness lattice by construction: + * folding can only ever add restriction, never remove it. The visible cost is + * that a nested loosening does not cross spellings, which is the direction LLP + * 0049 §fail-safe picks. + * + * This is the same shape PR #482 arrived at for symlink canonicalization, for + * the same reason. Neither branch depends on the other; whichever lands second + * should collapse the two into one pass over one spelling set rather than keep + * two. + * + * @ref LLP 0050#normalization [implements]: a folded spelling only ever adds restriction, entry side included + * @ref LLP 0049#fail-safe [constrained-by]: a widened reach must resolve to "suppress more", never to "starts forwarding" + * @param {string} cwd absolute, already `path.resolve`d + * @param {readonly ListScope[]} scopes + * @param {(cwd: string, declaredClass: UsageClass | null, foldedClass: UsageClass) => void} [onTightened] + * @returns {{ entry: LocalOnlyEntry, depth: number } | null} + */ +function selectGoverning(cwd, scopes, onTightened) { + const asDeclared = deepestMatch(cwd, scopes, false) + const folded = deepestMatch(cwd, scopes, true) + const declaredRank = asDeclared === null ? CLASS_RANK.full : CLASS_RANK[asDeclared.entry.class] + if (folded !== null && CLASS_RANK[folded.entry.class] > declaredRank) { + if (onTightened) onTightened(cwd, asDeclared === null ? null : asDeclared.entry.class, folded.entry.class) + return folded + } + return asDeclared ?? folded +} + /** * Merge the `.hypignore` walk result with an optional list-membership result, * returning whichever is strictly more restrictive (`ignore` > `local-only` > diff --git a/src/core/usage-policy/types.d.ts b/src/core/usage-policy/types.d.ts index 724d54d2..1e7ffdbd 100644 --- a/src/core/usage-policy/types.d.ts +++ b/src/core/usage-policy/types.d.ts @@ -61,6 +61,19 @@ export interface LocalOnlyEntry { class: UsageClass } +// A machine-local list entry paired with the precomputed folded spelling of +// its declared `dir` and the case-sensitivity verdict for the volume that +// directory lives on (LLP 0050 §normalization). Computed once per list parse +// per TTL window, so resolving many `cwd`s in one window folds each entry once +// rather than once per `cwd`. `foldedDir` is `foldPath(entry.dir, { +// caseInsensitive })`; `caseInsensitive` is false on every non-darwin host and +// on any volume whose probe was undetermined. +export interface ListScope { + entry: LocalOnlyEntry + caseInsensitive: boolean + foldedDir: string +} + // Version-2 on-disk shape of the machine-local list (LLP 0103): the // class-per-entry store that replaces the version-1 bare `dirs` array. export interface LocalOnlyListFileV2 { diff --git a/test/core/usage-policy-fold.test.js b/test/core/usage-policy-fold.test.js new file mode 100644 index 00000000..2004c6d7 --- /dev/null +++ b/test/core/usage-policy-fold.test.js @@ -0,0 +1,404 @@ +// @ts-check + +// Regression tests for the path-spelling fold: `realpath(2)` folds symlinks +// and nothing else, so on a filesystem that treats two spellings of one +// directory as the same directory the shared gate compared two strings that +// differ and returned `full` for a directory the user opted out of (#483). +// +// **What these tests can and cannot prove on a Linux host.** The Unicode half +// is honest here: `String.prototype.normalize` is a pure function of the +// string, the two spellings are built as literals, and the assertion is that +// the *comparison logic* folds them together. That is the whole mechanism, and +// it is the half that actually bites, because the two paths being compared are +// produced by different processes at different times (a CLI resolving a mark +// versus a client reporting a `cwd`), so an NFC/NFD divergence between them is +// ordinary rather than user error. +// +// The case half is different. Whether a *volume* folds `Proj` and `proj` is a +// property of the filesystem, and ext4 does not, so the tests below drive the +// case-folding logic through an **injected** volume verdict. That covers the +// matcher's behaviour given a verdict; it does not and cannot cover the probe +// that produces the verdict on macOS. The probe is asserted only to be inert +// off darwin, which is the one thing this host can witness. +// +// @ref LLP 0050#normalization [tests]: folding is only ever additive restriction, and NFC divergence no longer opens the gate + +import test from 'node:test' +import assert from 'node:assert/strict' + +import { createUsagePolicyResolver } from '../../src/core/usage-policy/matcher.js' +import { createVolumeCaseProbe, foldPath, PATH_CASE_PROBE_ERROR_KIND } from '../../src/core/usage-policy/fold.js' + +/** + * @import { UsageClass, UsagePolicyResolver } from '../../src/core/usage-policy/types.js' + */ + +// The same four characters, composed and decomposed. Spelled as escapes so the +// file cannot be silently re-normalized by an editor, a merge tool, or a +// `git` filter, which would turn both constants into the same string and make +// every test below pass vacuously. +const NFC = 'café' +const NFD = 'café' + +// The premise the whole file rests on. If this ever fails, nothing after it +// means anything. +test('the two fixture spellings are genuinely different strings that NFC folds together', () => { + assert.notEqual(NFC, NFD) + assert.equal(NFD.normalize('NFC'), NFC) + assert.equal(NFC.normalize('NFC'), NFC) +}) + +const LIST = '/state/usage-policy/local-only.json' + +/** + * A resolver over an in-memory machine-local list. No `.hypignore` exists, so + * every verdict below comes from list membership, which is the comparison + * under test. + * + * @param {readonly { dir: string, class: UsageClass }[]} entries + * @param {{ caseInsensitiveVolume?: (dir: string) => boolean, logEvent?: (name: string, fields?: Record) => void }} [deps] + * @returns {UsagePolicyResolver} + */ +function resolverOver(entries, deps = {}) { + const files = { [LIST]: JSON.stringify({ version: 2, entries }) } + return createUsagePolicyResolver({ + existsSync: (p) => Object.prototype.hasOwnProperty.call(files, p), + readFileSync: (p) => /** @type {Record} */ (files)[p], + localOnlyListPath: LIST, + ...deps, + }) +} + +// --- the leak: NFC/NFD divergence between two processes ------------------- + +test('resolve: a local-only entry declared NFC still governs a cwd that arrives NFD', () => { + const r = resolverOver([{ dir: `/root/${NFC}/proj`, class: 'local-only' }]) + assert.equal(r.resolve(`/root/${NFD}/proj`).class, 'local-only') +}) + +test('resolve: an ignore entry declared NFD still governs a cwd that arrives NFC', () => { + const r = resolverOver([{ dir: `/root/${NFD}`, class: 'ignore' }]) + assert.equal(r.resolve(`/root/${NFC}/proj/sub`).class, 'ignore') + assert.equal(r.isIgnored(`/root/${NFC}/proj/sub`), true) +}) + +test('resolve: the fold is on the ancestor segment, not only the leaf', () => { + // The divergent segment is an ancestor of the `cwd`, so the prefix test has + // to survive folding across a `/` boundary. + const r = resolverOver([{ dir: `/root/${NFD}/a/b`, class: 'local-only' }]) + assert.equal(r.resolve(`/root/${NFC}/a/b/c/d`).class, 'local-only') +}) + +// --- the fold must not merge across a path-segment boundary --------------- + +test('foldPath distributes over the path separator, so a prefix test stays segment-aware', () => { + for (const p of [`/root/${NFD}/a`, `/root/${NFC}/a`, '/plain/ascii/path', '/']) { + const segments = p.split('/') + assert.equal(foldPath(p), segments.map((s) => foldPath(s)).join('/')) + assert.equal(foldPath(p, { caseInsensitive: true }), segments.map((s) => foldPath(s, { caseInsensitive: true })).join('/')) + } +}) + +test('resolve: a sibling whose name merely shares a folded prefix is still NOT matched', () => { + const r = resolverOver([{ dir: `/root/${NFC}`, class: 'ignore' }]) + assert.equal(r.resolve(`/root/${NFD}-other`).class, 'full') + assert.equal(r.resolve(`/root/${NFC}xyz`).class, 'full') +}) + +// --- the property that keeps this from becoming a forwarding leak --------- + +test('resolve: a carve-out that gains reach only by folding does not punch a hole in a broader restrictive entry', () => { + // The analogue of the regression PR #482's round-1 review found: an argmax + // over match depth discards verdicts, so a less restrictive entry that only + // matches once folded can become the deepest match and displace a broader + // restrictive entry that already governed. Without the two-pass rule this + // resolves to `full` and the directory starts recording and forwarding. + const r = resolverOver([ + { dir: '/root/real', class: 'ignore' }, + { dir: `/root/real/${NFC}`, class: 'full' }, + ]) + assert.equal(r.resolve(`/root/real/${NFD}/sub`).class, 'ignore') + assert.equal(r.resolve(`/root/real/${NFD}`).class, 'ignore') +}) + +test('resolve: an entry that gains reach by folding overrides a shallower explicit full marker', () => { + // The tightening direction, against an entry that already matched: LLP 0103's + // explicit `full` marker governs `/root`, and the restrictive entry only + // reaches `cwd` once folded. Nearest-governs then has to prefer the deeper + // folded match, or an opted-out subtree keeps forwarding under the marker its + // parent carries. + const r = resolverOver([ + { dir: '/root', class: 'full' }, + { dir: `/root/${NFD}`, class: 'ignore' }, + ]) + assert.equal(r.resolve('/root/elsewhere').class, 'full') + assert.equal(r.resolve(`/root/${NFC}/deep`).class, 'ignore') +}) + +test('resolve: a carve-out declared in the same spelling as the entry it carves out of is still honored', () => { + // The positive half: the two-pass rule must not over-restrict a legitimate + // nested loosening, only one that crosses spellings. + const r = resolverOver([ + { dir: '/root/real', class: 'ignore' }, + { dir: `/root/real/${NFC}`, class: 'full' }, + ]) + assert.equal(r.resolve(`/root/real/${NFC}/sub`).class, 'full') +}) + +test('resolve: nearest-governs is measured on the folded spelling, not on the declared one', () => { + // NFD is *longer in code units* than NFC for the same name, so a depth + // measured on the declared string can rank a decomposed ancestor above a + // composed descendant and invert nearest-governs. Five accented characters + // is enough: the outer entry is 16 code units decomposed against the inner + // entry's 14 composed, but 11 against 14 once both are folded. + const outer = 'ééééé' + const outerNfd = outer.normalize('NFD') + assert.ok(`/root/${outerNfd}`.length > `/root/${outer}/ab`.length) + const r = resolverOver([ + { dir: `/root/${outerNfd}`, class: 'ignore' }, + { dir: `/root/${outer}/ab`, class: 'full' }, + ]) + assert.equal(r.resolve(`/root/${outer}/ab/deep`).class, 'full') + // ...and the outer entry still governs everything the carve-out does not. + assert.equal(r.resolve(`/root/${outer}/other`).class, 'ignore') +}) + +test('resolve: folding never loosens, over every arrangement of a nested pair', () => { + // Exhaustive rather than illustrative: for every pair of classes and every + // assignment of spellings to the outer and inner entry, the folded verdict is + // at least as restrictive as the verdict the declared spellings alone give. + const RANK = { ignore: 2, 'local-only': 1, full: 0 } + const classes = /** @type {const} */ (['ignore', 'local-only', 'full']) + const spellings = [NFC, NFD] + for (const outerClass of classes) { + for (const innerClass of classes) { + for (const outerSpelling of spellings) { + for (const innerSpelling of spellings) { + for (const cwdSpelling of spellings) { + const entries = [ + { dir: `/root/${outerSpelling}`, class: outerClass }, + { dir: `/root/${innerSpelling}/inner`, class: innerClass }, + ] + const cwd = `/root/${cwdSpelling}/inner/deep` + const folded = resolverOver(entries).resolve(cwd).class + // The pre-fold answer, computed here from the same fixture by the + // plain string rule the matcher used before this change. + const preFold = declaredOnlyVerdict(entries, cwd) + assert.ok( + RANK[folded] >= RANK[preFold], + `folded ${folded} is looser than pre-fold ${preFold} for outer=${outerClass} inner=${innerClass}` + ) + } + } + } + } + } +}) + +/** + * The pre-fold rule, reimplemented from `master`: longest matching declared + * `dir` wins, ties broken by the more restrictive class, nothing matching means + * `full`. + * + * @param {readonly { dir: string, class: 'ignore' | 'local-only' | 'full' }[]} entries + * @param {string} cwd + * @returns {'ignore' | 'local-only' | 'full'} + */ +function declaredOnlyVerdict(entries, cwd) { + const RANK = { ignore: 2, 'local-only': 1, full: 0 } + const matches = entries.filter((e) => cwd === e.dir || cwd.startsWith(e.dir + '/')) + if (matches.length === 0) return 'full' + return matches.reduce((best, e) => { + if (e.dir.length > best.dir.length) return e + if (e.dir.length === best.dir.length && RANK[e.class] > RANK[best.class]) return e + return best + }).class +} + +// --- the case half, over an injected volume verdict ----------------------- + +test('resolve: case is NOT folded by default, because this volume is case-sensitive', () => { + // The correctness bug in the other direction: on Linux and on a + // case-sensitive APFS volume `Proj` and `proj` are genuinely two + // directories, and folding them would over-restrict. + const r = resolverOver([{ dir: '/root/Proj', class: 'ignore' }]) + assert.equal(r.resolve('/root/proj').class, 'full') +}) + +test('resolve: case IS folded when the volume verdict says the volume is case-insensitive', () => { + const r = resolverOver([{ dir: '/root/Proj', class: 'local-only' }], { caseInsensitiveVolume: () => true }) + assert.equal(r.resolve('/root/proj/sub').class, 'local-only') +}) + +test('resolve: a case-insensitive volume verdict still does not let a carve-out loosen a broader entry', () => { + const r = resolverOver( + [ + { dir: '/root/real', class: 'ignore' }, + { dir: '/root/real/Proj', class: 'full' }, + ], + { caseInsensitiveVolume: () => true } + ) + assert.equal(r.resolve('/root/real/proj/sub').class, 'ignore') +}) + +test('resolve: the case verdict is asked per entry, so a per-volume answer applies per entry', () => { + /** @type {string[]} */ + const asked = [] + const r = resolverOver( + [ + { dir: '/insensitive/A', class: 'ignore' }, + { dir: '/sensitive/B', class: 'ignore' }, + ], + { + caseInsensitiveVolume: (dir) => { + asked.push(dir) + return dir.startsWith('/insensitive/') + }, + } + ) + assert.equal(r.resolve('/insensitive/a').class, 'ignore') + assert.equal(r.resolve('/sensitive/b').class, 'full') + assert.deepEqual(asked, ['/insensitive/A', '/sensitive/B']) +}) + +// --- the probe itself ------------------------------------------------------ + +test('createVolumeCaseProbe is inert off darwin: constant false, and it issues no syscall', () => { + // The only claim about the probe this host can witness. Its darwin behaviour + // is not exercised anywhere in this suite and is not verified by it. + let statCalls = 0 + const probe = createVolumeCaseProbe({ + platform: 'linux', + statSync: () => { + statCalls += 1 + return { dev: 1, ino: 1 } + }, + }) + assert.equal(probe('/root/Proj'), false) + assert.equal(probe('/anything'), false) + assert.equal(statCalls, 0) +}) + +test('createVolumeCaseProbe memoizes a definite verdict per volume, not per path', () => { + /** @type {string[]} */ + const statted = [] + const probe = createVolumeCaseProbe({ + platform: 'darwin', + statSync: (p) => { + statted.push(p) + // One volume (dev 7) where every spelling names the same inode. + return { dev: 7, ino: 42 } + }, + }) + assert.equal(probe('/vol/Proj'), true) + assert.equal(probe('/vol/Other'), true) + assert.equal(probe('/vol/deep/Nested'), true) + // Two stats for the first path (the path and its case-flipped spelling), then + // one per later path to learn its `dev`, and no second probe of the volume. + assert.deepEqual(statted, ['/vol/Proj', '/vol/pROJ', '/vol/Other', '/vol/deep/Nested']) +}) + +test('createVolumeCaseProbe does not memoize an undetermined answer as the volume verdict', () => { + // A directory whose name has no cased character (`/vol/123`) admits no probe, + // but that says nothing about the volume. Caching the fallback would let one + // such path decide the verdict for every other path on the same disk. + const probe = createVolumeCaseProbe({ + platform: 'darwin', + statSync: () => ({ dev: 7, ino: 42 }), + }) + assert.equal(probe('/vol/123'), false) + assert.equal(probe('/vol/Proj'), true) +}) + +test('createVolumeCaseProbe reports case-sensitive when the flipped spelling does not exist', () => { + const probe = createVolumeCaseProbe({ + platform: 'darwin', + statSync: (p) => { + if (p === '/vol/Proj') return { dev: 7, ino: 42 } + const err = /** @type {Error & { code: string }} */ (new Error('ENOENT')) + err.code = 'ENOENT' + throw err + }, + }) + assert.equal(probe('/vol/Proj'), false) +}) + +test('createVolumeCaseProbe fails toward the pre-fold behaviour and logs a hashed skip', () => { + /** @type {{ name: string, fields: Record }[]} */ + const events = [] + const probe = createVolumeCaseProbe({ + platform: 'darwin', + statSync: () => { + const err = /** @type {Error & { code: string }} */ (new Error('EACCES')) + err.code = 'EACCES' + throw err + }, + logSkip: (name, fields) => events.push({ name, fields: fields ?? {} }), + }) + assert.equal(probe('/vol/Secret'), false) + assert.equal(events.length, 1) + assert.equal(events[0].name, 'usage_policy.case_probe_skipped') + assert.equal(events[0].fields.error_kind, PATH_CASE_PROBE_ERROR_KIND) + assert.equal(events[0].fields.status, 'skipped') + assert.equal(events[0].fields.errno, 'eacces') + assert.match(String(events[0].fields.path_hash), /^[0-9a-f]{16}$/) + // The raw path never appears in any attribute value. + for (const value of Object.values(events[0].fields)) { + assert.ok(!String(value).includes('Secret'), `raw path leaked in ${String(value)}`) + } +}) + +// --- the structured signal on the hot path -------------------------------- + +test('resolve emits a hashed usage_policy.fold_tightened only when folding changed the verdict', () => { + /** @type {{ name: string, fields: Record }[]} */ + const events = [] + const r = resolverOver([{ dir: `/root/${NFC}`, class: 'local-only' }], { + logEvent: (name, fields) => events.push({ name, fields: fields ?? {} }), + }) + + // Same spelling: nothing to report. + assert.equal(r.resolve(`/root/${NFC}/a`).class, 'local-only') + assert.equal(events.length, 0) + + // Divergent spelling: the fold is what produced the restriction. + assert.equal(r.resolve(`/root/${NFD}/a`).class, 'local-only') + assert.equal(events.length, 1) + assert.equal(events[0].name, 'usage_policy.fold_tightened') + assert.equal(events[0].fields.hyp_operation, 'match_list') + assert.equal(events[0].fields.declared_class, 'none') + assert.equal(events[0].fields.folded_class, 'local-only') + assert.match(String(events[0].fields.cwd_hash), /^[0-9a-f]{16}$/) + for (const value of Object.values(events[0].fields)) { + assert.ok(!String(value).includes('caf'), `raw path leaked in ${String(value)}`) + } + + // An unrelated cwd nothing governs: still nothing to report. + events.length = 0 + assert.equal(r.resolve('/elsewhere').class, 'full') + assert.equal(events.length, 0) +}) + +// --- the hot path stays memoized on the lexical key ----------------------- + +test('resolve: folding happens on the cache miss only, so a repeated cwd re-reads nothing', () => { + let listReads = 0 + let caseVerdicts = 0 + const files = { [LIST]: JSON.stringify({ version: 2, entries: [{ dir: `/root/${NFC}`, class: 'ignore' }] }) } + const r = createUsagePolicyResolver({ + existsSync: (p) => Object.prototype.hasOwnProperty.call(files, p), + readFileSync: (p) => { + listReads += 1 + return /** @type {Record} */ (files)[p] + }, + localOnlyListPath: LIST, + caseInsensitiveVolume: () => { + caseVerdicts += 1 + return false + }, + now: () => 1000, + }) + for (let i = 0; i < 50; i += 1) assert.equal(r.resolve(`/root/${NFD}/a`).class, 'ignore') + assert.equal(listReads, 1) + assert.equal(caseVerdicts, 1) +}) From 1bf7baace10ac76102a7573078b4dbe28a062856 Mon Sep 17 00:00:00 2001 From: neutral-reconciler Date: Thu, 30 Jul 2026 07:52:58 +0000 Subject: [PATCH 2/3] Review fixes: pin the unmemoized probe failure, escape the NFD fixtures, correct two cost claims Round-1 review of #484. - `createVolumeCaseProbe` deliberately does not memoize a non-`ENOENT` failure of the case-flipped `stat`, but nothing pinned it: mutating the branch to cache the undetermined verdict left the suite green. On a real volume a transient `EACCES`/`EIO` would then disable case folding for every path on that disk for the life of the resolver, silently reopening the gap the module exists to close. New test kills the mutant. - The NFC/NFD fixtures were raw UTF-8, while the comment above them claimed they were spelled as escapes. Re-normalizing the source really would have collapsed them (verified: it reddens 3 tests), so the tripwire worked, but the stated mechanism did not exist. Now actually `\u`-escaped, so the file is pure ASCII and the collapse is unrepresentable rather than merely detectable. The tripwire test stays, for anyone who reintroduces raw characters. - The probe's JSDoc claimed "one pair of stat calls per distinct volume, not one per directory". The memo is keyed on `dev`, which must be learned first, so every call stats `dir` itself; only the flipped stat is saved. Corrected to the actual bound (n stats plus one per volume, per TTL window). - LLP 0050 quoted normalize('NFC') at ~60 ns, measured on a pure-ASCII path, which is the case where the fold does nothing. On a genuinely decomposed path, the macOS case the section exists for, it is ~550 ns and scales with length. Added the real numbers and the reason the cost is affordable (miss path only). - LLP 0050 said only that the unfolded CLI sites "can name a different governor". Named each site and its direction instead: the CLI can never promise more protection than the gate delivers, `--check` reports the right class but can name a narrower scope, `policy unset` can refuse to remove an entry the gate enforces (fails toward privacy), and `hyp purge --subtree` can report success while retaining rows (the one site that fails away from privacy). No behaviour change: the only non-test edits are JSDoc and documentation. Co-Authored-By: Claude --- ...50-ignore-enforced-in-adapters.decision.md | 50 ++++++++++++++++--- src/core/usage-policy/fold.js | 17 ++++--- test/core/usage-policy-fold.test.js | 48 +++++++++++++++--- 3 files changed, 96 insertions(+), 19 deletions(-) diff --git a/llp/0050-ignore-enforced-in-adapters.decision.md b/llp/0050-ignore-enforced-in-adapters.decision.md index e1b129c1..89f51b34 100644 --- a/llp/0050-ignore-enforced-in-adapters.decision.md +++ b/llp/0050-ignore-enforced-in-adapters.decision.md @@ -178,8 +178,23 @@ nearest-governs. The per-`cwd` memo is keyed on the **lexical** path and consulted **before** any folding, so a cache hit costs exactly what it did before. Entry spellings and the per-volume case verdict are computed once per list parse, inside the TTL -window LLP 0049 R6 already bounds. `String.prototype.normalize('NFC')` is -roughly 60 ns on a pure-ASCII path. +window LLP 0049 R6 already bounds. + +`String.prototype.normalize('NFC')` is roughly 60 ns on a pure-ASCII path, but +that is the case where the fold does nothing, so it is the wrong number to plan +against. When the string is **already** composed, normalize verifies and +returns: about 60 ns for a short ASCII path and about 90 ns for a 114-character +path with accented segments. When it is genuinely decomposed, which is exactly +the macOS case this section exists for, it has to recompose: about **550 ns** +for a 134-character NFD path, roughly 9x the ASCII figure, and it scales with +length (about 9.6 us for a pathological 1000-character all-decomposed path). + +That is affordable only because of where it sits. Folding is on the cache-**miss** +path (once per `cwd` per TTL window) and on the list parse (once per entry per +window), never per exchange. A miss over a 20-entry list with a long NFD `cwd` +measures about 8.2 us before this change and 9.7 us after. A caller that ever +moves the fold onto a per-row or per-exchange path has to re-measure with a +decomposed path, not an ASCII one. ### Relationship to the symlink class @@ -195,10 +210,33 @@ two, since running the argmax guard twice buys nothing. The fold is applied at the **gate** (`resolve` / list membership). The one-shot CLI membership sites (`hyp ignore --check`, `policy show`, `policy unset`) and `hyp purge --subtree` still compare lexically, so on a case-insensitive or -NFD-carrying volume the CLI can still name a different governor than the gate -used. Those sites are exactly what #482 reroutes through a single shared -spelling-aware predicate; they should adopt the fold there, once, rather than -grow a second copy of the rule. +NFD-carrying volume they can disagree with the gate. Those sites are exactly +what #482 reroutes through a single shared spelling-aware predicate; they should +adopt the fold there, once, rather than grow a second copy of the rule. + +The disagreement is bounded in one direction and not in the other, and the +difference matters enough to name each site: + +- **The CLI can never promise more protection than the gate delivers.** The + lexical predicate matches a subset of what the folded one does, and the gate's + class is `max(declared, folded)`, so any entry the CLI finds the gate also + found. There is no spelling on which `--check` reports a directory protected + while the gate forwards it. +- **`hyp ignore --check` / `policy show` report the right class and can name the + wrong scope.** The class comes from `resolve()`, so it is folded and correct. + Only `resolveCheckScopeDir`'s "which listed directory governs this?" lookup is + lexical, so when the entry reaches `cwd` only by folding it falls back to the + queried path. The class is right; the governing directory shown, and the + residual row count scoped to it, are narrower than the truth. +- **`policy unset` / `unignore --local-only` can refuse to remove an entry the + gate is enforcing**, when the user spells the path the other way. It reports + "not governed" and exits 0. That fails toward privacy: the opt-out stays on. +- **`hyp purge --subtree` can fail to purge rows it reports as purged**, when the + rows were recorded under a different spelling of the target. This is the one + site that fails **away** from privacy: the user asked for data to be deleted, + the command succeeds, and the rows remain. It is unchanged from the pre-fold + behaviour rather than introduced here, but it is the reason this seam should + not stay open for long. ## Consequences diff --git a/src/core/usage-policy/fold.js b/src/core/usage-policy/fold.js index e7a7044a..f2b7e306 100644 --- a/src/core/usage-policy/fold.js +++ b/src/core/usage-policy/fold.js @@ -121,12 +121,17 @@ function flipCase(p) { * Create a memoized per-volume case-sensitivity probe. * * The verdict is a property of the mounted volume, so it is memoized by the - * volume's `dev` number rather than by path: one pair of `stat` calls per - * distinct volume for the life of the resolver, not one per directory and not - * one per TTL window. That is strictly under the per-`cwd`-per-window bound - * LLP 0049 R6 already sets for the ancestor walk. A volume cannot change its - * case-sensitivity without being unmounted and reformatted, at which point its - * `dev` changes too, so there is nothing for a TTL to refresh. + * volume's `dev` number rather than by path. The memo is keyed on `dev`, which + * has to be *learned* before it can be consulted, so the cost is not zero on a + * hit: every call `stat`s `dir` itself (one `stat` per directory), and only the + * second, case-flipped `stat` is saved by the memo. So a list of `n` entries + * costs `n` stats plus one extra per distinct volume, per TTL window, rather + * than `2n`. That is still within the per-`cwd`-per-window bound LLP 0049 R6 + * sets for the ancestor walk, which already stats every ancestor. A volume + * cannot change its case-sensitivity without being unmounted and reformatted, + * at which point its `dev` changes too, so there is nothing for a TTL to + * refresh, and the flipped-spelling probe genuinely runs once per volume for + * the life of the resolver. * * **Inert off darwin.** On any other platform the probe returns `false` * immediately and issues **no syscall at all**, because no shipping diff --git a/test/core/usage-policy-fold.test.js b/test/core/usage-policy-fold.test.js index 2004c6d7..bd358cba 100644 --- a/test/core/usage-policy-fold.test.js +++ b/test/core/usage-policy-fold.test.js @@ -33,12 +33,15 @@ import { createVolumeCaseProbe, foldPath, PATH_CASE_PROBE_ERROR_KIND } from '../ * @import { UsageClass, UsagePolicyResolver } from '../../src/core/usage-policy/types.js' */ -// The same four characters, composed and decomposed. Spelled as escapes so the -// file cannot be silently re-normalized by an editor, a merge tool, or a -// `git` filter, which would turn both constants into the same string and make -// every test below pass vacuously. -const NFC = 'café' -const NFD = 'café' +// The same four characters, composed and decomposed. Spelled as \u escapes, so +// the source file is pure ASCII and an editor, a merge tool, or a `git` filter +// cannot silently re-normalize it. Re-normalizing raw literals would collapse +// both constants to the same string and make every test below pass vacuously; +// escaping makes that unrepresentable rather than merely detectable, and the +// tripwire immediately below still asserts the premise for anyone who +// reintroduces raw characters. +const NFC = 'caf\u00e9' +const NFD = 'cafe\u0301' // The premise the whole file rests on. If this ever fails, nothing after it // means anything. @@ -151,7 +154,7 @@ test('resolve: nearest-governs is measured on the folded spelling, not on the de // composed descendant and invert nearest-governs. Five accented characters // is enough: the outer entry is 16 code units decomposed against the inner // entry's 14 composed, but 11 against 14 once both are folded. - const outer = 'ééééé' + const outer = '\u00e9\u00e9\u00e9\u00e9\u00e9' const outerNfd = outer.normalize('NFD') assert.ok(`/root/${outerNfd}`.length > `/root/${outer}/ab`.length) const r = resolverOver([ @@ -310,6 +313,37 @@ test('createVolumeCaseProbe does not memoize an undetermined answer as the volum assert.equal(probe('/vol/Proj'), true) }) +test('createVolumeCaseProbe does not memoize a non-ENOENT failure of the flipped stat', () => { + // The other undetermined branch, and the one that actually bites on a real + // volume: the directory itself stats fine, but the case-flipped spelling + // fails with something that is *not* `ENOENT` (`EACCES` on a directory the + // daemon may not traverse, `EIO` on a flaky mount). `ENOENT` would be + // informative, since it means the flipped spelling does not resolve and the + // volume is therefore case-sensitive. Any other errno says nothing at all, so + // caching it would let one transient error disable case folding for every + // path on that disk for the life of the resolver, silently reopening the gap + // this module exists to close. + let flippedFails = true + const probe = createVolumeCaseProbe({ + platform: 'darwin', + statSync: (p) => { + if (p === '/vol/Proj' || p === '/vol/Other') return { dev: 7, ino: 42 } + if (flippedFails) { + const err = /** @type {Error & { code: string }} */ (new Error('EACCES')) + err.code = 'EACCES' + throw err + } + return { dev: 7, ino: 42 } + }, + logSkip: () => {}, + }) + assert.equal(probe('/vol/Proj'), false) + // The volume verdict must still be open, so a later probe of the same `dev` + // that *can* reach an answer is believed rather than served the stale `false`. + flippedFails = false + assert.equal(probe('/vol/Other'), true) +}) + test('createVolumeCaseProbe reports case-sensitive when the flipped spelling does not exist', () => { const probe = createVolumeCaseProbe({ platform: 'darwin', From aee8e6f6a3a6de65454132d071235cd552a45562 Mon Sep 17 00:00:00 2001 From: neutral-reconciler Date: Thu, 30 Jul 2026 08:11:02 +0000 Subject: [PATCH 3/3] Round-2 review: name why unconditional NFC folding is gate-only, and the purge seam it must not be reused in LLP 0050 justified folding NFC unconditionally with "there is no volume on which folding NFC and NFD together is wrong, because no filesystem this codebase targets lets two paths that differ only by normalization name two different directories". That premise is false, and demonstrably so on this host: `caf`+U+00E9 and `cafe`+U+0301 are two directories with two inodes, both present in one parent, each holding different content. The decision is still right, but for a gate-specific reason: the resolved class is max(declared, folded), so a fold that merges two distinct directories can only over-restrict. Recorded as such, with an explicit warning not to reuse foldPath where widening deletes or discloses. Also in "Not covered": - the `hyp purge ` retention gap now carries the observed transcript ("purged 0 rows from 0 partitions", exit 0, empty stderr) and notes the inversion that the succeeding purge is the noisier of the two; - records that `hyp purge --ignored` IS covered by this change, because it classifies through resolver.resolve(); verified against master, which leaves the row. That is the durable workaround for the subtree gap; - notes that closing the subtree gap is not a foldPath drop-in, because on a Linux volume that would delete a genuinely different sibling's rows. Docs only; no behaviour change. Co-Authored-By: Claude --- ...50-ignore-enforced-in-adapters.decision.md | 80 ++++++++++++++++--- 1 file changed, 68 insertions(+), 12 deletions(-) diff --git a/llp/0050-ignore-enforced-in-adapters.decision.md b/llp/0050-ignore-enforced-in-adapters.decision.md index 89f51b34..60d62891 100644 --- a/llp/0050-ignore-enforced-in-adapters.decision.md +++ b/llp/0050-ignore-enforced-in-adapters.decision.md @@ -106,7 +106,7 @@ not of the path and not of the platform: | mechanism | folded by | volume-dependent? | |---|---|---| | symlinked components | `realpath(2)` | no | -| Unicode normalization (NFC vs NFD) | nothing in `node:fs` | yes, but folding to NFC is safe everywhere | +| Unicode normalization (NFC vs NFD) | nothing in `node:fs` | yes, and folding is **unsafe** where it does not apply, but harmless *at the gate* (see below) | | case | nothing in `node:fs` | yes, and folding is **unsafe** where it does not apply | `realpath(2)` resolves symlinks and does nothing else. On a default macOS @@ -123,11 +123,34 @@ versus a client reporting a `cwd`). **Decision: list membership compares a folded spelling of both sides.** The fold is `src/core/usage-policy/fold.js`: -1. **NFC unconditionally.** It is a total function of the string, needs no - filesystem access, cannot fail, and is the identity on a path that is already - composed. There is no volume on which folding NFC and NFD together is wrong, - because no filesystem this codebase targets lets two paths that differ only - by normalization name two different directories. +1. **NFC unconditionally, and only because this is the gate.** It is a total + function of the string, needs no filesystem access, cannot fail, and is the + identity on a path that is already composed. What makes it safe here is + **not** that NFC and NFD always name one directory. They do not: on every + Linux volume this codebase targets, `caf` + U+00E9 and `cafe` + U+0301 are + two directories with two inodes, and both can exist in one parent + (demonstrated on an ext4-backed overlay host: distinct `ino`, distinct + contents, both listed by `readdir`). Folding them together therefore *can* + merge two genuinely different directories, exactly as unconditional case + folding would. + + It is safe at the gate anyway, for a reason specific to the gate: the + resolved class is `max(declared, folded)` (`selectGoverning`, and the argmax + discussion below), so a fold that merges two distinct directories can only + ever **over-restrict**, i.e. decline to record a directory that was in fact + permitted. That is a usability cost + and never a privacy or data-loss one. Case is put behind a probe rather than + given the same treatment because case aliasing is far more likely to collide + with a real, deliberately-distinct sibling (`Makefile` vs `makefile`) than a + normalization difference is, not because NFC folding is universally sound. + + **Do not reuse `foldPath` in a predicate where widening is not free.** In a + *deletion* predicate (`hyp purge`) or a *disclosure* predicate, widening + removes or reveals rows for a directory the user did not name, and the + `max()` argument above does not apply. Closing the purge seam below needs + either a per-volume normalization-insensitivity probe (no such probe exists; + the current one answers only the case question) or a darwin-only guard. + See "Not covered". 2. **Case only behind a per-volume probe.** Case-sensitivity is a property of the mounted volume: an APFS volume can be formatted case-sensitive and every ext4 volume is. Folding it unconditionally would merge two genuinely @@ -231,12 +254,45 @@ difference matters enough to name each site: - **`policy unset` / `unignore --local-only` can refuse to remove an entry the gate is enforcing**, when the user spells the path the other way. It reports "not governed" and exits 0. That fails toward privacy: the opt-out stays on. -- **`hyp purge --subtree` can fail to purge rows it reports as purged**, when the - rows were recorded under a different spelling of the target. This is the one - site that fails **away** from privacy: the user asked for data to be deleted, - the command succeeds, and the rows remain. It is unchanged from the pre-fold - behaviour rather than introduced here, but it is the reason this seam should - not stay open for long. +- **`hyp purge ` (the subtree target) silently retains rows it was asked + to delete**, when the rows were recorded under a different spelling of the + target. This is the one site that fails **away** from privacy: the user asked + for data to be deleted, the command reports success, and the rows remain. + Observed end to end (rows recorded NFD, purge argument NFC, and the reverse; + also a case alias): + + ``` + # the argument is typed NFC; the rows were recorded under the NFD spelling. + # the two render identically, which is the whole problem. + $ hyp purge ~/café/proj --yes + purged 0 rows from 0 partitions + $ echo $? + 0 + ``` + + Nothing is written to stderr and the exit status is 0, so the outcome is + indistinguishable from "that directory had nothing cached". Note the + inversion: a purge that *succeeds* prints the resurrection warning on stderr, + so the failing case is the **quieter** of the two. It is unchanged from the + pre-fold behaviour rather than introduced here, and it is tracked separately; + it is the reason this seam should not stay open for long. + +- **`hyp purge --ignored` is already covered by this change**, because that + target classifies each row through `resolver.resolve(row.cwd)` rather than + through a lexical prefix test, so it inherits the fold. Verified against + `master`: with an `ignore` entry declared NFC and rows recorded NFD, `master` + purges 0 rows and leaves the row, and this branch purges it. So marking the + directory and running `hyp purge --ignored` is the durable workaround for the + subtree gap above until that gap is closed. + +Whoever closes the subtree gap should note that it is **not** a matter of +dropping `foldPath` into the predicate. Purge deletes, so widening the match is +not free the way it is at the gate (see "NFC unconditionally, and only because +this is the gate"): on a Linux volume, folding would delete cached rows for a +genuinely different sibling directory that differs only by normalization. The +fix needs the fold gated on the volume actually being normalization-insensitive, +and the shared predicate PR #482 introduces (`scopeGoverns`, which reroutes this +same purge call site for the symlink class) is the right place to put it. ## Consequences