From ecea537194ddfbcc12742b58b9dbf594079edcb3 Mon Sep 17 00:00:00 2001 From: 0xdhx Date: Wed, 5 Aug 2026 07:23:25 -0500 Subject: [PATCH] docs(#2665): the guard watches config.toml but GSD also writes /hooks/ there MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found pre-push by this round's third adversarial review pass. Not a rebase regression — round 3 shipped it and #2755 doubled it. resolveExtraWatchTargets watches one config.toml per non-registry descriptor, and its comment asserted "GSD writes ONE named file into these third-party roots". That is false: bin/install.js also calls installSharedHooksBundle on the same root, populating /hooks/ with GSD's hook scripts and a CommonJS marker. So a suite-produced leak of a hook bundle into a developer's real ~/.kimi or ~/.kimi-code passes this guard silently — #2665's own hazard, in #2665's own safety net. Behaviour is deliberately unchanged and the gap is disclosed instead. Closing it is a layout decision rather than one more path, for the same reason getGlobalSkillsBase is already a deliberate non-target: the snapshot applies the config-root layout beneath every root it is given, and these roots are not ours. Happy to fix it here or take it as a separate issue — the maintainer's call. The enumeration-relative test could not have caught this: it asserts one target PER DESCRIPTOR and nothing about whether one per descriptor is enough, because its expectation is derived from the same array it checks. That is exactly the scope boundary round-2 Nit 7 asked to be marked, biting one layer up from where it was marked; the test now says so. 479979c4's message says "there are three" — that is three WATCHED targets, not a count of write surfaces. The hooks bundle is a fourth, and unwatched. lint:ci rc=0; tests/live-config-guard.test.cjs 24/24. Comments and catalog only. --- CONTEXT.md | 4 +-- docs/CONTEXT-INDEX.json | 4 +-- .../CONTEXT-INDEX.json | 4 +-- scripts/live-config-guard.cjs | 30 ++++++++++++++----- tests/live-config-guard.test.cjs | 6 ++++ 5 files changed, 34 insertions(+), 14 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 3f7d1de5a..89a482b9b 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -588,13 +588,13 @@ The prompt-level data/instruction isolation seam for untrusted web/document ingr `WORKSTREAM.NAME.POLICY.cjs-module=gsd-core/bin/lib/workstream-name-policy.cjs owns toWorkstreamSlug + active-name/path-segment validation` `WORKSTREAM.POINTER.SEAM.cjs-module=gsd-core/bin/lib/active-workstream-store.cjs owns read/write self-heal for .planning/active-workstream` `CONFIG.SEAM.loadConfig-context=loadConfig(cwd,{workstream}) replaces env-mutation fallback; no temporary process.env GSD_WORKSTREAM rewrites` -`CONFIG.LOCATION.SEAM.scrub-set=tests/helpers.cjs CONFIG_LOCATION_ENV_KEYS is DERIVED from five sources, never hand-listed: capability-registry runtimes[].runtime.configHome.env AND [].configHome.skillsHome.env + runtime-homes NON_REGISTRY_CONFIG_HOME_DESCRIPTORS[].env AND [].skillsHome.env (a descriptor is a descriptor — BOTH descriptor rungs walk skillsHome, which resolves independently via resolveSkillsBaseFromDescriptor) + runtime-homes GSD_LOCATION_ENV_KEYS + a residue list (GROK_AGENTS_HOME, GSD_RUNTIME, GSD_PROJECT, GSD_WORKSTREAM) + WRITE_ESCAPE_PERMISSION_ENV_KEYS (GSD_ALLOW_SYMLINKED_DEST — a permission, not a location: it names no path but disarms the symlink-escape guard, so blanking it makes the guard STRICTER, never looser); adding a config-location var means making it ENUMERABLE at one of those sources, not appending a literal` +`CONFIG.LOCATION.SEAM.scrub-set=tests/helpers.cjs CONFIG_LOCATION_ENV_KEYS is DERIVED from five sources rather than maintained as one hand-written list (source 4 IS a literal residue list, for vars that fit no other rung — what is never hand-listed is the SET): capability-registry runtimes[].runtime.configHome.env AND [].configHome.skillsHome.env + runtime-homes NON_REGISTRY_CONFIG_HOME_DESCRIPTORS[].env AND [].skillsHome.env (a descriptor is a descriptor — BOTH descriptor rungs walk skillsHome, which resolves independently via resolveSkillsBaseFromDescriptor) + runtime-homes GSD_LOCATION_ENV_KEYS + a residue list (GROK_AGENTS_HOME, GSD_RUNTIME, GSD_PROJECT, GSD_WORKSTREAM) + WRITE_ESCAPE_PERMISSION_ENV_KEYS (GSD_ALLOW_SYMLINKED_DEST — a permission, not a location: it names no path but disarms the symlink-escape guard, so blanking it makes the guard STRICTER, never looser); adding a config-location var means making it ENUMERABLE at one of those sources, not appending a literal` `CONFIG.LOCATION.SEAM.two-families=runtime configHomes (where a third-party runtime keeps config, registry- or descriptor-declared) and GSD's OWN location vars (GSD_HOME -> $GSD_HOME/.gsd store, GSD_AGENTS_DIR -> getAgentsDir priority 1) are DISTINCT families; no registry derivation reaches the second, and treating a miss there as a registry gap is what produced review round 2` `CONFIG.LOCATION.SEAM.kimi-two-homes=kimi declares TWO config-location vars: KIMI_CONFIG_DIR (registry, generic Agent-Skills root via resolveKimiGlobalDir) and KIMI_SHARE_DIR (KIMI_HOOKS_TOML_DESCRIPTOR, kimi's OWN native config.toml carrying GSD's [[hooks]] block via resolveKimiHooksTomlDir); a registry-only derivation covers the first and silently misses the second` `CONFIG.LOCATION.SEAM.in-process-scrub=TEST_ENV_BASE reaches CHILD env only; a test calling install() IN-PROCESS must additionally use helpers.scrubConfigLocationEnv() in beforeEach + its restorer in afterEach — HOME/USERPROFILE sandboxing is NOT sufficient because getGlobalConfigDir is env-FIRST` `LIVE-CONFIG.GUARD.SEAM.module=scripts/live-config-guard.cjs (deliberately NOT scripts/lib/, which the installer copies to users wholesale while uninstall removes only an allowlist; excluded from the npm tarball via package.json files[] together with its whole require chain run-tests.cjs/affected-tests-lib.cjs/run-affected-tests.cjs — a partial exclusion trips the #2858 shipped-requires-only-shipped gate); exports [resolveLiveConfigRoots, resolveExtraWatchTargets, snapshotLiveConfig, diffLiveConfig, formatViolations, newestMtime]; driven by scripts/run-tests.cjs pre/post suite` `LIVE-CONFIG.GUARD.SEAM.scope=ownership-based, never whole-root: GSD_OWNED_ENTRIES top-level footprint + children whose name startsWith GSD_ARTIFACT_PREFIX ('gsd-') under GSD_PREFIXED_PARENTS (dirs shared with the host agent); watching a shared root wholesale false-positives on the host's own writes and a guard that cries wolf gets disabled` -`LIVE-CONFIG.GUARD.SEAM.non-root-targets=resolveExtraWatchTargets covers THREE live write surfaces that are not runtime config ROOTS (skills bases are a DELIBERATE non-target — the config-root layout misfires beneath them, so they need their own layout): $GSD_HOME/.gsd watched WHOLESALE (exclusively GSD-owned, so the shared-root trap does not apply) plus ONE config.toml per NON_REGISTRY_CONFIG_HOME_DESCRIPTORS entry, each watched as a SINGLE FILE (those roots belong to their products) — today three targets, since #2755 split Kimi CLI (~/.kimi, KIMI_SHARE_DIR) from Kimi Code (~/.kimi-code, KIMI_CODE_HOME); the targets are DERIVED by iterating that array, never by calling a named resolver, so a further descriptor is picked up without editing the guard PROVIDED it owns the same NON_REGISTRY_OWNED_FILE ('config.toml') — one that owns a different filename needs a per-descriptor mapping, the named residual the guard states at its own definition; passed to snapshotLiveConfig explicitly so a fixture-root caller cannot pull the real ~/.gsd into its snapshot` +`LIVE-CONFIG.GUARD.SEAM.non-root-targets=resolveExtraWatchTargets covers THREE live write surfaces that are not runtime config ROOTS (skills bases are a DELIBERATE non-target — the config-root layout misfires beneath them, so they need their own layout): $GSD_HOME/.gsd watched WHOLESALE (exclusively GSD-owned, so the shared-root trap does not apply) plus ONE config.toml per NON_REGISTRY_CONFIG_HOME_DESCRIPTORS entry, each watched as a SINGLE FILE (those roots belong to their products) — today three targets, since #2755 split Kimi CLI (~/.kimi, KIMI_SHARE_DIR) from Kimi Code (~/.kimi-code, KIMI_CODE_HOME); the targets are DERIVED by iterating that array, never by calling a named resolver, so a further descriptor is picked up without editing the guard PROVIDED it owns the same NON_REGISTRY_OWNED_FILE ('config.toml') — one that owns a different filename needs a per-descriptor mapping, the named residual the guard states at its own definition. SECOND RESIDUAL: config.toml is not all GSD writes into those roots — installSharedHooksBundle also populates /hooks/, which is UNWATCHED; closing it is a layout decision, like skills bases; passed to snapshotLiveConfig explicitly so a fixture-root caller cannot pull the real ~/.gsd into its snapshot` `LIVE-CONFIG.GUARD.SEAM.truncation=MAX_ENTRIES/MAX_DEPTH bound the walk; a bound hit sets truncated and diffLiveConfig emits kind:'unverified' — a truncated scan MUST NOT read as clean; boundary covered at {limit-1,limit,limit+1} via newestMtime's injected budget plus fast-check monotonicity, per RULESET.TESTS.boundary-coverage + RULESET.TESTS.property-based-testing` `LIVE-CONFIG.GUARD.SEAM.severity=reports by default locally; CI wires GSD_STRICT_LIVE_CONFIG_GUARD=1 on Linux/macOS lanes (test.yml, all three test jobs) so a suite-produced leak FAILS those runs; Windows lanes stay report-only pending the documented pre-existing USERPROFILE sweep (~190 test sites sandbox HOME alone) — promote once that lands; skipped by GSD_SKIP_LIVE_CONFIG_GUARD=1` `LIVE-CONFIG.GUARD.SEAM.ci-blind=the AMBIENT-ENV half stays CI-blind — CI never has these vars set, so green CI is not evidence for it; what strict mode catches in CI is the suite's own default-root leaks (HOME/USERPROFILE-derived), the guard remains the only loud signal for ambient-var escapes` diff --git a/docs/CONTEXT-INDEX.json b/docs/CONTEXT-INDEX.json index 14bdf45d6..f8f430ba9 100644 --- a/docs/CONTEXT-INDEX.json +++ b/docs/CONTEXT-INDEX.json @@ -53,7 +53,7 @@ { "id": "CONFIG.LOCATION.SEAM.scrub-set", "klass": "CONFIG", - "value": "tests/helpers.cjs CONFIG_LOCATION_ENV_KEYS is DERIVED from five sources, never hand-listed: capability-registry runtimes[].runtime.configHome.env AND [].configHome.skillsHome.env + runtime-homes NON_REGISTRY_CONFIG_HOME_DESCRIPTORS[].env AND [].skillsHome.env (a descriptor is a descriptor — BOTH descriptor rungs walk skillsHome, which resolves independently via resolveSkillsBaseFromDescriptor) + runtime-homes GSD_LOCATION_ENV_KEYS + a residue list (GROK_AGENTS_HOME, GSD_RUNTIME, GSD_PROJECT, GSD_WORKSTREAM) + WRITE_ESCAPE_PERMISSION_ENV_KEYS (GSD_ALLOW_SYMLINKED_DEST — a permission, not a location: it names no path but disarms the symlink-escape guard, so blanking it makes the guard STRICTER, never looser); adding a config-location var means making it ENUMERABLE at one of those sources, not appending a literal" + "value": "tests/helpers.cjs CONFIG_LOCATION_ENV_KEYS is DERIVED from five sources rather than maintained as one hand-written list (source 4 IS a literal residue list, for vars that fit no other rung — what is never hand-listed is the SET): capability-registry runtimes[].runtime.configHome.env AND [].configHome.skillsHome.env + runtime-homes NON_REGISTRY_CONFIG_HOME_DESCRIPTORS[].env AND [].skillsHome.env (a descriptor is a descriptor — BOTH descriptor rungs walk skillsHome, which resolves independently via resolveSkillsBaseFromDescriptor) + runtime-homes GSD_LOCATION_ENV_KEYS + a residue list (GROK_AGENTS_HOME, GSD_RUNTIME, GSD_PROJECT, GSD_WORKSTREAM) + WRITE_ESCAPE_PERMISSION_ENV_KEYS (GSD_ALLOW_SYMLINKED_DEST — a permission, not a location: it names no path but disarms the symlink-escape guard, so blanking it makes the guard STRICTER, never looser); adding a config-location var means making it ENUMERABLE at one of those sources, not appending a literal" }, { "id": "CONFIG.LOCATION.SEAM.two-families", @@ -988,7 +988,7 @@ { "id": "LIVE-CONFIG.GUARD.SEAM.non-root-targets", "klass": "LIVE-CONFIG", - "value": "resolveExtraWatchTargets covers THREE live write surfaces that are not runtime config ROOTS (skills bases are a DELIBERATE non-target — the config-root layout misfires beneath them, so they need their own layout): $GSD_HOME/.gsd watched WHOLESALE (exclusively GSD-owned, so the shared-root trap does not apply) plus ONE config.toml per NON_REGISTRY_CONFIG_HOME_DESCRIPTORS entry, each watched as a SINGLE FILE (those roots belong to their products) — today three targets, since #2755 split Kimi CLI (~/.kimi, KIMI_SHARE_DIR) from Kimi Code (~/.kimi-code, KIMI_CODE_HOME); the targets are DERIVED by iterating that array, never by calling a named resolver, so a further descriptor is picked up without editing the guard PROVIDED it owns the same NON_REGISTRY_OWNED_FILE ('config.toml') — one that owns a different filename needs a per-descriptor mapping, the named residual the guard states at its own definition; passed to snapshotLiveConfig explicitly so a fixture-root caller cannot pull the real ~/.gsd into its snapshot" + "value": "resolveExtraWatchTargets covers THREE live write surfaces that are not runtime config ROOTS (skills bases are a DELIBERATE non-target — the config-root layout misfires beneath them, so they need their own layout): $GSD_HOME/.gsd watched WHOLESALE (exclusively GSD-owned, so the shared-root trap does not apply) plus ONE config.toml per NON_REGISTRY_CONFIG_HOME_DESCRIPTORS entry, each watched as a SINGLE FILE (those roots belong to their products) — today three targets, since #2755 split Kimi CLI (~/.kimi, KIMI_SHARE_DIR) from Kimi Code (~/.kimi-code, KIMI_CODE_HOME); the targets are DERIVED by iterating that array, never by calling a named resolver, so a further descriptor is picked up without editing the guard PROVIDED it owns the same NON_REGISTRY_OWNED_FILE ('config.toml') — one that owns a different filename needs a per-descriptor mapping, the named residual the guard states at its own definition. SECOND RESIDUAL: config.toml is not all GSD writes into those roots — installSharedHooksBundle also populates /hooks/, which is UNWATCHED; closing it is a layout decision, like skills bases; passed to snapshotLiveConfig explicitly so a fixture-root caller cannot pull the real ~/.gsd into its snapshot" }, { "id": "LIVE-CONFIG.GUARD.SEAM.scope", diff --git a/examples/dynamic-context-management/CONTEXT-INDEX.json b/examples/dynamic-context-management/CONTEXT-INDEX.json index 17f723f29..19885c75b 100644 --- a/examples/dynamic-context-management/CONTEXT-INDEX.json +++ b/examples/dynamic-context-management/CONTEXT-INDEX.json @@ -58,7 +58,7 @@ { "id": "CONFIG.LOCATION.SEAM.scrub-set", "klass": "CONFIG", - "value": "tests/helpers.cjs CONFIG_LOCATION_ENV_KEYS is DERIVED from five sources, never hand-listed: capability-registry runtimes[].runtime.configHome.env AND [].configHome.skillsHome.env + runtime-homes NON_REGISTRY_CONFIG_HOME_DESCRIPTORS[].env AND [].skillsHome.env (a descriptor is a descriptor — BOTH descriptor rungs walk skillsHome, which resolves independently via resolveSkillsBaseFromDescriptor) + runtime-homes GSD_LOCATION_ENV_KEYS + a residue list (GROK_AGENTS_HOME, GSD_RUNTIME, GSD_PROJECT, GSD_WORKSTREAM) + WRITE_ESCAPE_PERMISSION_ENV_KEYS (GSD_ALLOW_SYMLINKED_DEST — a permission, not a location: it names no path but disarms the symlink-escape guard, so blanking it makes the guard STRICTER, never looser); adding a config-location var means making it ENUMERABLE at one of those sources, not appending a literal", + "value": "tests/helpers.cjs CONFIG_LOCATION_ENV_KEYS is DERIVED from five sources rather than maintained as one hand-written list (source 4 IS a literal residue list, for vars that fit no other rung — what is never hand-listed is the SET): capability-registry runtimes[].runtime.configHome.env AND [].configHome.skillsHome.env + runtime-homes NON_REGISTRY_CONFIG_HOME_DESCRIPTORS[].env AND [].skillsHome.env (a descriptor is a descriptor — BOTH descriptor rungs walk skillsHome, which resolves independently via resolveSkillsBaseFromDescriptor) + runtime-homes GSD_LOCATION_ENV_KEYS + a residue list (GROK_AGENTS_HOME, GSD_RUNTIME, GSD_PROJECT, GSD_WORKSTREAM) + WRITE_ESCAPE_PERMISSION_ENV_KEYS (GSD_ALLOW_SYMLINKED_DEST — a permission, not a location: it names no path but disarms the symlink-escape guard, so blanking it makes the guard STRICTER, never looser); adding a config-location var means making it ENUMERABLE at one of those sources, not appending a literal", "line": 582 }, { @@ -1180,7 +1180,7 @@ { "id": "LIVE-CONFIG.GUARD.SEAM.non-root-targets", "klass": "LIVE-CONFIG", - "value": "resolveExtraWatchTargets covers THREE live write surfaces that are not runtime config ROOTS (skills bases are a DELIBERATE non-target — the config-root layout misfires beneath them, so they need their own layout): $GSD_HOME/.gsd watched WHOLESALE (exclusively GSD-owned, so the shared-root trap does not apply) plus ONE config.toml per NON_REGISTRY_CONFIG_HOME_DESCRIPTORS entry, each watched as a SINGLE FILE (those roots belong to their products) — today three targets, since #2755 split Kimi CLI (~/.kimi, KIMI_SHARE_DIR) from Kimi Code (~/.kimi-code, KIMI_CODE_HOME); the targets are DERIVED by iterating that array, never by calling a named resolver, so a further descriptor is picked up without editing the guard PROVIDED it owns the same NON_REGISTRY_OWNED_FILE ('config.toml') — one that owns a different filename needs a per-descriptor mapping, the named residual the guard states at its own definition; passed to snapshotLiveConfig explicitly so a fixture-root caller cannot pull the real ~/.gsd into its snapshot", + "value": "resolveExtraWatchTargets covers THREE live write surfaces that are not runtime config ROOTS (skills bases are a DELIBERATE non-target — the config-root layout misfires beneath them, so they need their own layout): $GSD_HOME/.gsd watched WHOLESALE (exclusively GSD-owned, so the shared-root trap does not apply) plus ONE config.toml per NON_REGISTRY_CONFIG_HOME_DESCRIPTORS entry, each watched as a SINGLE FILE (those roots belong to their products) — today three targets, since #2755 split Kimi CLI (~/.kimi, KIMI_SHARE_DIR) from Kimi Code (~/.kimi-code, KIMI_CODE_HOME); the targets are DERIVED by iterating that array, never by calling a named resolver, so a further descriptor is picked up without editing the guard PROVIDED it owns the same NON_REGISTRY_OWNED_FILE ('config.toml') — one that owns a different filename needs a per-descriptor mapping, the named residual the guard states at its own definition. SECOND RESIDUAL: config.toml is not all GSD writes into those roots — installSharedHooksBundle also populates /hooks/, which is UNWATCHED; closing it is a layout decision, like skills bases; passed to snapshotLiveConfig explicitly so a fixture-root caller cannot pull the real ~/.gsd into its snapshot", "line": 588 }, { diff --git a/scripts/live-config-guard.cjs b/scripts/live-config-guard.cjs index 7290371b8..85711c772 100644 --- a/scripts/live-config-guard.cjs +++ b/scripts/live-config-guard.cjs @@ -156,12 +156,16 @@ function resolveLiveConfigRoots(deps = {}) { * into, one per NON_REGISTRY_CONFIG_HOME_DESCRIPTORS entry: Kimi * CLI's ~/.kimi (KIMI_SHARE_DIR) and, since #2755, Kimi Code's * ~/.kimi-code (KIMI_CODE_HOME). The INVERSE case: those roots - * belong to their products, so only the one file GSD writes is - * watched, never the root. This is the KNOWN GAP above accepted - * deliberately in one direction — GSD demonstrably writes these - * files (bin/install.js resolves the hooks-toml dir at two sites), - * so a concurrent write by those products is the only false - * positive, and neither runs during the suite. + * belong to their products, so the root is never watched + * wholesale. This is the KNOWN GAP above accepted deliberately + * in one direction — GSD demonstrably writes these files + * (bin/install.js resolves the hooks-toml dir at two sites), so + * a concurrent write by those products is the only false + * positive, and neither runs during the suite. NOT watched, and + * it is a real residual rather than a bound: /hooks/, the + * bundle installSharedHooksBundle writes into the same roots — + * see resolveExtraWatchTargets for why closing it is a layout + * decision. * * @returns {string[]} absolute paths; empty if the built lib is absent. */ @@ -190,8 +194,18 @@ function resolveExtraWatchTargets(deps = {}) { // untestable and the targets resolved against different worlds. for (const descriptor of NON_REGISTRY_CONFIG_HOME_DESCRIPTORS) { const dir = resolveConfigHomeFromDescriptor(descriptor, { env, home: homedir() }); - // GSD writes ONE named file into these third-party roots; the root itself - // belongs to the runtime, so it is never watched wholesale. + // The root belongs to the runtime, so it is never watched wholesale — only + // the named file below. + // + // NAMED RESIDUAL (#2665, found pre-push while rebasing): config.toml is NOT + // the only thing GSD writes here. bin/install.js also calls + // installSharedHooksBundle(kimiHooksRoot), which populates /hooks/ + // with GSD's hook scripts and a CommonJS marker. That subtree is UNWATCHED, + // so a suite-produced leak of a hook bundle into a developer's real ~/.kimi + // or ~/.kimi-code passes this guard silently. Closing it needs a layout + // decision, not one more path: the same reason getGlobalSkillsBase is a + // deliberate non-target above. Under-watching fails quiet, like the + // NON_REGISTRY_OWNED_FILE residual it sits beside. targets.push(path.resolve(path.join(dir, NON_REGISTRY_OWNED_FILE))); } } catch { diff --git a/tests/live-config-guard.test.cjs b/tests/live-config-guard.test.cjs index 1f8e1a1a5..8e00e0b96 100644 --- a/tests/live-config-guard.test.cjs +++ b/tests/live-config-guard.test.cjs @@ -210,6 +210,12 @@ describe('#2665: guard watches non-root write surfaces', () => { // named resolver instead would cover one of today's two entries and silently // miss tomorrow's — the same partial-enumeration defect that put // KIMI_SHARE_DIR outside the scrub set, one layer over. + // + // SCOPE BOUNDARY (per round-2 Nit 7, and it bites here): this asserts one + // target PER DESCRIPTOR and nothing about whether one target per descriptor + // is ENOUGH. It is not — /hooks/ is also GSD-written and unwatched + // (named residual in resolveExtraWatchTargets). A test whose expectation is + // derived from the same array it checks cannot see that class. for (const d of NON_REGISTRY_CONFIG_HOME_DESCRIPTORS) { const dir = resolveConfigHomeFromDescriptor(d, { env, home }); assert.ok(