* fix(#3712): confine in-process installs to a sandboxed HOME
A runtime kind may declare a global `home` override resolved from os.homedir()
rather than from the caller's configDir — codex's skills kind (`home: ".agents"`,
ADR-1239 / #2088) is the only live case. Sandboxing configDir/targetDir does not
contain it, and assertDestWithinConfigHome cannot see the class: that gate
confines a destSubpath to whatever root it is handed, and here the root IS the
escaped home. So an in-process caller that forgot to sandbox HOME wrote to, and
pruned gsd-* entries from, the developer's REAL ~/.agents/skills.
tests/agent-descriptor-parity.install.test.cjs's K1 loop did exactly that: it
iterates every agents-kind runtime (codex included) with a sandboxed targetDir
and an un-sandboxed HOME. Reproduced against a canary home on next @ adb46cdd8 —
71 gsd-* skill dirs deleted, a foreign `cloudflare` skill surviving, suite still
exit 0. It is silent because the runtime's own config home is untouched, so the
manifest keeps reporting a healthy install.
FIVE writers resolve a kind `home` and then destroy under it. Three are reachable
today — installRuntimeArtifacts, uninstallRuntimeArtifacts (install-engine.cts)
and applySurface (surface.cts). Two are descriptor-dependent and guarded against a
future descriptor change rather than a present escape: installOpencodeFamilySkills
(behind the combined-family early return) and installAgentsKindStandalone. Those
two are scoped to the single kind each destroys — passing the whole layout made
codex's unrelated skills override trip a writer that never touches it.
- src/test-home-guard.cts: refuse when a run under a test runner cannot be shown
to have sandboxed HOME. NODE_TEST_CONTEXT (set by `node --test`) gates it, so
installs outside a Node test context are untouched; GSD_TEST_MODE is unusable,
as several candidate files including the offender never set it. Homes are
compared by FILESYSTEM IDENTITY (st_dev + st_ino), not by pathname:
path.resolve() resolves neither symlinks nor case, and realpath returns a
canonical pathname that two routes to one directory can still disagree on (bind
mounts). Verified on macOS/APFS — HOME=/users/<name> made the strings differ
while naming the same directory, and the lexical form ALLOWED a write into the
physical real home. FAILS CLOSED: a pair is "different" only when both identify,
or one is definitively absent (ENOENT/ENOTDIR) while the other identifies; every
other errno is "cannot tell" and refuses. Only when neither home identifies is a
marker consulted, and it carries the sandbox PATH and must equal the home in
effect — a boolean checked first let an ambient or stale value disarm the guard.
- helpers: promote sandboxHome() out of its two byte-identical private copies,
which is also what makes them record the sandbox; the three withFakeHome()
helpers record it too. The marker NAME is duplicated as a bare string rather
than required from the compiled guard, keeping helpers.cjs's documented
no-built-lib-at-import-time contract; a test pins the two together.
- agent-descriptor-parity: sandbox HOME across the K1 loop.
- helpers-process-isolation: #3156's canary asserts on <home>/.gsd only, and its
`--cursor --local` spawn cannot reach `.agents` at all, so an assertion added
there would pass with all confinement removed. Add a discriminating row — a
`--codex --global` spawn against a seeded ambient home — which also asserts the
runtime still declares the override. Its check is a sampled inventory (dir names
+ each SKILL.md), not a tree compare.
- install-write-confinement: predicate rows through the deps seam, covering the
symlinked HOME, ambient and stale markers, and each sameDirectory branch
(both-identify, one-absent, neither-identifiable), plus wiring rows that drive
the REAL entrypoints so deleting a guard call site is red.
Verified: guard fires end-to-end against a real un-sandboxed HOME (exit 1, zero
deletions); the case-variant fail-open reproduced on APFS before the fix and
refuses after; K1 file 29/29 green with skills intact; mutation-tested — each of
the three reachable call sites, lexical-only comparison, and treating an unknown
errno as "absent" each take exactly one row red, with every mutation echoed back;
the process-isolation row negative-controlled by reverting installerEnv to its
pre-#3156 leak (16/0 -> 13/3); a full npm test leaves ~/.agents/skills at 71.
Stated residual: the two descriptor-dependent writers have no wiring test, because
no runtime declares a `home` override on those kinds and neither can be exercised
without inventing a descriptor.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* chore(#3712): add changeset
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(#3712): let a sandbox nested inside the real home through the guard
All six Windows shards of #3725 failed on legitimately sandboxed
destinations. On Windows os.tmpdir() is %LOCALAPPDATA%\Temp — inside the
user's home — so every sandbox a test creates is a descendant of the real
home, and "does this land inside the real home?" answers yes for the safe
case and the dangerous one alike. POSIX conceals this: /tmp and
/var/folders both sit outside $HOME.
Add the missing conjunct: a destination inside the real home is allowed
only when it also sits beneath a HOME that was sandboxed away from the
passwd home. Both halves are required — dropping the first re-admits a
plain un-sandboxed install, and dropping the second decays into the
"is HOME sandboxed?" check the module rejects, which a layout resolved
before the sandbox walks straight through. Each is mutation-proven by a
row that goes red without it.
Also covers the two fail-closed branches of the new exemption, which
survived mutation to `true` with the suite green, and avoids `<user>` in
a docblock — the prompt-injection scanner reads it as a delimiter tag.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(#3712): close three writer/rollback gaps found reviewing the whole PR
Cross-AI review of the full PR (not just the round's delta) surfaced
three ways the guard could still be defeated:
- The nested-sandbox exemption trusted the SPELLING of a destination.
With HOME sandboxed to a directory inside the real home — legitimate on
Windows — an aliased `.agents` (symlink, junction, subordinate bind
mount) beneath it redirected an allowed path into the real home. Decide
containment on the path the write RESOLVES to: walk up to the nearest
existing ancestor, canonicalize, re-append the tail.
- `migrateLegacyDevPreferencesToSkill` is a SIXTH writer that resolves a
skills-kind `home` override. It creates rather than prunes, which is
why it was missed, and `_runLegacyInstallMigrations` runs it before
`installRuntimeArtifacts`' own assertion. Guarded, scoped to that kind.
- Worst of the three: `bin/install.js` snapshots the resolved skills root
before installing, and its outer catch rolls back by deleting and
recreating every snapshotted `gsd-*` directory there. The guard's own
throw landed in that catch, so refusing an un-sandboxed codex install
provoked exactly the mutation the guard exists to prevent. Refusals are
now marked and rethrown without rollback — nothing was written, so
there is no partial install to undo. Every other error still rolls back.
Also carries the sandbox marker into `installSpawnEnv`, so spawned
installers are not refused on passwd-less CI images, and corrects three
claims that no longer hold: "every writer" (six, and named), the
unconditional "fails CLOSED" (the passwd-less marker branch is a
deliberate weakening, and TOCTOU is out of scope), and the assertion that
Windows os.tmpdir() is always %LOCALAPPDATA%\Temp (Node honors TEMP/TMP).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(#3712): name the guard's two limits instead of overclaiming
Round-2 review found the prose had drifted ahead of the code. Corrected,
with no behavior change:
- The module still said FIVE writers; there are six, and the sixth is
now named along with why it was missed (it creates rather than prunes)
and why it carries its own assertion (it runs before the main one).
- The canonicalization docblock listed subordinate bind mounts among the
aliases it closes. It does not close them: a bind mount is not a link,
so realpath keeps the mount-point spelling. `sameDirectory` already
recorded that limit; the new helper now inherits it explicitly rather
than contradicting it. Closing it needs mount-table introspection.
- "FAILS CLOSED" was unqualified while the passwd-less marker branch is
a deliberate weakening — with no passwd entry, nothing can contradict a
marker naming the real home.
- "Refuses BEFORE any write" was too broad: legacy install migrations run
ahead of the layout-driven ones, which is exactly why the two
rollbackInstallerMigrations() calls still execute before the rethrow.
Only the codex skills-root rollback is skipped, and that is the only
_codexPreConfigRollback() call site — applySurface is never called from
bin/install.js and uninstall cannot reach it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(#3712): sandbox HOME in the opencode-family home-override parity rows
The last two Windows failures, and the same platform asymmetry in a
different disguise. This row drives a skills-kind `home` override on
purpose — precisely what the guard polices — but relied on the override
temp dir happening to sit outside the real home. It does on POSIX
(/tmp, /var/folders); on Windows os.tmpdir() is under %USERPROFILE%, so
the guard correctly refused and only Windows went red.
Declare the sandbox instead of depending on the platform: HOME becomes
the override itself, which is the home the call writes under. This is the
fix the guard's own message prescribes, applied to the test rather than
to the guard.
Both failing Windows shards fail on exactly these two rows and nothing
else; every other shard is green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(#3712): make sameDirectory answer NO when it cannot tell
Review Major 1. sameDirectory()'s only caller is the passwd-less marker
branch, which reads a `true` as permission to PROCEED:
if (marker && sameDirectory(marker, osMod.homedir())) return;
The fallthrough returned `true` whenever neither side identified — two
absent paths, or two stats failing EACCES/EPERM/EIO on a locked-down
host — on the reasoning that "cannot tell" should make the caller refuse.
That reasoning was inverted with respect to this caller: it turned the
passwd-less escape hatch into an unconditional bypass for any marker
value at all, on precisely the hosts the fallback exists to serve. Only
two things now answer yes: one resolved pathname, or two readable
identities that match. Restoring the old fallthrough takes the new row
red.
Also from review:
- Major 2 asked whether st_dev/st_ino discriminate directories on
Windows, where Node derives them from BY_HANDLE_FILE_INFORMATION. The
whole guard rests on that primitive, so assert it rather than argue it:
a row comparing two distinct temp directories, and one directory
reached by two spellings. It runs on every platform in the matrix, so
Windows answers the question itself.
- Minor 1: the refusal now names the real home it compared against, not
just the destination it refused. That is the one fact needed to tell a
true positive from a false one, and its absence is what made the
Windows case a CI-log dig rather than a glance.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(#3712): refuse a HOME that merely spells the real home more widely
Review round: one Blocker, four Minors, a Nit.
N2 (the one with teeth) — a destination's ancestor chain is linear, so
"inside the real home AND inside the effective HOME" admits two
arrangements, not one. The intended `effectiveHome ⊂ realHome` is the
Windows temp shape; `realHome ⊂ effectiveHome` — HOME at /Users, /home,
C:\Users — is not a sandbox at all, it is the real home reached by a
wider spelling, and it was exempting a stale destination pointing
straight at ~/.agents. Third conjunct added; the docblock no longer
claims two conditions suffice. Removing the conjunct reds the new row
and nothing else.
N4 — the migration guard resolved its OWN layout, and without
capabilityRegistry, so a registry-dependent descriptor could make it
vouch for a path the migration does not write: a guard reporting safe
while the unsafe write proceeds. It now guards the destination already
resolved by _resolveDevPreferencesSkillTarget, keyed on
`installRoot !== targetDir` — which is exactly the condition under which
a `home` override was declared, read off that same result.
N1 — CONTEXT.md gains the Test Home Guard Module glossary entry that
contributor-standards.md requires of a new Module. Not CI-enforced, so
green CI was never evidence it was met.
N3 — the docs/INVENTORY.md row was misfiled between install-fs-adapter
and install-model-override-resolver; the table is alphabetical and the
manifest already had it right. Moved, and its text now names six writers
and the third conjunct.
N5 — applySurface's signature docblock was two parameters stale; this PR
added the second of them.
N6 — the duplicated rollbackInstallerMigrations() adjacent to the new
rethrow: two identical consecutive calls, not two phases.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(#3712): guard the sixth writer, and close two false-ALLOW paths
Review round 3 (NEW-1, NEW-2) plus three defects Codex found in the
whole-PR pass, each reproduced before it was fixed.
NEW-1 — migrateLegacyDevPreferencesToSkill called the guard with no
`deps`, so it bound real os/process.env and could not be wiring-tested
the way the other three reachable writers were. It now takes the same
optional `deps: { os?, env? }` tail parameter. The wiring block gains
the missing fourth row, and a fifth pinning the ALLOW half; the test
file's header docblock said "FIVE writers ... the three reachable
today", contradicting the six/four statement this PR already put in
src/test-home-guard.cts, CONTEXT.md, docs/INVENTORY.md and the
changeset. Both directions of the guard's condition now fail a row
when broken — previously neither did.
NEW-2 — derivesFromSandboxedHome's docblock claimed "THREE conditions
are required, and no two of them suffice". False for {2,3}: isInside is
reflexive, so whenever conjunct 1 fires conjunct 2 already returns
false on its own. Reworded as a fast path, which is what it is.
Codex 1 (false ALLOW) — on a host with no readable passwd entry the
marker branch returned as soon as the marker matched the effective
HOME. That attests a caller sandboxed HOME and says nothing about where
an already-resolved destination points, so a layout captured before
sandboxHome() — still naming the real ~/.agents — was waved straight
through: the same stale-layout shape the primary branch refuses by
design. The marker must now identify AND contain every destination.
Codex 2 (false ALLOW) — `installRoot !== targetDir` was the stand-in
for "the skills kind declared a home override". The two are not
equivalent: the inequality is false when the override resolves onto
targetDir itself, which is exactly a configDir of $HOME/.agents. The
guard was skipped and SKILL.md written into the real home under a test
runner. _resolveDevPreferencesSkillTarget now reports hasHomeOverride
off the same resolution instead of inferring it from two paths.
Codex 3 (prose) — the shared refusal message claimed every guarded
writer prunes; the migrate writer only creates. The changeset headline
claimed in-process installer calls can no longer reach the real home,
which is wider than the guard: writeNonClaudeDefaults still writes
~/.gsd/defaults.json through os.homedir(). INVENTORY's and CONTEXT's
fail-closed sentences omitted sameDirectory's pathname-equality
shortcut. All four narrowed to what the code does.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(#3712): let the sandbox marker follow an overridden HOME
Found by Codex in the whole-PR pass. installSpawnEnv spreads
`overrides` last so an explicit HOME wins — deliberate, and its
docblock tells callers needing per-spawn isolation to pass their own
{ HOME, USERPROFILE }. But the #3712 marker was set before that spread,
so such a caller got HOME=<theirs> and marker=<helper default>. On a
host with no readable passwd entry the guard compares the two and
refuses a legitimately sandboxed spawn — tests/install.test.cjs:7143
and install-shared.cjs's own runInstaller both take that path.
The marker is now derived from the final HOME unless the caller
supplied one explicitly. The contract test asserted HOME after an
override but not the marker, which is why it stayed green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(#3712): name the shipped guard condition, not the deleted one
Review round 4 of #3725. Two artifacts this PR adds still described
`target.installRoot !== targetDir` in the PRESENT tense as the live guard
condition on `migrateLegacyDevPreferencesToSkill`. The shipped condition is
`runtime && target.hasHomeOverride` (src/install-engine.cts:510).
This is not ordinary doc drift. The named condition is the exact false-ALLOW
the previous round closed: a `home` override resolving onto `targetDir` — a
configDir of `$HOME/.agents`, which is where codex's override points — makes
the inequality FALSE while the override is declared, so the guard was skipped.
A maintainer reading CONTEXT.md:290 as authoritative would believe the guard
still skips that case.
- tests/install-write-confinement.test.cjs — the ALLOW-half row's comment.
Its "teeth" rationale is unchanged and still correct as written.
- CONTEXT.md:290 — the Test Home Guard Module glossary entry, a documented
PR gate. Now states the condition and names the inequality only as what it
is NOT, with the reason.
The three surviving mentions of the inequality are all past-tense or negated
(src/install-engine.cts:448, :508 and the sibling test comment at :3698) and
are correct as they stand.
Verified: `npm run lint:ci` exit 0; full `npm test` 31327 tests / 31312 pass /
0 fail / 14 skipped, run with TMPDIR unset.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(#3712): canonicalization fails closed, matching identify's errno split
Codex full-PR review of #3725, run against the round-4 head.
`resolveThroughLinks` caught EVERY realpathSync error and fell back to
`path.resolve(dest)` — the lexical spelling. That inverts the function's own
purpose. An aliased `<sandbox>/.agents` that cannot be canonicalized keeps its
sandbox spelling, satisfies the nested-sandbox exemption at :227, and the write
is ALLOWED into the real home — the exact escape this walk exists to close. The
module documents that it fails CLOSED with ONE named exception (the marker
branch); this was a second, unnamed one.
Split by errno, and deliberately by the SAME split `identify` already draws
rather than a second policy in one module — both answer "does this path exist
as named?", so they must not disagree:
ENOENT / ENOTDIR -> walk up. The ordinary case: a fresh install resolves a
destination nothing has created yet, so realpath fails on the leaf and on
every not-yet-created ancestor. Refusing here rejects every install.
anything else (EACCES, EPERM, ELOOP, EIO) -> refuse. The component exists but
cannot be resolved, so the guard cannot tell where the write lands.
Three rows in the predicate block, beside the other aliasing rows:
- a symlink CYCLE in the destination path (ELOOP) -> REFUSE
- a destination that does not exist yet (ENOENT) -> ALLOW
- a component behind a regular file (ENOTDIR) -> ALLOW
Teeth checked against the artifact the test loads, not the source: reverting
the condition to the swallow-everything shape in the compiled
test-home-guard.cjs turns row 1 — and only row 1 — red. The ENOTDIR row caught
a stale build during development, which is the point of asserting on the
compiled file.
CONTEXT.md and the resolveThroughLinks docblock both record the new behaviour,
so this does not repeat the prose-vs-code drift the round-4 finding was about.
The changeset's existing scope sentence now bounds "six writers" to the
`installRuntimeArtifacts` call tree and names `cmdGenerateDevPreferences` —
which resolves the same codex `home` override through `getGlobalSkillsBase` and
writes SKILL.md beneath it unguarded. It has no in-process caller today (its
only direct require-and-call is a spawnSync with HOME sandboxed), so it is
latent rather than live, and whether it belongs in this PR is raised with the
maintainer rather than decided here.
Verified: `npm run lint:ci` exit 0; full `npm test` 31330 tests / 31315 pass /
0 fail / 14 skipped, TMPDIR unset.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Behruz Nassre Esfahani
committed by
GitHub
GitHub
parent
1178c5f995
commit
a44d513566
5
.changeset/calm-pandas-greet.md
Normal file
5
.changeset/calm-pandas-greet.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3725
|
||||
---
|
||||
**In-process installs can no longer write a kind's `home` override into your real home** — a runtime kind with a global `home` override (codex skills → `$HOME/.agents`) resolves from `os.homedir()`, not from the caller's config dir, so a test that sandboxed only its target directory pruned every `gsd-*` skill from the developer's real `~/.agents/skills` while the suite still passed and the manifest still reported a healthy install. All six writers that resolve a kind `home` now refuse when a `node --test` run would land inside the real home, compared by filesystem identity rather than pathname and decided on where the write resolves rather than how it is spelled. Scope is stated rather than implied: the six are the writers on the `installRuntimeArtifacts` call tree, and this covers destinations a runtime kind resolves through a `home` override, not every path the installer touches through `os.homedir()` (`writeNonClaudeDefaults`' `~/.gsd/defaults.json` is still reached by a spawned installer with an un-sandboxed HOME) and not writers off that tree (`cmdGenerateDevPreferences` resolves the same codex `home` override through `getGlobalSkillsBase` and writes `SKILL.md` beneath it unguarded; it has no in-process caller today, so it is latent rather than live). Canonicalization fails CLOSED rather than falling back to the lexical spelling — an unresolvable component (`EACCES`/`ELOOP`/`EIO`) is refused, since that fallback is the exact ALLOW an aliased `<sandbox>/.agents` needs; only `ENOENT`/`ENOTDIR` walk up, matching `identify`'s own errno split. Two limits are named in the source rather than papered over: a subordinate bind mount of the real directory into a sandbox is not detectable without mount-table introspection, and on a host with no readable passwd entry the guard falls back to a caller-set marker — which must itself identify, and must contain every destination, so a layout captured before the sandbox is still refused. Real installs are unaffected. (#3712)
|
||||
1
.gitignore
vendored
1
.gitignore
vendored
@@ -79,6 +79,7 @@ build/
|
||||
/gsd-core/bin/lib/install-effort-resolver.cjs
|
||||
/gsd-core/bin/lib/install-model-override-resolver.cjs
|
||||
/gsd-core/bin/lib/install-engine.cjs
|
||||
/gsd-core/bin/lib/test-home-guard.cjs
|
||||
/gsd-core/bin/lib/embedding-adapter.cjs
|
||||
/gsd-core/bin/lib/adapter-declarative.cjs
|
||||
/gsd-core/bin/lib/adapter-imperative.cjs
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -41,6 +41,7 @@ const {
|
||||
// consentRequired, hostPrecedenceRank) instead of the id being re-derived
|
||||
// and re-interpreted at each call site. See src/install-scope.cts.
|
||||
const { resolveScope } = require('../gsd-core/bin/lib/install-scope.cjs');
|
||||
const { isTestHomeGuardRefusal } = require('../gsd-core/bin/lib/test-home-guard.cjs');
|
||||
// getDirName (runtime -> local config dir name) is relocated out of this
|
||||
// installer to the runtime-name-policy leaf (ADR-1508 / #1510 Phase 1) so the
|
||||
// conversion module's rewrite engine can consume it without importing
|
||||
@@ -11614,7 +11615,20 @@ function install(isGlobal, runtime = DEFAULT_RUNTIME, options = {}) {
|
||||
// #3245 CR finding 2 — any throw in the pre-config install operations (skills copy,
|
||||
// agents copy, VERSION write, manifest write, etc.) triggers the Codex pre-config
|
||||
// rollback so the caller is never left in a partially-installed state.
|
||||
rollbackInstallerMigrations();
|
||||
// (The second, identical rollbackInstallerMigrations() that used to sit here was
|
||||
// a duplicate of the line above, not a second phase — removed in #3725 review.)
|
||||
// #3712 — the test-home guard refuses before any LAYOUT-DRIVEN write, so no
|
||||
// gsd-* directory in the skills root has been touched and there is nothing
|
||||
// there to undo. (Legacy install migrations DO run first; that is why the
|
||||
// rollbackInstallerMigrations() calls above still execute, and why the one
|
||||
// migration that can reach a `home` override carries its own assertion.)
|
||||
// Running the codex rollback anyway would delete and recreate every
|
||||
// snapshotted gsd-* directory in the resolved skills root, which for an
|
||||
// un-sandboxed codex install IS the real ~/.agents/skills: the guard's own
|
||||
// refusal would provoke the mutation it exists to prevent. This is the only
|
||||
// _codexPreConfigRollback() call site, and applySurface/uninstall cannot
|
||||
// reach it. Every other error still rolls back. Found by review, not by CI.
|
||||
if (isTestHomeGuardRefusal(_earlyInstallErr)) throw _earlyInstallErr;
|
||||
if (_codexPreConfigRollback) {
|
||||
_codexPreConfigRollback();
|
||||
}
|
||||
|
||||
@@ -508,6 +508,7 @@
|
||||
"task-command-router.cjs",
|
||||
"teams-status.cjs",
|
||||
"template.cjs",
|
||||
"test-home-guard.cjs",
|
||||
"text-lines.cjs",
|
||||
"token-scanner.cjs",
|
||||
"uat-predicate.cjs",
|
||||
|
||||
@@ -640,6 +640,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`.
|
||||
| `task-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools task` |
|
||||
| `teams-status.cjs` | Detects agent-teams status from environment and runtime; pure core (#1355) |
|
||||
| `template.cjs` | Template selection and filling with variable substitution |
|
||||
| `test-home-guard.cjs` | Test-home confinement guard (#3712) — refuses any of the six writers that resolve a kind `home` — `installRuntimeArtifacts`, `uninstallRuntimeArtifacts`, `applySurface`, `migrateLegacyDevPreferencesToSkill`, plus the descriptor-dependent `installOpencodeFamilySkills` and `installAgentsKindStandalone` when a `node --test` run would resolve a kind's global `home` override (codex skills -> `$HOME/.agents`, ADR-1239/#2088) inside the real passwd home, which silently pruned every `gsd-*` skill there; compares homes by filesystem identity (`st_dev`+`st_ino`) rather than by pathname, since a case-variant, symlinked, or bind-mounted HOME names one directory under two names; fails CLOSED unless both homes identify or one is definitively absent (the pathname-equality shortcut in `sameDirectory` can answer yes without identifying, so the marker branch requires the marker to identify separately before it may allow anything); a destination inside the real home is exempted only when HOME differs from the passwd home, the passwd home is not itself beneath that HOME (`/Users`, `C:\Users` are not sandboxes), and the destination resolves beneath it (Windows puts the temp root inside the home, so containment alone cannot tell a sandbox from the danger); where no passwd entry is readable it falls back to `sandboxHome()`'s path-valued marker, which must identify AND contain every resolved destination — a marker matching HOME attests only that HOME was sandboxed, so on its own it waved through a layout captured before the sandbox that still named the real `~/.agents`; it remains a deliberate weakening rather than a closed door, since nothing there can contradict a marker naming the real home; does not defend against a subordinate bind mount of the real directory into the sandbox (realpath cannot unify bind-mounted spellings) or a cross-process TOCTOU swap; inert for normal installs outside a Node test context |
|
||||
| `text-lines.cjs` | Line-terminator handling seam — `splitLines`/`normalizeEol`/`detectEol`/`joinLines`, the sole owner of `\r?\n` splitting and CRLF normalization; closes #3360's split-then-match fix in `frontmatter.cjs` (ADR-3212 §3, epic #3212 Phase 2, #3413) |
|
||||
| `token-scanner.cjs` | Tokenizer-first seam for stateful grammars — `tokenizeShellLike` (quote-aware shell tokenizer, the primitive `hooks/lib/git-cmd.js` migrated onto) and `indentWidth` (bullet-nesting depth, closes #3169's cross-reference-vs-declaration false positive in `decisions.cts`) (ADR-3212 §4, epic #3212 Phase 3, #3414) |
|
||||
| `normalize-test-command.cjs` | Normalizes a resolved test command to a one-shot form so a watch-mode runner (vitest/jest) cannot hang a verification gate (#1857); shared by all three live test-command gates (regression, post-merge, audit-fix) |
|
||||
|
||||
@@ -82,6 +82,8 @@ export default tseslint.config(
|
||||
// lint the src/install-model-override-resolver.cts source, not this.
|
||||
'gsd-core/bin/lib/install-model-override-resolver.cjs',
|
||||
'gsd-core/bin/lib/install-engine.cjs',
|
||||
// #3712: tsc-generated runtime artifact — lint src/test-home-guard.cts, not this.
|
||||
'gsd-core/bin/lib/test-home-guard.cjs',
|
||||
// #2874 (epic #2866 Phase 5): tsc-generated runtime artifact — lint the
|
||||
// src/install-fs-adapter.cts source, not this.
|
||||
'gsd-core/bin/lib/install-fs-adapter.cjs',
|
||||
|
||||
@@ -32,6 +32,7 @@ import retiredArtifactCleanup = require('./retired-artifact-cleanup.cjs');
|
||||
import { posixNormalize } from './shell-command-projection.cjs';
|
||||
import { isPathConfined } from './external-descriptor-trust.cjs';
|
||||
import { ensureCommonJsMarker } from './commonjs-marker.cjs';
|
||||
import testHomeGuard = require('./test-home-guard.cjs');
|
||||
// #2874 (ADR-58 cleanup phase): the injectable fs seam for the
|
||||
// installRuntimeArtifacts call tree. `installFs()` resolves to real
|
||||
// `node:fs` unless a call is wrapped in `withInstallFs(deps.fs, ...)` —
|
||||
@@ -436,7 +437,7 @@ function _tryResolveUserArtifactStagingRoot(configDir: string): string | null {
|
||||
* no skills layout to migrate into (mirrors `migrateLegacyDevPreferencesToSkill`'s
|
||||
* own early return for that case).
|
||||
*/
|
||||
function _resolveDevPreferencesSkillTarget(targetDir: string, runtime?: string, scope: string = 'global'): { skillFile: string; installRoot: string } | null {
|
||||
function _resolveDevPreferencesSkillTarget(targetDir: string, runtime?: string, scope: string = 'global'): { skillFile: string; installRoot: string; hasHomeOverride: boolean } | null {
|
||||
let skillDir: string;
|
||||
// #2911: the actual install root the skill dir resolves under — defaults to
|
||||
// targetDir, but a skills-kind `home` override (e.g. Codex -> $HOME/.agents)
|
||||
@@ -444,6 +445,15 @@ function _resolveDevPreferencesSkillTarget(targetDir: string, runtime?: string,
|
||||
// must confine against installRoot, not targetDir, or it would flag the
|
||||
// legitimate override destination as an escape.
|
||||
let installRoot: string = targetDir;
|
||||
// Reported in Codex review of #3725: `installRoot !== targetDir` was used as the
|
||||
// stand-in for "the skills kind declared a `home` override", and the two are NOT
|
||||
// equivalent — a resolved `home` that happens to EQUAL targetDir (a configDir of
|
||||
// `$HOME/.agents`, which is exactly where codex's override points) makes the
|
||||
// inequality false while the override is very much declared, skipping the guard
|
||||
// and writing SKILL.md into the real home. Report the declaration itself instead
|
||||
// of inferring it from two paths, read off the SAME layout resolution the
|
||||
// destination came from so the guard cannot vouch for a path this does not write.
|
||||
let hasHomeOverride = false;
|
||||
if (runtime) {
|
||||
const layout: any = runtimeArtifactLayout.resolveRuntimeArtifactLayout(runtime, targetDir, scope as any);
|
||||
const skillsKindEntry = layout.kinds.find((k: any) => k.kind === 'skills');
|
||||
@@ -454,19 +464,54 @@ function _resolveDevPreferencesSkillTarget(targetDir: string, runtime?: string,
|
||||
// -> $HOME/.agents) instead of always resolving against targetDir, so a
|
||||
// legacy dev-preferences migration lands in the SAME tree the installer
|
||||
// and surface-apply use. Runtimes with no `home` override are unaffected.
|
||||
hasHomeOverride = skillsKindEntry.home != null;
|
||||
installRoot = skillsKindEntry.home ?? targetDir;
|
||||
skillDir = path.join(runtimeArtifactInstallPlan.assertDestWithinConfigHome(installRoot, skillsKindEntry.destSubpath), stemName);
|
||||
} else {
|
||||
// Legacy fallback for callers that have not yet been updated to pass runtime
|
||||
skillDir = path.join(runtimeArtifactInstallPlan.assertDestWithinConfigHome(targetDir, 'skills'), 'gsd-dev-preferences');
|
||||
}
|
||||
return { skillFile: path.join(skillDir, 'SKILL.md'), installRoot };
|
||||
return { skillFile: path.join(skillDir, 'SKILL.md'), installRoot, hasHomeOverride };
|
||||
}
|
||||
|
||||
function migrateLegacyDevPreferencesToSkill(targetDir: string, saved: Map<string, string>, runtime?: string, scope: string = 'global'): boolean {
|
||||
/**
|
||||
* @param deps - #3712 test seam, mirroring the one on `installRuntimeArtifacts`
|
||||
* and `uninstallRuntimeArtifacts`. This is the SIXTH writer that resolves a
|
||||
* skills-kind `home`, and its guard's trigger condition — "HOME equals the
|
||||
* passwd home" — cannot be reproduced without pointing at the developer's real
|
||||
* home, so it is injected rather than simulated. Production callers pass
|
||||
* nothing and bind real `os`/`process.env`.
|
||||
*/
|
||||
function migrateLegacyDevPreferencesToSkill(
|
||||
targetDir: string,
|
||||
saved: Map<string, string>,
|
||||
runtime?: string,
|
||||
scope: string = 'global',
|
||||
deps: { os?: any; env?: Record<string, string | undefined> } = {},
|
||||
): boolean {
|
||||
if (!saved || !saved.has('dev-preferences.md')) return false;
|
||||
const target = _resolveDevPreferencesSkillTarget(targetDir, runtime, scope);
|
||||
if (!target) return false; // runtime has no skills layout at this scope (e.g. cline local)
|
||||
// #3712 — the SIXTH writer that resolves a skills-kind `home` override.
|
||||
// Exported and directly callable, and `_runLegacyInstallMigrations` runs it
|
||||
// BEFORE installRuntimeArtifacts' own assertion, so a future runtime pairing a
|
||||
// home override with this migration would write to the real home ahead of any
|
||||
// guard. It creates rather than prunes, which is why it was missed.
|
||||
//
|
||||
// Guards the destination ALREADY RESOLVED above, never a second resolution of
|
||||
// its own. An earlier revision re-ran resolveRuntimeArtifactLayout() here —
|
||||
// and without `capabilityRegistry`, so a registry-dependent descriptor could
|
||||
// make the two disagree and leave the guard vouching for a path the migration
|
||||
// does not write. That is the generative-fix-divergence shape; reported in
|
||||
// review of #3725. `target.hasHomeOverride` is that same resolution's own answer
|
||||
// to "did the skills kind declare a `home`?" — not re-derived, and not inferred
|
||||
// from `installRoot !== targetDir`, which is false whenever the override happens
|
||||
// to resolve onto targetDir itself (Codex review of #3725).
|
||||
if (runtime && target.hasHomeOverride) {
|
||||
testHomeGuard.assertTestHomeSandboxed('migrateLegacyDevPreferencesToSkill', runtime, [
|
||||
{ kind: 'skills', home: path.dirname(target.skillFile) },
|
||||
], { os: deps.os, env: deps.env });
|
||||
}
|
||||
const { skillFile, installRoot } = target;
|
||||
const skillDir = path.dirname(skillFile);
|
||||
// Security fix: `existsSync` FOLLOWS symlinks and reports `false` for a
|
||||
@@ -997,7 +1042,7 @@ function installRuntimeArtifacts(
|
||||
resolvedProfile: any,
|
||||
resolveAttribution: ResolveAttribution = () => undefined,
|
||||
capabilityRegistry?: any,
|
||||
deps: { fs?: any } = {},
|
||||
deps: { fs?: any; os?: any; env?: Record<string, string | undefined> } = {},
|
||||
): any {
|
||||
return withInstallFs(deps.fs, (): any => {
|
||||
// A removed descriptor kind is no longer visited by the layout loop, so it
|
||||
@@ -1025,6 +1070,11 @@ function installRuntimeArtifacts(
|
||||
_runLegacyInstallMigrations(runtime, configDir, scope);
|
||||
|
||||
const layout = runtimeArtifactLayout.resolveRuntimeArtifactLayout(runtime, configDir, scope as 'global' | 'local', capabilityRegistry);
|
||||
// #3712: a global `home` override escapes the sandboxed configDir. Refuse to
|
||||
// execute when a test run would land that escape in the developer's real home.
|
||||
testHomeGuard.assertTestHomeSandboxed('installRuntimeArtifacts', runtime, layout?.kinds, {
|
||||
os: deps.os, env: deps.env,
|
||||
});
|
||||
const planResult = runtimeArtifactInstallPlan.createRuntimeArtifactInstallPlan({
|
||||
// `Layout` is structurally identical across the layout/install-plan .cjs
|
||||
// modules but nominally distinct to tsc (untyped .cjs boundary) — bridge it.
|
||||
@@ -1261,6 +1311,13 @@ function installOpencodeFamilySkills(
|
||||
const layout: any = runtimeArtifactLayout.resolveRuntimeArtifactLayout(runtime, targetDir);
|
||||
const skillsKindEntry = layout.kinds.find((k: any) => k.kind === 'skills');
|
||||
if (!skillsKindEntry) return 0;
|
||||
// #3712: combined-family runtimes take installRuntimeArtifacts' early return
|
||||
// BEFORE its guard runs, and this writer honors `skillsKindEntry.home` below and
|
||||
// then prunes that destination. opencode/kilo declare no `home` today, so there
|
||||
// is no live escape — but that makes this a bypass waiting on a descriptor
|
||||
// change rather than a safe omission, so it is guarded at the writer instead.
|
||||
// Scoped to the SKILLS kind alone, for the same reason as the agents writer.
|
||||
testHomeGuard.assertTestHomeSandboxed('installOpencodeFamilySkills', runtime, [skillsKindEntry]);
|
||||
const rawDir = rawCommandsDir;
|
||||
if (!rawDir || !installFs().existsSync(rawDir)) return 0;
|
||||
|
||||
@@ -1438,6 +1495,14 @@ function installAgentsKindStandalone(
|
||||
const layout: any = runtimeArtifactLayout.resolveRuntimeArtifactLayout(runtime, targetDir, scope as 'global' | 'local', capabilityRegistry);
|
||||
const agentsKindEntry = layout.kinds.find((k: any) => k.kind === 'agents');
|
||||
if (!agentsKindEntry) return null;
|
||||
// #3712: this writer selects `agentsKindEntry.home` over targetDir below and then
|
||||
// prunes that destination via _removeGsdEntries, so it is a fifth route into the
|
||||
// developer's real home. No agents kind declares a `home` override today, so like
|
||||
// installOpencodeFamilySkills it is guarded against a descriptor change rather
|
||||
// than a present escape. Scoped to the AGENTS kind alone: passing the whole
|
||||
// layout made codex's unrelated skills-kind override trip a writer that never
|
||||
// touches it, which is a false refusal, not a tighter guard.
|
||||
testHomeGuard.assertTestHomeSandboxed('installAgentsKindStandalone', runtime, [agentsKindEntry]);
|
||||
|
||||
// ADR-1235 §1: same agentCtx shape createRuntimeArtifactInstallPlan builds
|
||||
// for the generic layout-driven loop (runtime-artifact-install-plan.cts) —
|
||||
@@ -1816,7 +1881,12 @@ function installOpencodeFamilyArtifacts(
|
||||
* @param configDir resolved runtime config directory
|
||||
* @param scope
|
||||
*/
|
||||
function uninstallRuntimeArtifacts(runtime: string, configDir: string, scope: string): void {
|
||||
function uninstallRuntimeArtifacts(
|
||||
runtime: string,
|
||||
configDir: string,
|
||||
scope: string,
|
||||
deps: { os?: any; env?: Record<string, string | undefined> } = {},
|
||||
): void {
|
||||
// A retired descriptor kind is absent from the current uninstall plan, just
|
||||
// as it is absent from the install plan. Sweep manifest-proven output from
|
||||
// retired kinds before removing the current layout so a direct uninstall
|
||||
@@ -1830,6 +1900,12 @@ function uninstallRuntimeArtifacts(runtime: string, configDir: string, scope: st
|
||||
const stagedLegacyArtifacts = _runLegacyUninstallCleanup(runtime, configDir, scope);
|
||||
|
||||
const layout: any = runtimeArtifactLayout.resolveRuntimeArtifactLayout(runtime, configDir, scope as any);
|
||||
// #3712: uninstall resolves the SAME `kind.home` override as install and then
|
||||
// prunes it via _removeGsdEntries below, so it is a second escape route into
|
||||
// the developer's real home, not a read-only path. Guard it identically.
|
||||
testHomeGuard.assertTestHomeSandboxed('uninstallRuntimeArtifacts', runtime, layout?.kinds, {
|
||||
os: deps.os, env: deps.env,
|
||||
});
|
||||
const plan: any = runtimeArtifactInstallPlan.createRuntimeArtifactUninstallPlan(layout);
|
||||
const kindsByName = new Map<string, any>(layout.kinds.map((kind: any) => [kind.kind as string, kind]));
|
||||
for (const item of plan.items) {
|
||||
|
||||
@@ -12,7 +12,7 @@
|
||||
* readSurface(runtimeConfigDir)
|
||||
* writeSurface(runtimeConfigDir, surfaceState)
|
||||
* resolveSurface(runtimeConfigDir, manifest, clusterMap?, registry?)
|
||||
* applySurface(runtimeConfigDir, layout, manifest, clusterMap?, registry?)
|
||||
* applySurface(runtimeConfigDir, layout, manifest, clusterMap?, registry?, opts?, deps?)
|
||||
* listSurface(runtimeConfigDir, manifest, clusterMap?, registry?)
|
||||
* pruneSkillDirs(skillsDir, retainedNames, prefix, manifest)
|
||||
*
|
||||
@@ -34,6 +34,17 @@ import path from 'node:path';
|
||||
import { platformWriteSync, posixNormalize } from './shell-command-projection.cjs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import installProfiles = require('./install-profiles.cjs');
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import testHomeGuard = require('./test-home-guard.cjs');
|
||||
|
||||
/**
|
||||
* #3712 test seam, mirroring the `Deps` shape in src/test-home-guard.cts. Declared
|
||||
* here rather than imported because that module uses `export =` on a value.
|
||||
*/
|
||||
type TestHomeGuardDeps = {
|
||||
os?: { homedir(): string; userInfo(): { homedir: string } };
|
||||
env?: Record<string, string | undefined>;
|
||||
};
|
||||
const {
|
||||
readActiveProfile,
|
||||
resolveProfile,
|
||||
@@ -360,10 +371,16 @@ function resolveSurface(runtimeConfigDir: string, manifest: Map<string, string[]
|
||||
* Re-stage the active surface using the resolved layout.
|
||||
* Iterates layout.kinds and syncs each artifact kind to its destination.
|
||||
*/
|
||||
function applySurface(runtimeConfigDir: string, layout: Layout, manifest: Map<string, string[]> | object, clusterMap?: ClusterMap | Record<string, string[]>, registry?: { capabilityClusters?: Record<string, string[]>; profileMembership?: Record<string, { tier: string; profiles: string[] }> }, opts?: ApplySurfaceOptions): { name: string; skills: Set<string>; agents: Set<string> } {
|
||||
function applySurface(runtimeConfigDir: string, layout: Layout, manifest: Map<string, string[]> | object, clusterMap?: ClusterMap | Record<string, string[]>, registry?: { capabilityClusters?: Record<string, string[]>; profileMembership?: Record<string, { tier: string; profiles: string[] }> }, opts?: ApplySurfaceOptions, deps: TestHomeGuardDeps = {}): { name: string; skills: Set<string>; agents: Set<string> } {
|
||||
if (path.resolve(runtimeConfigDir) !== path.resolve(layout.configDir)) {
|
||||
throw new TypeError('applySurface runtimeConfigDir must match layout.configDir');
|
||||
}
|
||||
// #3712: the dest selection below prefers `kind.home` over layout.configDir and
|
||||
// then hands it to the destructive _syncGsdDir, so surface apply is a third
|
||||
// escape route into the developer's real home alongside install/uninstall.
|
||||
testHomeGuard.assertTestHomeSandboxed('applySurface', layout.runtime, layout.kinds, {
|
||||
os: deps.os, env: deps.env,
|
||||
});
|
||||
const skillManifest = normalizeSkillManifest(layout.configDir, manifest);
|
||||
const resolved = resolveSurface(layout.configDir, skillManifest, clusterMap, registry);
|
||||
// Profile toggles must converge retired surfaces too. Once a kind disappears
|
||||
|
||||
439
src/test-home-guard.cts
Normal file
439
src/test-home-guard.cts
Normal file
@@ -0,0 +1,439 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* #3712 — refuse to let an in-process test run reach the developer's REAL home.
|
||||
*
|
||||
* A runtime kind may declare a global `home` override that is resolved from
|
||||
* `os.homedir()` rather than from the caller's `configDir` (today: codex's
|
||||
* skills kind, `home: ".agents"`, ADR-1239 / #2088 — Codex auto-discovers
|
||||
* global skills from `$HOME/.agents/skills`). Sandboxing `configDir`/`targetDir`
|
||||
* therefore does NOT contain such a kind, and no `assertDestWithinConfigHome`
|
||||
* check can see it: that gate confines a destSubpath to whatever root it is
|
||||
* handed, and here the root IS the escaped home.
|
||||
*
|
||||
* The live defect: an in-process caller that forgot to sandbox HOME installed —
|
||||
* and, via `_removeGsdEntries` / `_syncGsdDir`, PRUNED — inside the real
|
||||
* `~/.agents/skills`, deleting every `gsd-*` skill (observed 71 -> 0, a foreign
|
||||
* `cloudflare` skill surviving) while the suite still exited 0. It is silent
|
||||
* because the runtime's own config home is untouched, so the manifest keeps
|
||||
* reporting a healthy install.
|
||||
*
|
||||
* This lives in its own module because SIX writers resolve a kind `home` and then
|
||||
* write or destroy what is under it. Four are reachable today —
|
||||
* `installRuntimeArtifacts` and `uninstallRuntimeArtifacts` (install-engine.cts)
|
||||
* and `applySurface` (surface.cts); guarding only the install path, as the first
|
||||
* cut of this fix did, left the other two as live bypasses. Two more are
|
||||
* descriptor-dependent and guarded against a future descriptor change rather than
|
||||
* a present escape: `installOpencodeFamilySkills`, which sits behind the
|
||||
* combined-family early return and honors `skillsKindEntry.home` (no
|
||||
* combined-family runtime declares one), and `installAgentsKindStandalone`, which
|
||||
* honors `agentsKindEntry.home` and prunes it (no agents kind declares one).
|
||||
* The sixth is `migrateLegacyDevPreferencesToSkill`, which resolves the skills
|
||||
* kind's `home` and writes `SKILL.md` under it. It CREATES rather than prunes,
|
||||
* which is why the first pass of this fix missed it, and it runs from
|
||||
* `_runLegacyInstallMigrations` — i.e. BEFORE `installRuntimeArtifacts`' own
|
||||
* assertion — so it carries its own call.
|
||||
*
|
||||
* DETECTION — three signals, in order:
|
||||
*
|
||||
* 1. `NODE_TEST_CONTEXT` is set by `node --test` and inherited by children, and
|
||||
* is absent from normal installs outside a Node test context, so ordinary
|
||||
* production installs are untouched. Precisely: an install spawned BENEATH a
|
||||
* `node --test` process with an un-sandboxed HOME is refused — that is the
|
||||
* point, and it is also why this is not stated as "never affects installs".
|
||||
* `GSD_TEST_MODE` is NOT usable: several in-process test files — including
|
||||
* the one that caused #3712 — never set it, so gating on it would miss the
|
||||
* exact case this guard exists for.
|
||||
* 2. `os.userInfo().homedir` reads the passwd entry and ignores `$HOME`, while
|
||||
* `os.homedir()` prefers `$HOME`. They disagree exactly when a caller
|
||||
* redirected HOME and agree when one forgot. This is the PRIMARY signal and
|
||||
* the only one used whenever a passwd entry is readable. It is asked about
|
||||
* the DESTINATION, not about HOME state: a destination outside the passwd
|
||||
* home is allowed, and one inside it is refused UNLESS it sits beneath a
|
||||
* HOME that was sandboxed away from the passwd home. That last exemption is
|
||||
* not a softening — on Windows the temp root is inside the user's home, so
|
||||
* without it every sandboxed run is refused (#3725). Both halves are
|
||||
* required; see `derivesFromSandboxedHome`.
|
||||
* 3. `SANDBOX_MARKER` — consulted ONLY when the passwd entry cannot be read.
|
||||
*
|
||||
* FAILS CLOSED, with one named exception. If the passwd entry is unreadable
|
||||
* (some CI images) we cannot establish that HOME is sandboxed, so a home-override
|
||||
* kind is refused rather than allowed — UNLESS the marker below names the home in
|
||||
* effect. That branch is a deliberate weakening, not a closed door: with no passwd
|
||||
* entry there is nothing that could contradict a marker naming the real home, so a
|
||||
* caller that sandboxed to its own real home would be believed. The alternative is
|
||||
* refusing every run on such a host. Everything else in that branch answers NO
|
||||
* when it cannot tell — see `sameDirectory`. The cost of a false refusal is a failed test naming its own fix;
|
||||
* the cost of a false allow is silent, unrecoverable deletion of a developer's
|
||||
* skills.
|
||||
*
|
||||
* The marker carries the sandbox PATH, not a boolean, and is checked only in that
|
||||
* unreadable-passwd branch — deliberately, because a boolean checked first is not
|
||||
* proof of anything. An ambient or stale `=1` inherited from a parent process, or
|
||||
* left set by an earlier in-process test, would have disarmed the guard entirely
|
||||
* even when HOME plainly equalled the passwd home. Requiring the value to EQUAL
|
||||
* the home now in effect means a leftover marker from some other directory cannot
|
||||
* vouch for this call.
|
||||
*/
|
||||
|
||||
import fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
|
||||
/**
|
||||
* Set by tests/helpers.cjs `sandboxHome()` to the sandboxed home PATH. Only
|
||||
* consulted when the passwd entry is unreadable; see the module docblock.
|
||||
*/
|
||||
const SANDBOX_MARKER = 'GSD_TEST_HOME_SANDBOX';
|
||||
|
||||
/**
|
||||
* Are these two paths the same directory on disk?
|
||||
*
|
||||
* Compared by FILESYSTEM IDENTITY — `st_dev` + `st_ino` — not by pathname.
|
||||
* `path.resolve()` normalizes separators and `..` but resolves neither symlinks
|
||||
* nor case, and `realpathSync` returns a canonical *pathname*, which two routes to
|
||||
* one directory can still disagree on (a Linux bind mount is the standard case).
|
||||
* `statSync` follows symlinks and reports the inode itself, so a case-variant
|
||||
* HOME, a symlinked HOME, and a bind-mounted HOME all compare equal.
|
||||
*
|
||||
* Fails CLOSED, and "closed" here means returning FALSE. The sole caller is the
|
||||
* passwd-less marker branch, which READS a true as permission to proceed:
|
||||
*
|
||||
* if (marker && sameDirectory(marker, osMod.homedir())) return; // allows
|
||||
*
|
||||
* So "cannot tell" must answer NO. An earlier revision returned true for the
|
||||
* unknown/unknown case (both stats failing with something other than
|
||||
* ENOENT/ENOTDIR — EACCES, EPERM, EIO) on the reasoning that "cannot tell" should
|
||||
* make the caller refuse; that reasoning was inverted with respect to this
|
||||
* caller, and it turned an unreadable-stat environment into an unconditional
|
||||
* bypass for anyone who set the marker to any value at all. Reported in review of
|
||||
* #3725. Only two things now answer yes: the same resolved pathname, or two
|
||||
* readable identities that match.
|
||||
*/
|
||||
function sameDirectory(a: string, b: string): boolean {
|
||||
if (path.resolve(a) === path.resolve(b)) return true;
|
||||
const ia = identify(a);
|
||||
const ib = identify(b);
|
||||
if (ia.kind === 'ok' && ib.kind === 'ok') return ia.dev === ib.dev && ia.ino === ib.ino;
|
||||
return false;
|
||||
}
|
||||
|
||||
type Identity =
|
||||
| { kind: 'ok'; dev: number; ino: number }
|
||||
| { kind: 'absent' }
|
||||
| { kind: 'unknown' };
|
||||
|
||||
function identify(p: string): Identity {
|
||||
try {
|
||||
const st = fs.statSync(p);
|
||||
return { kind: 'ok', dev: st.dev, ino: st.ino };
|
||||
} catch (err) {
|
||||
const code = (err as NodeJS.ErrnoException | undefined)?.code;
|
||||
if (code === 'ENOENT' || code === 'ENOTDIR') return { kind: 'absent' };
|
||||
return { kind: 'unknown' };
|
||||
}
|
||||
}
|
||||
|
||||
type Kind = { kind?: string; home?: string; destSubpath?: string };
|
||||
/** The slice of `node:os` this guard needs — injected so the trigger condition is testable. */
|
||||
type OsLike = { homedir(): string; userInfo(): { homedir: string } };
|
||||
type Deps = { os?: OsLike; env?: Record<string, string | undefined> };
|
||||
|
||||
/**
|
||||
* @param operation - the writer being guarded, for the error message
|
||||
* (e.g. `installRuntimeArtifacts`), so a failure names its own call site.
|
||||
* @param runtime - canonical runtime id, for the error message.
|
||||
* @param kinds - resolved layout kinds; only those carrying `home` can escape.
|
||||
* @param deps - test seam. The guard's own trigger condition is "HOME equals the
|
||||
* passwd home", which cannot be reproduced without pointing at the developer's
|
||||
* real home, so it is injected rather than simulated. Mirrors the `deps.os`
|
||||
* seam in scripts/live-config-guard.cjs.
|
||||
* @throws {Error} when a global `home` override cannot be shown to be sandboxed.
|
||||
*/
|
||||
/**
|
||||
* Stamped on every Error this module throws.
|
||||
*
|
||||
* A refusal happens before any LAYOUT-DRIVEN write — legacy install migrations
|
||||
* run first and are rolled back on their own path — so it is not a partial
|
||||
* codex-skills install and must not trigger that rollback. `bin/install.js`'s
|
||||
* pre-config rollback
|
||||
* deletes and recreates every snapshotted `gsd-*` directory in the resolved
|
||||
* skills root, which for an un-sandboxed codex install IS the real
|
||||
* `~/.agents/skills`. Without this marker the guard's own refusal would provoke
|
||||
* the mutation it exists to prevent. Found by review, not by CI.
|
||||
*/
|
||||
const REFUSAL_FLAG = 'gsdTestHomeGuardRefusal';
|
||||
|
||||
/**
|
||||
* Static remediation line, shared by every refusal this module raises. Hoisted to
|
||||
* module scope because `resolveThroughLinks` refuses too and sits outside
|
||||
* `assertTestHomeSandboxed`'s body, where this used to be a local.
|
||||
*/
|
||||
const SANDBOX_FIX_HINT =
|
||||
`Fix the TEST, not this guard: sandbox HOME and USERPROFILE BEFORE resolving the ` +
|
||||
`layout, for the duration of the call — use sandboxHome(t, dir) from tests/helpers.cjs ` +
|
||||
`(see #3712).`;
|
||||
|
||||
function refusal(message: string): Error {
|
||||
const err = new Error(message);
|
||||
(err as Error & Record<string, unknown>)[REFUSAL_FLAG] = true;
|
||||
return err;
|
||||
}
|
||||
|
||||
/** Did `err` come from this guard refusing before any write happened? */
|
||||
function isTestHomeGuardRefusal(err: unknown): boolean {
|
||||
return Boolean(err && typeof err === 'object' && (err as Record<string, unknown>)[REFUSAL_FLAG]);
|
||||
}
|
||||
|
||||
function assertTestHomeSandboxed(
|
||||
operation: string,
|
||||
runtime: string,
|
||||
kinds: Kind[] | undefined,
|
||||
deps: Deps = {},
|
||||
): void {
|
||||
const osMod = deps.os ?? os;
|
||||
const env = deps.env ?? process.env;
|
||||
|
||||
if (!env['NODE_TEST_CONTEXT']) return; // a real install
|
||||
|
||||
const overriding = (kinds ?? []).filter((k) => k && k.home);
|
||||
if (overriding.length === 0) return; // nothing can escape
|
||||
|
||||
let passwdHome: string | null = null;
|
||||
try {
|
||||
passwdHome = osMod.userInfo().homedir || null;
|
||||
} catch {
|
||||
passwdHome = null;
|
||||
}
|
||||
|
||||
const fix = SANDBOX_FIX_HINT;
|
||||
|
||||
const realHome = passwdHome === null ? { kind: 'unknown' as const } : identify(passwdHome);
|
||||
if (realHome.kind === 'ok') {
|
||||
let effectiveHome: string | null = null;
|
||||
try {
|
||||
effectiveHome = osMod.homedir() || null;
|
||||
} catch {
|
||||
effectiveHome = null;
|
||||
}
|
||||
|
||||
// The question is NOT "is HOME sandboxed right now" — it is "does this
|
||||
// destination land in the real home, other than by deriving from a sandbox
|
||||
// beneath it". The first half alone is not enough: a layout resolved BEFORE
|
||||
// sandboxHome() captures the real `~/.agents` in `kind.home`, and applySurface
|
||||
// takes an already-resolved layout, so a HOME-state check returns "sandboxed"
|
||||
// while the stale destination still points at the real home.
|
||||
//
|
||||
// The second half is not optional either — see `derivesFromSandboxedHome`.
|
||||
// Containment in the real home is not by itself evidence of danger on a
|
||||
// platform whose temp root lives inside the home.
|
||||
for (const kind of overriding) {
|
||||
const declared = path.resolve(path.join(kind.home as string, kind.destSubpath ?? ''));
|
||||
// Both questions are asked of the path the write will REACH, not the one
|
||||
// it was spelled as; see resolveThroughLinks.
|
||||
const dest = resolveThroughLinks(declared);
|
||||
if (!isInside(dest, realHome)) continue;
|
||||
if (derivesFromSandboxedHome(dest, effectiveHome, realHome, passwdHome as string)) continue;
|
||||
throw refusal(
|
||||
`${operation}("${runtime}") was called under a test runner with a destination inside ` +
|
||||
`your REAL home. The "${kind.kind}" kind declares a global home override, so it ` +
|
||||
`resolves from os.homedir() and NOT from the sandboxed configDir — this call would ` +
|
||||
`write inside ${dest} (and install, uninstall and surface-apply also PRUNE GSD ` +
|
||||
`entries there).\nThe real home it was ` +
|
||||
`compared against is ${passwdHome} — if that is not your home, this ` +
|
||||
`refusal is the bug and not the call.\n${fix}`,
|
||||
);
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
// The real home cannot be identified, so containment against it cannot be
|
||||
// evaluated at all. Fall back to the weaker signal: a marker naming the home
|
||||
// currently in effect. Only reachable on hosts with no readable passwd entry,
|
||||
// where the alternative is refusing every such run.
|
||||
//
|
||||
// TWO things are required, not one. An earlier revision returned as soon as the
|
||||
// marker matched the effective HOME, which attested that a caller sandboxed HOME
|
||||
// but said NOTHING about where these destinations resolve — so a layout captured
|
||||
// BEFORE sandboxHome(), still naming the real `~/.agents`, was waved straight
|
||||
// through on a passwd-less host. That is the same stale-layout shape the primary
|
||||
// branch above refuses by design, and it made the marker a bypass for exactly
|
||||
// the case the guard exists for. Reported in Codex review of #3725.
|
||||
//
|
||||
// The marker must also IDENTIFY: `sameDirectory` answers yes for two identical
|
||||
// unidentifiable pathnames (see its docblock), and that is not enough to place a
|
||||
// destination against.
|
||||
const marker = env[SANDBOX_MARKER];
|
||||
const markerId = marker ? identify(marker) : { kind: 'unknown' as const };
|
||||
if (marker && markerId.kind === 'ok' && sameDirectory(marker, osMod.homedir())) {
|
||||
const stale = overriding.find(
|
||||
(kind) => !isInside(resolveThroughLinks(path.resolve(path.join(kind.home as string, kind.destSubpath ?? ''))), markerId),
|
||||
);
|
||||
if (!stale) return;
|
||||
throw refusal(
|
||||
`${operation}("${runtime}") was called under a test runner on a host with no identifiable ` +
|
||||
`passwd home. HOME was sandboxed and recorded, but the "${stale.kind}" kind resolves to ` +
|
||||
`${resolveThroughLinks(path.resolve(path.join(stale.home as string, stale.destSubpath ?? '')))}, ` +
|
||||
`which is NOT beneath that sandbox — so this layout was resolved before the sandbox and still ` +
|
||||
`names another home. A recorded sandbox vouches for HOME, never for a destination that does ` +
|
||||
`not derive from it.\n${fix}`,
|
||||
);
|
||||
}
|
||||
throw refusal(
|
||||
`${operation}("${runtime}") was called under a test runner and this environment has no ` +
|
||||
`identifiable passwd home, so GSD cannot establish where the real home is. The ` +
|
||||
`"${overriding[0]?.kind}" kind declares a global home override, which resolves from ` +
|
||||
`os.homedir() and would write inside whatever real home that is (and, for the pruning ` +
|
||||
`writers, delete GSD entries there). Refusing rather than guessing.\n${fix}`,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* The path `dest` will actually resolve to when it is written.
|
||||
*
|
||||
* `dest` usually does not exist yet — that is the point, it is about to be
|
||||
* created — so `realpathSync` cannot be called on it directly. Walk up to the
|
||||
* nearest ancestor that DOES exist, canonicalize that, and re-append the tail.
|
||||
*
|
||||
* Without this the sandbox exemption below is escapable: with HOME sandboxed to
|
||||
* `$REAL/tmp-home` and `$REAL/tmp-home/.agents` a symlink (or Windows junction)
|
||||
* onto the real `$REAL/.agents`, the lexical ancestor
|
||||
* walk reaches the sandbox and the write is allowed — straight into the real
|
||||
* home. Identity comparison alone does not close it, because each ancestor is
|
||||
* identified in isolation and the alias sits BETWEEN dest and the sandbox.
|
||||
* Found by review, not by CI.
|
||||
*
|
||||
* KNOWN LIMITATION — a subordinate BIND MOUNT of the real `~/.agents` at
|
||||
* `<sandbox>/.agents` is NOT closed by this. A bind mount is not a link:
|
||||
* `realpathSync` keeps the mount-point spelling, so the canonical destination
|
||||
* still reads as "beneath the sandbox" while the write reaches the real
|
||||
* directory. `sameDirectory` above already records that realpath cannot unify
|
||||
* bind-mounted spellings; this inherits that limit. Closing it needs mount-table
|
||||
* introspection, which is not portable, and the setup — deliberately mounting the
|
||||
* one directory this guard protects into a sandbox inside the home — is not a
|
||||
* shape any caller here produces. A cross-process swap of a checked directory
|
||||
* between this call and the write (TOCTOU) is likewise out of reach and not
|
||||
* claimed.
|
||||
*
|
||||
* Canonicalization itself fails CLOSED — see the errno split below. A component
|
||||
* that EXISTS but cannot be resolved is refused, because falling back to the
|
||||
* lexical spelling is the exact ALLOW an unresolvable alias needs.
|
||||
*/
|
||||
function resolveThroughLinks(dest: string): string {
|
||||
const tail: string[] = [];
|
||||
let cur = path.resolve(dest);
|
||||
for (;;) {
|
||||
try {
|
||||
return path.join(fs.realpathSync(cur), ...tail);
|
||||
} catch (err) {
|
||||
// Same errno split `identify` already draws, and deliberately the same one:
|
||||
// both functions are asking "does this path exist as named?", so they must
|
||||
// not disagree. ENOENT/ENOTDIR mean NOT THERE — the expected case, since a
|
||||
// fresh install resolves a destination that does not exist yet and realpath
|
||||
// fails on the leaf and on every not-yet-created ancestor. That is what this
|
||||
// walk is FOR, so keep walking.
|
||||
//
|
||||
// Any other errno means the component EXISTS but could not be canonicalized —
|
||||
// EACCES/EPERM on a directory whose mode changed, ELOOP on a symlink cycle,
|
||||
// EIO on a failing mount. Falling through to the lexical spelling there is a
|
||||
// FAIL-OPEN that precisely inverts this function's purpose: an aliased
|
||||
// `<sandbox>/.agents` that cannot be resolved keeps its sandbox spelling,
|
||||
// satisfies the nested-sandbox exemption, and the write is ALLOWED straight
|
||||
// into the real home. The module documents that it fails CLOSED with exactly
|
||||
// ONE named exception (the marker branch); a swallowed canonicalization error
|
||||
// was a second, unnamed one. Refuse instead. (Codex review of #3725.)
|
||||
const code = (err as NodeJS.ErrnoException | null)?.code;
|
||||
if (code !== 'ENOENT' && code !== 'ENOTDIR') {
|
||||
throw refusal(
|
||||
`A component of the destination could not be canonicalized, so this guard cannot ` +
|
||||
`tell whether the write would reach your REAL home.\nFailed on: ${cur} ` +
|
||||
`(${code ?? 'unknown error'})\nSymlinks and junctions are resolved BEFORE the ` +
|
||||
`decision, because an aliased "<sandbox>/.agents" would otherwise read as confined ` +
|
||||
`while pointing at the real one. A component that cannot be resolved is refused ` +
|
||||
`rather than assumed safe.\n${SANDBOX_FIX_HINT}`,
|
||||
);
|
||||
}
|
||||
const parent = path.dirname(cur);
|
||||
if (parent === cur) return path.resolve(dest);
|
||||
tail.unshift(path.basename(cur));
|
||||
cur = parent;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Is `dest` inside a HOME that has been sandboxed away from the passwd home?
|
||||
*
|
||||
* Asked only once `dest` is already known to be inside the real home, and it is
|
||||
* what makes that fact non-fatal on Windows: there `os.tmpdir()` is
|
||||
* `%USERPROFILE%\AppData\Local\Temp` by default (Node honors `TEMP`/`TMP`,
|
||||
* so this is the usual case rather than a guarantee) — so in practice every
|
||||
* sandbox a test creates is a descendant of the real home. Containment in the
|
||||
* real home is therefore true of the correctly-sandboxed case and the dangerous
|
||||
* one alike, and cannot separate them by itself. POSIX conceals this, because
|
||||
* `/tmp` and `/var/folders` both sit outside `$HOME`. All six Windows shards of
|
||||
* #3725 failed on legitimately sandboxed destinations before this conjunct
|
||||
* existed.
|
||||
*
|
||||
* TWO conditions carry the decision, and neither suffices alone:
|
||||
*
|
||||
* - the passwd home is NOT beneath that HOME — see the comment on that check;
|
||||
* without it a HOME that merely spells the real home more widely (`/Users`,
|
||||
* `C:\Users`) is mistaken for a sandbox.
|
||||
* - `dest` is beneath that sandboxed HOME — otherwise this decays into the
|
||||
* "is HOME sandboxed?" check the module docblock rejects, and a layout
|
||||
* resolved before the sandbox walks straight through.
|
||||
*
|
||||
* The first check below — HOME differs from the passwd home — is a cheap fast
|
||||
* path, NOT an independent condition: `isInside` is reflexive, so whenever HOME
|
||||
* identifies the passwd home the second check already returns false on its own.
|
||||
* Removing it changes no outcome; it is kept to answer the common case without
|
||||
* an ancestor walk. Raised in review of #3725 against prose that claimed the
|
||||
* three were independent.
|
||||
*
|
||||
* Fails CLOSED: an unreadable or unidentifiable HOME returns false, so the
|
||||
* caller refuses rather than exempting a destination it cannot place.
|
||||
*/
|
||||
function derivesFromSandboxedHome(
|
||||
dest: string,
|
||||
effectiveHome: string | null,
|
||||
realHome: { dev: number; ino: number },
|
||||
passwdHome: string,
|
||||
): boolean {
|
||||
if (effectiveHome === null) return false;
|
||||
const eff = identify(effectiveHome);
|
||||
if (eff.kind !== 'ok') return false;
|
||||
if (eff.dev === realHome.dev && eff.ino === realHome.ino) return false;
|
||||
// A sandbox sits BENEATH the real home (the Windows temp shape) — never above
|
||||
// it. A destination's ancestor chain is linear, so "inside the real home AND
|
||||
// inside the effective HOME" admits two arrangements, not one: the intended
|
||||
// `effectiveHome ⊂ realHome`, and `realHome ⊂ effectiveHome` — HOME pointed at
|
||||
// `/Users`, `/home`, or `C:\Users`. The second is not a sandbox, it is the real
|
||||
// home reached by a wider spelling, and without this it exempts a stale
|
||||
// destination. Reported in review of #3725 as the one leaking cell of a
|
||||
// six-row truth table.
|
||||
if (isInside(passwdHome, eff)) return false;
|
||||
return isInside(dest, eff);
|
||||
}
|
||||
|
||||
/**
|
||||
* Is `child` at or beneath the directory identified by `rootId`?
|
||||
*
|
||||
* Walks `child`'s ancestors comparing filesystem identity rather than string
|
||||
* prefixes, because `child` typically does not exist yet (that is the point — it
|
||||
* is about to be created) while its ancestors do. A prefix test would miss a
|
||||
* case-variant or symlinked spelling of the same ancestor, which is the exact
|
||||
* class this guard exists to catch.
|
||||
*/
|
||||
function isInside(child: string, rootId: { dev: number; ino: number }): boolean {
|
||||
let cur = path.resolve(child);
|
||||
for (;;) {
|
||||
const id = identify(cur);
|
||||
if (id.kind === 'ok' && id.dev === rootId.dev && id.ino === rootId.ino) return true;
|
||||
const parent = path.dirname(cur);
|
||||
if (parent === cur) return false;
|
||||
cur = parent;
|
||||
}
|
||||
}
|
||||
|
||||
export = { assertTestHomeSandboxed, isTestHomeGuardRefusal, SANDBOX_MARKER };
|
||||
@@ -92,7 +92,7 @@ const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const os = require('node:os');
|
||||
const path = require('node:path');
|
||||
const { cleanup } = require('./helpers.cjs');
|
||||
const { cleanup, createTempDir, sandboxHome } = require('./helpers.cjs');
|
||||
|
||||
const REPO_ROOT = path.join(__dirname, '..');
|
||||
const LIB_DIR = path.join(REPO_ROOT, 'gsd-core', 'bin', 'lib');
|
||||
@@ -629,6 +629,16 @@ test('agent-descriptor-parity: K1 — every registry runtime declaring an agents
|
||||
assert.ok(runtimesWithAgentsKind.includes('kimi-code'), 'kimi-code (found via golden fixture, not analysis) must be covered here');
|
||||
assert.ok(runtimesWithAgentsKind.includes('cline'), 'cline must be covered here');
|
||||
|
||||
// #3712: this loop reaches `codex`, whose global skills kind declares a `home`
|
||||
// override (`.agents`) resolved from os.homedir() rather than from targetDir.
|
||||
// Sandboxing targetDir alone does NOT contain it — before this line the call
|
||||
// below pruned every gsd-* skill from the developer's REAL ~/.agents/skills
|
||||
// (71 -> 0) while the suite still exited 0. Sandbox HOME for the whole loop so
|
||||
// codex's skills land in <homeDir>/.agents/skills inside a temp dir we clean.
|
||||
const homeDir = createTempDir('gsd-adp-home-');
|
||||
t.after(() => cleanup(homeDir));
|
||||
sandboxHome(t, homeDir);
|
||||
|
||||
for (const runtime of runtimesWithAgentsKind) {
|
||||
const { commandsGsd, root } = buildSourceTree(SAMPLE_AGENTS);
|
||||
const targetDir = buildTargetDir(commandsGsd);
|
||||
|
||||
@@ -54,22 +54,14 @@ const TEST_ATTRIBUTION = () => 'Co-Authored-By: Test <t@example.com>';
|
||||
|
||||
/**
|
||||
* Sandbox HOME/USERPROFILE for the duration of a test. Some runtimes (e.g.
|
||||
* codex) resolve a kind's `home` via os.homedir(); without this, an
|
||||
* in-process install would write into the developer's real home directory.
|
||||
* Mirrors tests/install-runtime-artifacts.test.cjs's sandboxHome().
|
||||
* codex) resolve a kind's `home` via os.homedir(); without this, an in-process
|
||||
* install would write into the developer's real home directory.
|
||||
*
|
||||
* #3712: promoted to tests/helpers.cjs, from the byte-identical copy that used
|
||||
* to live here. It now also sets the sandbox marker src/test-home-guard.cts
|
||||
* needs to stay permissive on hosts with no readable passwd entry.
|
||||
*/
|
||||
function sandboxHome(t, dir) {
|
||||
const savedHome = process.env.HOME;
|
||||
const savedUserProfile = process.env.USERPROFILE;
|
||||
process.env.HOME = dir;
|
||||
process.env.USERPROFILE = dir;
|
||||
t.after(() => {
|
||||
if (savedHome === undefined) delete process.env.HOME;
|
||||
else process.env.HOME = savedHome;
|
||||
if (savedUserProfile === undefined) delete process.env.USERPROFILE;
|
||||
else process.env.USERPROFILE = savedUserProfile;
|
||||
});
|
||||
}
|
||||
const { sandboxHome } = require('./helpers.cjs');
|
||||
|
||||
// ─── E3 — the opencode-family early return (matrix row E3) ──────────────────
|
||||
|
||||
|
||||
@@ -363,6 +363,20 @@ describe('#3156: a raw installer spawn cannot write into the ambient HOME', () =
|
||||
'USERPROFILE must track HOME — os.homedir() reads it on Windows');
|
||||
assert.strictEqual(build({ HOME: '/explicit', USERPROFILE: '/explicit' }).HOME, '/explicit',
|
||||
'an explicit HOME override must still win (overrides spread last)');
|
||||
// #3712 / Codex review of #3725: the marker attests to the home ACTUALLY in
|
||||
// effect. Spreading `overrides` last used to leave it naming this helper's
|
||||
// default home while HOME was the caller's, and on a passwd-less host the
|
||||
// test-home guard compares the two and refuses a legitimately sandboxed
|
||||
// spawn. Asserting HOME alone did not catch it — the marker has to move too.
|
||||
assert.strictEqual(build().GSD_TEST_HOME_SANDBOX, build().HOME,
|
||||
'the marker must name the default sandbox home');
|
||||
const overridden = build({ HOME: '/explicit', USERPROFILE: '/explicit' });
|
||||
assert.strictEqual(overridden.GSD_TEST_HOME_SANDBOX, '/explicit',
|
||||
'the marker must follow an overridden HOME, not keep naming the default one');
|
||||
assert.strictEqual(
|
||||
build({ HOME: '/explicit', GSD_TEST_HOME_SANDBOX: '/caller-chosen' }).GSD_TEST_HOME_SANDBOX,
|
||||
'/caller-chosen',
|
||||
'an explicitly supplied marker still wins over the derived one');
|
||||
}
|
||||
});
|
||||
|
||||
@@ -410,3 +424,73 @@ describe('#3156: a raw installer spawn cannot write into the ambient HOME', () =
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('#3712: a raw installer spawn cannot reach the ambient HOME\'s shared skills root', () => {
|
||||
const fs = require('node:fs');
|
||||
const os = require('node:os');
|
||||
const { execFileSync } = require('node:child_process');
|
||||
const { cleanup } = require('./helpers.cjs');
|
||||
const { installerEnv } = require('./helpers/install-shared.cjs');
|
||||
const INSTALL_PATH = path.join(__dirname, '..', 'bin', 'install.js');
|
||||
|
||||
// #3156's canary asserts on <canaryHome>/.gsd. That is not the only ambient-HOME
|
||||
// surface: a kind may declare a global `home` override resolved from os.homedir()
|
||||
// — codex's skills kind (`.agents`) — and the installer PRUNES gsd-* entries under
|
||||
// it. A 71-skill deletion there passed the .gsd-only canary unnoticed (#3712).
|
||||
//
|
||||
// The runtime choice is load-bearing, not incidental. The .gsd row spawns
|
||||
// `--cursor --local`, and cursor declares no home override, so an `.agents`
|
||||
// assertion on THAT spawn passes even with all confinement removed. Only a
|
||||
// runtime that actually carries the override can discriminate — today that is
|
||||
// codex at global scope, asserted below so a descriptor change fails here loudly.
|
||||
test('installing codex globally leaves the ambient HOME\'s .agents/skills sampled inventory unchanged', () => {
|
||||
const { runtimes } = require('../gsd-core/bin/lib/capability-registry.cjs');
|
||||
const codexGlobalSkills = (runtimes?.codex?.runtime?.artifactLayout?.global ?? [])
|
||||
.find((e) => e.kind === 'skills' && e.home);
|
||||
assert.ok(codexGlobalSkills,
|
||||
'codex global skills must still declare a `home` override — without it this row '
|
||||
+ 'is non-discriminating and must be re-pointed at whichever runtime carries one');
|
||||
|
||||
const canaryHome = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3712-canary-home-'));
|
||||
const projectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3712-project-'));
|
||||
const realHome = process.env.HOME;
|
||||
const realUserProfile = process.env.USERPROFILE;
|
||||
|
||||
const skillsRoot = path.join(canaryHome, '.agents', 'skills');
|
||||
fs.mkdirSync(skillsRoot, { recursive: true });
|
||||
for (const name of ['gsd-plan-phase', 'gsd-dev-preferences', 'cloudflare']) {
|
||||
fs.mkdirSync(path.join(skillsRoot, name), { recursive: true });
|
||||
fs.writeFileSync(path.join(skillsRoot, name, 'SKILL.md'), `# ${name}\n`);
|
||||
}
|
||||
// A SAMPLE, not a byte-for-byte tree compare: immediate dir names plus each
|
||||
// dir's SKILL.md. It catches the demonstrated regression (whole gsd-* dirs
|
||||
// pruned) and would miss nested-file, mode, or metadata mutations.
|
||||
const inventory = () => fs.readdirSync(skillsRoot).sort()
|
||||
.map((d) => `${d}:${fs.readFileSync(path.join(skillsRoot, d, 'SKILL.md'), 'utf-8')}`)
|
||||
.join('|');
|
||||
const before = inventory();
|
||||
|
||||
try {
|
||||
// Make the AMBIENT home the canary, exactly as on a developer machine.
|
||||
process.env.HOME = canaryHome;
|
||||
process.env.USERPROFILE = canaryHome;
|
||||
|
||||
execFileSync(process.execPath, [INSTALL_PATH, '--codex', '--global', '--no-sdk'], {
|
||||
cwd: projectDir,
|
||||
encoding: 'utf-8',
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
env: installerEnv(),
|
||||
timeout: 300_000,
|
||||
});
|
||||
|
||||
assert.strictEqual(inventory(), before,
|
||||
'the installer pruned or wrote GSD skills in the ambient HOME instead of its own sandbox');
|
||||
} finally {
|
||||
if (realHome === undefined) delete process.env.HOME; else process.env.HOME = realHome;
|
||||
if (realUserProfile === undefined) delete process.env.USERPROFILE;
|
||||
else process.env.USERPROFILE = realUserProfile;
|
||||
cleanup(canaryHome);
|
||||
cleanup(projectDir);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -997,10 +997,86 @@ function installSpawnHome() {
|
||||
|
||||
function installSpawnEnv(overrides = {}) {
|
||||
const home = installSpawnHome();
|
||||
return { ...process.env, ...testEnvBase(), HOME: home, USERPROFILE: home, ...overrides };
|
||||
// #3712 — carry the sandbox marker too, not just HOME. The guard falls back to
|
||||
// the marker on hosts with no readable passwd entry (some CI images) and
|
||||
// otherwise REFUSES. A spawned installer inherits NODE_TEST_CONTEXT and has a
|
||||
// legitimately redirected HOME, so without this it is refused on exactly the
|
||||
// environment the fallback exists to serve. The name is the same constant
|
||||
// sandboxHome() writes; see the note there on why it is a bare string.
|
||||
const env = {
|
||||
...process.env,
|
||||
...testEnvBase(),
|
||||
HOME: home,
|
||||
USERPROFILE: home,
|
||||
[TEST_HOME_SANDBOX_MARKER]: home,
|
||||
...overrides,
|
||||
};
|
||||
// The marker attests to the home ACTUALLY in effect, so it has to follow an
|
||||
// overridden HOME rather than keep naming this helper's default one. Spreading
|
||||
// `overrides` last is deliberate (an explicit HOME must win — see the docblock's
|
||||
// "A test needing that passes its own { HOME, USERPROFILE }"), but it left the
|
||||
// marker stale: a caller supplying its own HOME got HOME=<theirs> and
|
||||
// marker=<helper default>. On a passwd-less host the guard compares the two and
|
||||
// REFUSES a legitimately sandboxed spawn — tests/install.test.cjs:7143 and
|
||||
// install-shared.cjs's own runInstaller both take that path. An explicitly
|
||||
// supplied marker still wins over both. Reported in Codex review of #3725.
|
||||
if (!(TEST_HOME_SANDBOX_MARKER in overrides)) env[TEST_HOME_SANDBOX_MARKER] = env.HOME;
|
||||
return env;
|
||||
}
|
||||
|
||||
module.exports = { runGsdTools, createTempDir, createTempProject, createTempGitProject, cleanup, tmpRootCandidates, readFileNormalized, readWorkflowCombined, parseFrontmatter, isUsageOutput, captureConsole, toPosixPath, absPlanningPath, runNpm, isolatedNpmEnv, withIsolatedProcessState, delay, waitFor, resetRuntimeWarningCaches, SESSION_ENV_KEYS, saveSessionEnv, restoreSessionEnv, clearSessionEnv, isolateWorkstreamEnv, restoreWorkstreamEnv, TOOLS_PATH, SESSION_IDENTITY_ENV_KEYS, scrubConfigLocationEnv, installSpawnEnv, installSpawnHome };
|
||||
/**
|
||||
* #3712 — sandbox HOME/USERPROFILE for the duration of ONE test.
|
||||
*
|
||||
* The spawn-side helpers above (#3156) cover CHILD processes only. A test that
|
||||
* calls the installer IN-PROCESS gets no protection from them, and a runtime kind
|
||||
* may declare a global `home` override that resolves from `os.homedir()` rather
|
||||
* than from the sandboxed configDir — codex's skills kind (`.agents`, ADR-1239 /
|
||||
* #2088) is the live case. Without this, such a call writes to, and prunes
|
||||
* `gsd-*` entries from, the developer's REAL ~/.agents/skills.
|
||||
*
|
||||
* Promoted here from the identical private copies in executed-plan.test.cjs and
|
||||
* install-runtime-artifacts.test.cjs so new in-process callers have one obvious
|
||||
* helper to reach for instead of re-deriving it (or forgetting it).
|
||||
*
|
||||
* Pass the test's own temp configDir as `dir` where possible: codex's skills dir
|
||||
* then resolves to `<configDir>/.agents/skills`, keeping every artifact the call
|
||||
* writes inside the directory the test already cleans up.
|
||||
*
|
||||
* @param {{ after: (fn: () => void) => void }} t - node:test context.
|
||||
* @param {string} dir - directory to use as HOME for the duration of the test.
|
||||
*/
|
||||
// #3712: the marker NAME is a constant, duplicated here deliberately rather than
|
||||
// required from the compiled guard. helpers.cjs is imported by ~370 test files and
|
||||
// documents (see builtLib above) that it must NOT load gsd-core/bin/lib at module
|
||||
// scope — an unbuilt tree would then fail on import alone, turning a missing
|
||||
// `npm run build:lib` into a whole-suite crash. A lazy require inside sandboxHome
|
||||
// would satisfy that too, but a bare string needs no build at all. The pairing is
|
||||
// pinned by a test so the two cannot drift.
|
||||
const TEST_HOME_SANDBOX_MARKER = 'GSD_TEST_HOME_SANDBOX';
|
||||
|
||||
function sandboxHome(t, dir) {
|
||||
const savedHome = process.env.HOME;
|
||||
const savedUserProfile = process.env.USERPROFILE;
|
||||
const savedMarker = process.env[TEST_HOME_SANDBOX_MARKER];
|
||||
process.env.HOME = dir;
|
||||
process.env.USERPROFILE = dir;
|
||||
// Records WHICH directory this call sandboxed to. src/test-home-guard.cts fails
|
||||
// CLOSED when it cannot read a passwd entry to compare HOME against (some CI
|
||||
// images), and consults this only in that branch, accepting it only when it
|
||||
// names the home actually in effect — so a stale marker cannot vouch for a
|
||||
// later, un-sandboxed call.
|
||||
process.env[TEST_HOME_SANDBOX_MARKER] = dir;
|
||||
t.after(() => {
|
||||
if (savedHome === undefined) delete process.env.HOME;
|
||||
else process.env.HOME = savedHome;
|
||||
if (savedUserProfile === undefined) delete process.env.USERPROFILE;
|
||||
else process.env.USERPROFILE = savedUserProfile;
|
||||
if (savedMarker === undefined) delete process.env[TEST_HOME_SANDBOX_MARKER];
|
||||
else process.env[TEST_HOME_SANDBOX_MARKER] = savedMarker;
|
||||
});
|
||||
}
|
||||
|
||||
module.exports = { runGsdTools, createTempDir, createTempProject, createTempGitProject, cleanup, tmpRootCandidates, readFileNormalized, readWorkflowCombined, parseFrontmatter, isUsageOutput, captureConsole, toPosixPath, absPlanningPath, runNpm, isolatedNpmEnv, withIsolatedProcessState, delay, waitFor, resetRuntimeWarningCaches, SESSION_ENV_KEYS, saveSessionEnv, restoreSessionEnv, clearSessionEnv, isolateWorkstreamEnv, restoreWorkstreamEnv, TOOLS_PATH, SESSION_IDENTITY_ENV_KEYS, scrubConfigLocationEnv, installSpawnEnv, installSpawnHome, sandboxHome, TEST_HOME_SANDBOX_MARKER };
|
||||
|
||||
// Lazy, for the reason builtLib() is lazy: reading either of these is what
|
||||
// forces the built-lib require, so a test file that needs neither can still
|
||||
|
||||
@@ -218,18 +218,11 @@ function readAllSkillMd(dir) {
|
||||
// write to (and an uninstall would mutate) the developer's REAL ~/.agents/skills.
|
||||
// Sandbox HOME/USERPROFILE to configDir before resolving the layout or invoking
|
||||
// install/uninstall so codex's resolved skills dir is configDir/.agents/skills.
|
||||
function sandboxHome(t, dir) {
|
||||
const savedHome = process.env.HOME;
|
||||
const savedUserProfile = process.env.USERPROFILE;
|
||||
process.env.HOME = dir;
|
||||
process.env.USERPROFILE = dir;
|
||||
t.after(() => {
|
||||
if (savedHome === undefined) delete process.env.HOME;
|
||||
else process.env.HOME = savedHome;
|
||||
if (savedUserProfile === undefined) delete process.env.USERPROFILE;
|
||||
else process.env.USERPROFILE = savedUserProfile;
|
||||
});
|
||||
}
|
||||
//
|
||||
// #3712: promoted to tests/helpers.cjs, from the byte-identical copy that used to
|
||||
// live here. It now also sets the sandbox marker src/test-home-guard.cts needs to
|
||||
// stay permissive on hosts with no readable passwd entry.
|
||||
const { sandboxHome } = require('./helpers.cjs');
|
||||
|
||||
describe('installRuntimeArtifacts — skills runtimes write gsd-prefixed skill dirs', () => {
|
||||
for (const runtime of SKILLS_RUNTIMES_LAYOUT) {
|
||||
@@ -5093,13 +5086,20 @@ describe('Bug #2911: migrateLegacyDevPreferencesToSkill honors the skills-kind h
|
||||
function withFakeHome(fakeHome, fn) {
|
||||
const savedHome = process.env.HOME;
|
||||
const savedUserProfile = process.env.USERPROFILE;
|
||||
// #3712: record WHICH home this sandboxed to. src/test-home-guard.cts fails
|
||||
// closed on hosts with no readable passwd entry, and this is what proves a
|
||||
// genuinely-sandboxed caller there. Without it these calls would be refused.
|
||||
const savedMarker = process.env.GSD_TEST_HOME_SANDBOX;
|
||||
process.env.HOME = fakeHome;
|
||||
process.env.USERPROFILE = fakeHome;
|
||||
process.env.GSD_TEST_HOME_SANDBOX = fakeHome;
|
||||
try {
|
||||
return fn();
|
||||
} finally {
|
||||
if (savedHome === undefined) delete process.env.HOME; else process.env.HOME = savedHome;
|
||||
if (savedUserProfile === undefined) delete process.env.USERPROFILE; else process.env.USERPROFILE = savedUserProfile;
|
||||
if (savedMarker === undefined) delete process.env.GSD_TEST_HOME_SANDBOX;
|
||||
else process.env.GSD_TEST_HOME_SANDBOX = savedMarker;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -3182,3 +3182,632 @@ describe('install()/uninstall() degrade (never abort) on a staging-root resoluti
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
/**
|
||||
* #3712 — an in-process run must never reach the developer's REAL home.
|
||||
*
|
||||
* The confinement rows above bound a write to the root they are handed. A kind
|
||||
* may ALSO declare a global `home` override resolved from `os.homedir()` rather
|
||||
* than from configDir (today: codex's skills kind, `home: ".agents"`, ADR-1239 /
|
||||
* #2088). assertDestWithinConfigHome cannot see that class: it confines a
|
||||
* destSubpath to whatever root it is given, and here the root IS the escaped home.
|
||||
*
|
||||
* The live defect: an in-process caller that forgot to sandbox HOME pruned the
|
||||
* real ~/.agents/skills — 71 gsd-* dirs deleted, a foreign `cloudflare` skill
|
||||
* surviving, suite still exit 0, because the runtime's own config home was
|
||||
* untouched and the manifest kept reporting a healthy install.
|
||||
*
|
||||
* SIX writers resolve a kind `home` and then write or destroy under it. The four
|
||||
* reachable today are covered below by driving the REAL entrypoint — not the
|
||||
* predicate — so that deleting a guard call site turns these rows red. The other
|
||||
* two, `installOpencodeFamilySkills` and `installAgentsKindStandalone`, are
|
||||
* guarded but not wiring-tested: no runtime declares a `home` override on those
|
||||
* kinds, so neither path can be exercised without inventing a descriptor.
|
||||
*
|
||||
* The sixth, `migrateLegacyDevPreferencesToSkill`, CREATES rather than prunes and
|
||||
* runs from `_runLegacyInstallMigrations` — i.e. BEFORE installRuntimeArtifacts'
|
||||
* own assertion — so it carries its own guard call and its own wiring row. The
|
||||
* count read FIVE/three until review of #3725 caught the row missing.
|
||||
*/
|
||||
describe('#3712 in-process home confinement', () => {
|
||||
const { createTempDir, sandboxHome } = require('./helpers.cjs');
|
||||
const installEngine = require('../gsd-core/bin/lib/install-engine.cjs');
|
||||
const surface = require('../gsd-core/bin/lib/surface.cjs');
|
||||
const runtimeArtifactLayout = require('../gsd-core/bin/lib/runtime-artifact-layout.cjs');
|
||||
const testHomeGuard = require('../gsd-core/bin/lib/test-home-guard.cjs');
|
||||
|
||||
const escapingKinds = (home) => [{ kind: 'skills', home, destSubpath: 'skills' }];
|
||||
const confinedKinds = [{ kind: 'skills', destSubpath: 'skills' }];
|
||||
const fakeOs = (homedir, passwdHome) => ({
|
||||
homedir: () => homedir,
|
||||
userInfo: () => ({ homedir: passwdHome }),
|
||||
});
|
||||
const underTest = { NODE_TEST_CONTEXT: 'child-v8' };
|
||||
|
||||
// ── the predicate ────────────────────────────────────────────────────────
|
||||
// Driven with REAL directories: the whole question is filesystem identity, and
|
||||
// synthetic paths cannot exercise it. The passwd home is injected via the deps
|
||||
// seam so no row depends on the developer's actual home.
|
||||
|
||||
describe('predicate', () => {
|
||||
test('refuses a destination that lands inside the real home', (t) => {
|
||||
const realHome = createTempDir('gsd-3712-real-');
|
||||
t.after(() => cleanup(realHome));
|
||||
assert.throws(
|
||||
() => testHomeGuard.assertTestHomeSandboxed('installRuntimeArtifacts', 'codex',
|
||||
escapingKinds(path.join(realHome, '.agents')),
|
||||
{ os: fakeOs(realHome, realHome), env: underTest }),
|
||||
/destination inside\s+your REAL home/,
|
||||
);
|
||||
});
|
||||
|
||||
test('allows a destination outside the real home — the correct-usage path', (t) => {
|
||||
const realHome = createTempDir('gsd-3712-real2-');
|
||||
const sandbox = createTempDir('gsd-3712-sandbox-');
|
||||
t.after(() => { cleanup(realHome); cleanup(sandbox); });
|
||||
assert.doesNotThrow(() => testHomeGuard.assertTestHomeSandboxed(
|
||||
'installRuntimeArtifacts', 'codex',
|
||||
escapingKinds(path.join(sandbox, '.agents')),
|
||||
{ os: fakeOs(sandbox, realHome), env: underTest },
|
||||
));
|
||||
});
|
||||
|
||||
// The round-5 defect. A HOME-state check ("is HOME sandboxed right now?")
|
||||
// returns "sandboxed" here and lets the write through, because the layout was
|
||||
// resolved BEFORE the sandbox and its kind.home still names the real home.
|
||||
// applySurface takes an already-resolved layout, so this is reachable, not
|
||||
// theoretical. Asking about the DESTINATION is what closes it.
|
||||
test('refuses a STALE layout resolved before HOME was sandboxed', (t) => {
|
||||
const realHome = createTempDir('gsd-3712-stale-real-');
|
||||
const sandbox = createTempDir('gsd-3712-stale-sandbox-');
|
||||
t.after(() => { cleanup(realHome); cleanup(sandbox); });
|
||||
// kind.home captured the REAL home; HOME is now the sandbox.
|
||||
assert.throws(
|
||||
() => testHomeGuard.assertTestHomeSandboxed('applySurface', 'codex',
|
||||
escapingKinds(path.join(realHome, '.agents')),
|
||||
{ os: fakeOs(sandbox, realHome), env: underTest }),
|
||||
/destination inside\s+your REAL home/,
|
||||
'a sandboxed HOME does not make a destination resolved before the sandbox safe',
|
||||
);
|
||||
});
|
||||
|
||||
// The round-6 defect, found by CI rather than by review: all six Windows
|
||||
// shards of #3725 failed here, every one of them on a legitimately sandboxed
|
||||
// destination. On Windows `os.tmpdir()` is `%LOCALAPPDATA%\Temp` —
|
||||
// `%USERPROFILE%\AppData\Local\Temp` — so every sandbox a test creates is a
|
||||
// DESCENDANT of the real home, and "does this land inside the real home?"
|
||||
// answers yes for the safe case and the dangerous one alike. POSIX hides this:
|
||||
// /tmp and /var/folders both sit outside $HOME, so the containment question
|
||||
// happens to discriminate there and the flaw is invisible.
|
||||
//
|
||||
// What actually separates the two is whether the destination derives from a
|
||||
// HOME that was sandboxed — hence the added conjunct. This must NOT decay into
|
||||
// the "is HOME sandboxed?" check the row above rejects; the row below holds
|
||||
// that line.
|
||||
test('allows a sandbox nested INSIDE the real home — the Windows temp-root shape', (t) => {
|
||||
const realHome = createTempDir('gsd-3712-nested-home-');
|
||||
t.after(() => cleanup(realHome));
|
||||
const sandbox = path.join(realHome, 'AppData', 'Local', 'Temp', 'gsd-sandbox-a');
|
||||
fs.mkdirSync(sandbox, { recursive: true });
|
||||
assert.doesNotThrow(() => testHomeGuard.assertTestHomeSandboxed(
|
||||
'installRuntimeArtifacts', 'codex',
|
||||
escapingKinds(path.join(sandbox, '.agents')),
|
||||
{ os: fakeOs(sandbox, realHome), env: underTest },
|
||||
));
|
||||
});
|
||||
|
||||
// The guard against fixing the row above by weakening it to a HOME-state
|
||||
// check. Same nested-sandbox environment, but the layout was resolved before
|
||||
// the sandbox existed, so the destination still names the real home's
|
||||
// `.agents`. HOME is sandboxed and the write is still fatal.
|
||||
test('a nested sandbox does NOT excuse a STALE destination in the real home', (t) => {
|
||||
const realHome = createTempDir('gsd-3712-nested-stale-');
|
||||
t.after(() => cleanup(realHome));
|
||||
const sandbox = path.join(realHome, 'AppData', 'Local', 'Temp', 'gsd-sandbox-b');
|
||||
fs.mkdirSync(sandbox, { recursive: true });
|
||||
assert.throws(
|
||||
() => testHomeGuard.assertTestHomeSandboxed('applySurface', 'codex',
|
||||
escapingKinds(path.join(realHome, '.agents')),
|
||||
{ os: fakeOs(sandbox, realHome), env: underTest }),
|
||||
/destination inside\s+your REAL home/,
|
||||
'a sandbox beneath the real home vouches only for what is beneath the sandbox',
|
||||
);
|
||||
});
|
||||
|
||||
// Review N2: the leaking cell of the truth table. A destination's ancestor
|
||||
// chain is linear, so "inside the real home AND inside the effective HOME"
|
||||
// admits `realHome ⊂ effectiveHome` as well as the intended
|
||||
// `effectiveHome ⊂ realHome`. HOME at /Users, /home or C:\Users is not a
|
||||
// sandbox — it is the real home spelled more widely — and without the third
|
||||
// conjunct it exempts a stale destination pointing straight at ~/.agents.
|
||||
test('a HOME that is an ANCESTOR of the real home is not a sandbox', (t) => {
|
||||
const container = createTempDir('gsd-3712-ancestor-');
|
||||
t.after(() => cleanup(container));
|
||||
const realHome = path.join(container, 'someone');
|
||||
fs.mkdirSync(path.join(realHome, '.agents'), { recursive: true });
|
||||
assert.throws(
|
||||
() => testHomeGuard.assertTestHomeSandboxed('applySurface', 'codex',
|
||||
escapingKinds(path.join(realHome, '.agents')),
|
||||
// HOME is the DIRECTORY THAT CONTAINS the real home, so it differs from
|
||||
// the passwd home and still contains the destination.
|
||||
{ os: fakeOs(container, realHome), env: underTest }),
|
||||
/destination inside\s+your REAL home/,
|
||||
'a HOME above the real home spells it more widely; it does not sandbox it',
|
||||
);
|
||||
});
|
||||
|
||||
// The escape the nested-sandbox exemption opens if it trusts the SPELLING of
|
||||
// a path instead of where it resolves. HOME is sandboxed to a directory
|
||||
// inside the real home (legitimate on Windows), but the sandbox's `.agents`
|
||||
// is an alias onto the real one, so a lexical ancestor walk reaches the
|
||||
// sandbox while the write lands in `$REAL/.agents`. On Windows the alias
|
||||
// would be a junction; the mechanism is the same.
|
||||
test('refuses a sandbox whose .agents is an ALIAS onto the real one', (t) => {
|
||||
const realHome = createTempDir('gsd-3712-alias-');
|
||||
t.after(() => cleanup(realHome));
|
||||
const realAgents = path.join(realHome, '.agents');
|
||||
fs.mkdirSync(path.join(realAgents, 'skills'), { recursive: true });
|
||||
const sandbox = path.join(realHome, 'AppData', 'Local', 'Temp', 'gsd-sandbox-c');
|
||||
fs.mkdirSync(sandbox, { recursive: true });
|
||||
fs.symlinkSync(realAgents, path.join(sandbox, '.agents'), 'dir');
|
||||
assert.throws(
|
||||
() => testHomeGuard.assertTestHomeSandboxed('installRuntimeArtifacts', 'codex',
|
||||
escapingKinds(path.join(sandbox, '.agents')),
|
||||
{ os: fakeOs(sandbox, realHome), env: underTest }),
|
||||
/destination inside\s+your REAL home/,
|
||||
'containment must be decided on where the write LANDS, not how it is spelled',
|
||||
);
|
||||
});
|
||||
|
||||
// The refusal happens before any write, so it is not a partial install.
|
||||
// bin/install.js reads this to decide NOT to run its pre-config rollback,
|
||||
// which would otherwise delete and recreate every snapshotted gsd-* dir in
|
||||
// the real skills root — the exact mutation this guard exists to prevent.
|
||||
test('a refusal is marked so callers can tell it from a partial install', (t) => {
|
||||
const realHome = createTempDir('gsd-3712-flag-');
|
||||
t.after(() => cleanup(realHome));
|
||||
let caught;
|
||||
try {
|
||||
testHomeGuard.assertTestHomeSandboxed('installRuntimeArtifacts', 'codex',
|
||||
escapingKinds(path.join(realHome, '.agents')),
|
||||
{ os: fakeOs(realHome, realHome), env: underTest });
|
||||
} catch (err) { caught = err; }
|
||||
assert.ok(caught, 'precondition: the guard must have refused');
|
||||
assert.equal(testHomeGuard.isTestHomeGuardRefusal(caught), true);
|
||||
assert.equal(testHomeGuard.isTestHomeGuardRefusal(new Error('unrelated')), false,
|
||||
'an unrelated failure IS a partial install and must still roll back');
|
||||
assert.equal(testHomeGuard.isTestHomeGuardRefusal(undefined), false);
|
||||
});
|
||||
|
||||
// The exemption above is the only path in this guard that can turn a
|
||||
// destination inside the real home into an allowed write, so its
|
||||
// cannot-tell branches have to refuse. Mutating either of them to `true`
|
||||
// left the whole suite green before this row existed.
|
||||
test('refuses when HOME cannot be placed at all, rather than exempting it', (t) => {
|
||||
const realHome = createTempDir('gsd-3712-nohome-place-');
|
||||
t.after(() => cleanup(realHome));
|
||||
const unplaceable = {
|
||||
'an empty homedir': () => '',
|
||||
'a homedir that throws': () => { throw new Error('no home'); },
|
||||
'a homedir that does not exist': () => path.join(realHome, 'absent-sandbox'),
|
||||
};
|
||||
for (const [label, homedir] of Object.entries(unplaceable)) {
|
||||
assert.throws(
|
||||
() => testHomeGuard.assertTestHomeSandboxed('installRuntimeArtifacts', 'codex',
|
||||
escapingKinds(path.join(realHome, '.agents')),
|
||||
{ os: { homedir, userInfo: () => ({ homedir: realHome }) }, env: underTest }),
|
||||
/destination inside\s+your REAL home/,
|
||||
`${label} must refuse — an unplaceable HOME is not evidence of a sandbox`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('the message names the operation and the fix, so it is not silenced blindly', (t) => {
|
||||
const realHome = createTempDir('gsd-3712-msg-');
|
||||
t.after(() => cleanup(realHome));
|
||||
let message = '';
|
||||
try {
|
||||
testHomeGuard.assertTestHomeSandboxed('applySurface', 'codex',
|
||||
escapingKinds(path.join(realHome, '.agents')),
|
||||
{ os: fakeOs(realHome, realHome), env: underTest });
|
||||
} catch (err) { message = err.message; }
|
||||
assert.match(message, /applySurface/, 'must name the writer that was refused');
|
||||
assert.match(message, /sandboxHome/, 'must point at the helper that fixes it');
|
||||
assert.match(message, /Fix the TEST, not this guard/);
|
||||
assert.match(message, /BEFORE resolving the layout/, 'must state the ordering that matters');
|
||||
});
|
||||
|
||||
test('is inert outside a test runner — real codex installs still write to $HOME/.agents', (t) => {
|
||||
// The override exists so a REAL install lands in the user's home. Blocking
|
||||
// that would break codex installs outright, so this row is load-bearing.
|
||||
const realHome = createTempDir('gsd-3712-prod-');
|
||||
t.after(() => cleanup(realHome));
|
||||
assert.doesNotThrow(() => testHomeGuard.assertTestHomeSandboxed(
|
||||
'installRuntimeArtifacts', 'codex',
|
||||
escapingKinds(path.join(realHome, '.agents')),
|
||||
{ os: fakeOs(realHome, realHome), env: {} },
|
||||
));
|
||||
});
|
||||
|
||||
test('ignores kinds with no home override — they cannot escape configDir', (t) => {
|
||||
const realHome = createTempDir('gsd-3712-nohome-');
|
||||
t.after(() => cleanup(realHome));
|
||||
assert.doesNotThrow(() => testHomeGuard.assertTestHomeSandboxed(
|
||||
'installRuntimeArtifacts', 'claude', confinedKinds,
|
||||
{ os: fakeOs(realHome, realHome), env: underTest },
|
||||
));
|
||||
});
|
||||
|
||||
// Identity, not pathname: path.resolve() resolves neither symlinks nor case,
|
||||
// and realpath returns a canonical pathname two routes to one directory can
|
||||
// still disagree on. Driven with the REAL fs — an injected os cannot prove
|
||||
// canonicalization, since that is what is under test.
|
||||
test('a destination reached through a SYMLINKED spelling of the real home is refused', (t) => {
|
||||
const realHome = createTempDir('gsd-3712-symreal-');
|
||||
const linkParent = createTempDir('gsd-3712-symlink-');
|
||||
const linkHome = path.join(linkParent, 'home-link');
|
||||
t.after(() => { cleanup(realHome); cleanup(linkParent); });
|
||||
fs.symlinkSync(realHome, linkHome, 'dir');
|
||||
|
||||
assert.throws(
|
||||
() => testHomeGuard.assertTestHomeSandboxed('installRuntimeArtifacts', 'codex',
|
||||
escapingKinds(path.join(linkHome, '.agents')),
|
||||
{ os: fakeOs(linkHome, realHome), env: underTest }),
|
||||
/destination inside\s+your REAL home/,
|
||||
'two spellings of one directory are not two directories',
|
||||
);
|
||||
});
|
||||
|
||||
test('refuses rather than guessing when the passwd home cannot be identified', () => {
|
||||
assert.throws(
|
||||
() => testHomeGuard.assertTestHomeSandboxed('installRuntimeArtifacts', 'codex',
|
||||
escapingKinds('/nonexistent-root-3712/.agents'),
|
||||
{
|
||||
os: { homedir: () => '/nonexistent-root-3712', userInfo: () => { throw new Error('no passwd entry'); } },
|
||||
env: underTest,
|
||||
}),
|
||||
/no identifiable passwd home/,
|
||||
);
|
||||
});
|
||||
|
||||
test('a caller that recorded THIS home still works with no passwd entry', (t) => {
|
||||
// Keeps fail-closed from breaking passwd-less CI for callers that DID sandbox.
|
||||
const sandbox = createTempDir('gsd-3712-marker-');
|
||||
t.after(() => cleanup(sandbox));
|
||||
assert.doesNotThrow(() => testHomeGuard.assertTestHomeSandboxed(
|
||||
'installRuntimeArtifacts', 'codex',
|
||||
escapingKinds(path.join(sandbox, '.agents')),
|
||||
{
|
||||
os: { homedir: () => sandbox, userInfo: () => { throw new Error('no passwd entry'); } },
|
||||
env: { ...underTest, [testHomeGuard.SANDBOX_MARKER]: sandbox },
|
||||
},
|
||||
));
|
||||
});
|
||||
|
||||
test('a STALE marker naming a different home does not vouch for this call', (t) => {
|
||||
const sandbox = createTempDir('gsd-3712-marker2-');
|
||||
const other = createTempDir('gsd-3712-other-');
|
||||
t.after(() => { cleanup(sandbox); cleanup(other); });
|
||||
assert.throws(
|
||||
() => testHomeGuard.assertTestHomeSandboxed('installRuntimeArtifacts', 'codex',
|
||||
escapingKinds(path.join(sandbox, '.agents')),
|
||||
{
|
||||
os: { homedir: () => sandbox, userInfo: () => { throw new Error('no passwd entry'); } },
|
||||
env: { ...underTest, [testHomeGuard.SANDBOX_MARKER]: other },
|
||||
}),
|
||||
/no identifiable passwd home/,
|
||||
'the marker must name the home actually in effect',
|
||||
);
|
||||
});
|
||||
|
||||
// Codex review of #3725. The marker attests that a caller sandboxed HOME; it
|
||||
// says NOTHING about where an already-resolved destination points. A layout
|
||||
// captured before sandboxHome() still names the real `~/.agents`, and the
|
||||
// marker branch waved it straight through — the exact stale-layout shape the
|
||||
// primary branch refuses by design, reachable on any passwd-less host. Both
|
||||
// halves are now required: the marker names the home in effect AND every
|
||||
// destination derives from it.
|
||||
test('a marker matching HOME does not vouch for a destination that does not derive from it', (t) => {
|
||||
const sandbox = createTempDir('gsd-3712-marker-stale-');
|
||||
const realHome = createTempDir('gsd-3712-marker-real-');
|
||||
t.after(() => { cleanup(sandbox); cleanup(realHome); });
|
||||
assert.throws(
|
||||
() => testHomeGuard.assertTestHomeSandboxed('applySurface', 'codex',
|
||||
escapingKinds(path.join(realHome, '.agents')),
|
||||
{
|
||||
os: { homedir: () => sandbox, userInfo: () => { throw new Error('no passwd entry'); } },
|
||||
env: { ...underTest, [testHomeGuard.SANDBOX_MARKER]: sandbox },
|
||||
}),
|
||||
/NOT beneath that sandbox/,
|
||||
'a recorded sandbox vouches for HOME, never for a destination outside it',
|
||||
);
|
||||
});
|
||||
|
||||
// Codex review of #3725. resolveThroughLinks swallowed EVERY realpathSync
|
||||
// error and fell back to the LEXICAL spelling. That is a fail-open, and it
|
||||
// inverts the function's whole purpose: an aliased `<sandbox>/.agents` that
|
||||
// cannot be canonicalized keeps its sandbox spelling, satisfies the
|
||||
// nested-sandbox exemption, and the write is ALLOWED into the real home. The
|
||||
// module documents ONE named fail-open (the marker branch); this was a second,
|
||||
// unnamed one. ENOENT/ENOTDIR still walk up — that is the ordinary
|
||||
// destination-does-not-exist-yet case, and the split matches identify()'s.
|
||||
test('a destination component that cannot be canonicalized is refused, not assumed safe', (t) => {
|
||||
const sandbox = createTempDir('gsd-3712-uncanon-');
|
||||
const realHome = createTempDir('gsd-3712-uncanon-real-');
|
||||
t.after(() => { cleanup(sandbox); cleanup(realHome); });
|
||||
// A symlink CYCLE is the portable way to make realpathSync fail with
|
||||
// something other than "not there": every lookup through it returns ELOOP
|
||||
// while the parent directory resolves normally.
|
||||
const loopA = path.join(sandbox, 'loop-a');
|
||||
const loopB = path.join(sandbox, 'loop-b');
|
||||
try {
|
||||
fs.symlinkSync(loopB, loopA);
|
||||
fs.symlinkSync(loopA, loopB);
|
||||
} catch {
|
||||
t.skip('symlink creation unsupported on this platform/privilege');
|
||||
return;
|
||||
}
|
||||
assert.throws(
|
||||
() => testHomeGuard.assertTestHomeSandboxed('applySurface', 'codex',
|
||||
escapingKinds(path.join(loopA, '.agents')),
|
||||
{ os: fakeOs(sandbox, realHome), env: underTest }),
|
||||
/could not be canonicalized/,
|
||||
'an unresolvable component must refuse — falling back to the lexical spelling is the '
|
||||
+ 'exact ALLOW an aliased <sandbox>/.agents needs to reach the real home',
|
||||
);
|
||||
});
|
||||
|
||||
// Non-vacuity for the row above, and the reason it cannot simply refuse on
|
||||
// ANY realpath error: the ordinary case is a destination that does not exist
|
||||
// yet, where realpath fails ENOENT on the leaf and on every not-yet-created
|
||||
// ancestor. Refusing there would reject every fresh install.
|
||||
test('… but a destination that merely does not exist yet still resolves and is allowed', (t) => {
|
||||
const sandbox = createTempDir('gsd-3712-uncanon-enoent-');
|
||||
const realHome = createTempDir('gsd-3712-uncanon-enoent-real-');
|
||||
t.after(() => { cleanup(sandbox); cleanup(realHome); });
|
||||
const neverCreated = path.join(sandbox, 'not-created-yet', '.agents');
|
||||
assert.strictEqual(fs.existsSync(neverCreated), false,
|
||||
'the row is only meaningful while the destination is absent');
|
||||
assert.doesNotThrow(
|
||||
() => testHomeGuard.assertTestHomeSandboxed('applySurface', 'codex',
|
||||
escapingKinds(neverCreated),
|
||||
{ os: fakeOs(sandbox, realHome), env: underTest }),
|
||||
'ENOENT is the expected shape of a fresh install, not a canonicalization failure',
|
||||
);
|
||||
});
|
||||
|
||||
// ENOTDIR takes the same walk-up branch as ENOENT, deliberately: both mean
|
||||
// "no such path as named", which is the question identify() answers the same
|
||||
// way. Pinned so the two cannot drift apart.
|
||||
test('… and a component sitting behind a FILE (ENOTDIR) walks up rather than refusing', (t) => {
|
||||
const sandbox = createTempDir('gsd-3712-uncanon-enotdir-');
|
||||
const realHome = createTempDir('gsd-3712-uncanon-enotdir-real-');
|
||||
t.after(() => { cleanup(sandbox); cleanup(realHome); });
|
||||
const asFile = path.join(sandbox, 'a-file');
|
||||
fs.writeFileSync(asFile, 'not a directory\n');
|
||||
assert.doesNotThrow(
|
||||
() => testHomeGuard.assertTestHomeSandboxed('applySurface', 'codex',
|
||||
escapingKinds(path.join(asFile, '.agents')),
|
||||
{ os: fakeOs(sandbox, realHome), env: underTest }),
|
||||
'ENOTDIR means the path does not exist as named — identify() calls that absent, and this '
|
||||
+ 'walk must agree with it rather than inventing a second errno policy',
|
||||
);
|
||||
});
|
||||
|
||||
// Review of #3725, Major 1. sameDirectory() is read by the marker branch as
|
||||
// permission to PROCEED, so "cannot tell" has to answer no. It used to answer
|
||||
// yes whenever neither side identified — two absent paths here, or two stats
|
||||
// failing EACCES/EPERM/EIO on a locked-down host — which turned the passwd-less
|
||||
// escape hatch into an unconditional bypass for any marker value at all.
|
||||
// The absent/absent shape is the portable way to pin it; the errno shapes take
|
||||
// the same branch.
|
||||
test('a marker and a home that BOTH fail to identify do not vouch for each other', (t) => {
|
||||
const root = createTempDir('gsd-3712-unident-');
|
||||
t.after(() => cleanup(root));
|
||||
// Two DIFFERENT paths, so the same-pathname shortcut cannot fire, and
|
||||
// neither exists, so neither identifies.
|
||||
const missingHome = path.join(root, 'home-never-created');
|
||||
const missingMarker = path.join(root, 'marker-never-created');
|
||||
assert.throws(
|
||||
() => testHomeGuard.assertTestHomeSandboxed('installRuntimeArtifacts', 'codex',
|
||||
escapingKinds(path.join(missingHome, '.agents')),
|
||||
{
|
||||
os: { homedir: () => missingHome, userInfo: () => { throw new Error('no passwd entry'); } },
|
||||
env: { ...underTest, [testHomeGuard.SANDBOX_MARKER]: missingMarker },
|
||||
}),
|
||||
/no identifiable passwd home/,
|
||||
'two unidentifiable paths are not evidence that either is a sandbox',
|
||||
);
|
||||
});
|
||||
|
||||
// Review of #3725, Major 2, raised as a question rather than a finding: on
|
||||
// Windows Node derives st_dev/st_ino from BY_HANDLE_FILE_INFORMATION, and the
|
||||
// uniqueness guarantees are weaker than POSIX. If dev collapsed to a per-volume
|
||||
// constant AND ino were 0 for directories, isInside() would match its very
|
||||
// first ancestor and report every path on the drive as inside the real home.
|
||||
// The whole guard rests on this primitive, so assert it directly instead of
|
||||
// reasoning about it — this row runs on every platform in the matrix, so
|
||||
// Windows answers the question itself.
|
||||
test('filesystem identity discriminates directories on this platform', (t) => {
|
||||
const a = createTempDir('gsd-3712-ident-a-');
|
||||
const b = createTempDir('gsd-3712-ident-b-');
|
||||
t.after(() => { cleanup(a); cleanup(b); });
|
||||
const sa = fs.statSync(a);
|
||||
const sb = fs.statSync(b);
|
||||
assert.ok(sa.dev !== sb.dev || sa.ino !== sb.ino,
|
||||
`two distinct directories share an identity (dev=${sa.dev}/${sb.dev}, ino=${sa.ino}/${sb.ino}) — ` +
|
||||
'isInside() would then match its first ancestor and refuse everything');
|
||||
const again = fs.statSync(path.join(a, '..', path.basename(a)));
|
||||
assert.equal(again.dev, sa.dev, 'one directory reached by two spellings must keep one dev');
|
||||
assert.equal(again.ino, sa.ino, 'one directory reached by two spellings must keep one ino');
|
||||
});
|
||||
|
||||
// An ambient marker must not disarm the guard on the PRIMARY path: it is only
|
||||
// consulted when the real home cannot be identified at all.
|
||||
test('an ambient marker does NOT disarm the guard when the real home is identifiable', (t) => {
|
||||
const realHome = createTempDir('gsd-3712-ambient-');
|
||||
t.after(() => cleanup(realHome));
|
||||
assert.throws(
|
||||
() => testHomeGuard.assertTestHomeSandboxed('installRuntimeArtifacts', 'codex',
|
||||
escapingKinds(path.join(realHome, '.agents')),
|
||||
{
|
||||
os: fakeOs(realHome, realHome),
|
||||
env: { ...underTest, [testHomeGuard.SANDBOX_MARKER]: realHome },
|
||||
}),
|
||||
/destination inside\s+your REAL home/,
|
||||
'destination containment is authoritative; no marker may short-circuit it',
|
||||
);
|
||||
});
|
||||
|
||||
test('the marker name tests/helpers.cjs writes matches the one the guard reads', () => {
|
||||
// helpers.cjs duplicates this string rather than requiring the compiled guard,
|
||||
// to keep its documented no-built-lib-at-import-time contract. Pin the pair.
|
||||
const { TEST_HOME_SANDBOX_MARKER } = require('./helpers.cjs');
|
||||
assert.strictEqual(TEST_HOME_SANDBOX_MARKER, testHomeGuard.SANDBOX_MARKER,
|
||||
'the helper and the guard must agree on the marker env var name');
|
||||
});
|
||||
});
|
||||
|
||||
// ── the wiring ───────────────────────────────────────────────────────────
|
||||
// These call the REAL entrypoints. Deleting a guard call site makes them fail;
|
||||
// the predicate rows above would stay green, which is why both halves exist.
|
||||
|
||||
describe('wiring — every writer that resolves kind.home is guarded', () => {
|
||||
/**
|
||||
* Sandbox the real HOME (so nothing real is ever at risk), then tell the guard
|
||||
* — through its deps seam only — that this sandboxed home IS the passwd home.
|
||||
* The layout then resolves its `home` override under that directory, so the
|
||||
* destination lands inside what the guard believes is the real home: exactly
|
||||
* "the caller forgot", reproduced against a throwaway directory.
|
||||
*/
|
||||
function forgottenSandbox(t) {
|
||||
const configDir = createTempDir('gsd-3712-cfg-');
|
||||
const homeDir = createTempDir('gsd-3712-home-');
|
||||
t.after(() => { cleanup(configDir); cleanup(homeDir); });
|
||||
sandboxHome(t, homeDir);
|
||||
return { configDir, homeDir, deps: { os: fakeOs(homeDir, homeDir), env: underTest } };
|
||||
}
|
||||
|
||||
test('installRuntimeArtifacts refuses before it writes or prunes anything', (t) => {
|
||||
const { configDir, homeDir, deps } = forgottenSandbox(t);
|
||||
const profile = { name: 'full', skills: '*', agents: new Set() };
|
||||
assert.throws(
|
||||
() => installEngine.installRuntimeArtifacts(
|
||||
'codex', configDir, 'global', profile, () => undefined, undefined, deps,
|
||||
),
|
||||
/destination inside\s+your REAL home/,
|
||||
'the guard must be wired into installRuntimeArtifacts, not merely exported',
|
||||
);
|
||||
assert.ok(!fs.existsSync(path.join(homeDir, '.agents', 'skills')),
|
||||
'it must refuse BEFORE creating the escaped destination');
|
||||
});
|
||||
|
||||
test('uninstallRuntimeArtifacts refuses — it prunes the same escaped home', (t) => {
|
||||
const { configDir, deps } = forgottenSandbox(t);
|
||||
assert.throws(
|
||||
() => installEngine.uninstallRuntimeArtifacts('codex', configDir, 'global', deps),
|
||||
/destination inside\s+your REAL home/,
|
||||
'uninstall resolves the same kind.home and calls _removeGsdEntries on it',
|
||||
);
|
||||
});
|
||||
|
||||
test('applySurface refuses — its dest selection prefers kind.home, then syncs', (t) => {
|
||||
const { configDir, deps } = forgottenSandbox(t);
|
||||
const layout = runtimeArtifactLayout.resolveRuntimeArtifactLayout('codex', configDir, 'global');
|
||||
assert.throws(
|
||||
() => surface.applySurface(configDir, layout, new Map(), undefined, undefined, undefined, deps),
|
||||
/destination inside\s+your REAL home/,
|
||||
'surface apply is the third writer that reaches kind.home',
|
||||
);
|
||||
});
|
||||
|
||||
// The fourth reachable writer, and the one the first pass of #3712 missed: it
|
||||
// CREATES SKILL.md under the skills kind's `home` instead of pruning, and it
|
||||
// runs from _runLegacyInstallMigrations — BEFORE installRuntimeArtifacts'
|
||||
// assertion — so the rows above cannot cover it. Driven directly because that
|
||||
// is the seam it has; `saved` must carry dev-preferences.md or the function
|
||||
// returns early before ever resolving a target.
|
||||
test('migrateLegacyDevPreferencesToSkill refuses — it writes SKILL.md under kind.home', (t) => {
|
||||
const { configDir, homeDir, deps } = forgottenSandbox(t);
|
||||
const saved = new Map([['dev-preferences.md', '# saved\n']]);
|
||||
assert.throws(
|
||||
() => installEngine.migrateLegacyDevPreferencesToSkill(configDir, saved, 'codex', 'global', deps),
|
||||
/destination inside\s+your REAL home/,
|
||||
'the legacy migration is the sixth writer that reaches kind.home, and it runs first',
|
||||
);
|
||||
assert.ok(!fs.existsSync(path.join(homeDir, '.agents', 'skills', 'gsd-dev-preferences')),
|
||||
'it must refuse BEFORE creating the escaped skill directory');
|
||||
});
|
||||
|
||||
// The guard call is conditional on `target.hasHomeOverride` — the skills
|
||||
// kind's own answer to "did it declare a home override?", read off the same
|
||||
// layout resolution the write uses. Pin the ALLOW half too, so widening that
|
||||
// condition cannot pass silently. configDir sits INSIDE the (fake) real home
|
||||
// deliberately: that is what gives this row teeth. With the condition as
|
||||
// written the guard is never consulted, because no home override was
|
||||
// declared; widened to run unconditionally it would see a destination inside
|
||||
// the real home with no sandbox to derive from, and refuse an ordinary
|
||||
// confined migration.
|
||||
test('… and still migrates for a runtime whose skills kind declares no home override', (t) => {
|
||||
const { homeDir, deps } = forgottenSandbox(t);
|
||||
const configDir = path.join(homeDir, '.claude');
|
||||
fs.mkdirSync(configDir, { recursive: true });
|
||||
const layout = runtimeArtifactLayout.resolveRuntimeArtifactLayout('claude', configDir, 'global');
|
||||
const skills = layout.kinds.find((k) => k.kind === 'skills');
|
||||
assert.ok(skills && !skills.home,
|
||||
'claude skills must carry NO home override — if this fails the descriptor drifted '
|
||||
+ 'and this row is no longer testing the ALLOW half');
|
||||
const saved = new Map([['dev-preferences.md', '# saved\n']]);
|
||||
assert.strictEqual(
|
||||
installEngine.migrateLegacyDevPreferencesToSkill(configDir, saved, 'claude', 'global', deps),
|
||||
true,
|
||||
'a confined destination must not be refused — the guard is destination-keyed, not runtime-keyed',
|
||||
);
|
||||
});
|
||||
|
||||
// Codex review of #3725. `installRoot !== targetDir` was the stand-in for "the
|
||||
// skills kind declared a home override", and the two are not equivalent: the
|
||||
// inequality is FALSE when the override resolves onto targetDir itself — a
|
||||
// configDir of `$HOME/.agents`, which is exactly where codex's override points.
|
||||
// The guard was skipped and SKILL.md was written into the real home under a
|
||||
// test runner. The condition now reads the declaration off the same layout
|
||||
// resolution instead of comparing two paths.
|
||||
test('… and refuses when the configDir IS the home-override destination', (t) => {
|
||||
const { homeDir, deps } = forgottenSandbox(t);
|
||||
const configDir = path.join(homeDir, '.agents');
|
||||
fs.mkdirSync(configDir, { recursive: true });
|
||||
const saved = new Map([['dev-preferences.md', '# saved\n']]);
|
||||
assert.throws(
|
||||
() => installEngine.migrateLegacyDevPreferencesToSkill(configDir, saved, 'codex', 'global', deps),
|
||||
/destination inside\s+your REAL home/,
|
||||
'an override that resolves onto targetDir is still a declared override',
|
||||
);
|
||||
assert.ok(!fs.existsSync(path.join(configDir, 'skills', 'gsd-dev-preferences')),
|
||||
'it must refuse BEFORE creating the skill directory in the real home');
|
||||
});
|
||||
});
|
||||
|
||||
// ── the property the one real fix depends on ─────────────────────────────
|
||||
|
||||
test('a sandboxed HOME actually contains codex\'s global skills kind', (t) => {
|
||||
const configDir = createTempDir('gsd-3712-contain-cfg-');
|
||||
const homeDir = createTempDir('gsd-3712-contain-home-');
|
||||
t.after(() => { cleanup(configDir); cleanup(homeDir); });
|
||||
|
||||
const ambientHome = path.resolve(os.homedir());
|
||||
sandboxHome(t, homeDir);
|
||||
|
||||
const layout = runtimeArtifactLayout.resolveRuntimeArtifactLayout('codex', configDir, 'global');
|
||||
const skills = layout.kinds.find((k) => k.kind === 'skills');
|
||||
assert.ok(skills && skills.home,
|
||||
'codex skills must still carry the global home override this guard exists for — '
|
||||
+ 'if this fails the descriptor drifted, and the guard is now testing nothing');
|
||||
|
||||
const dest = path.resolve(path.join(skills.home, skills.destSubpath));
|
||||
assert.ok(dest.startsWith(path.resolve(homeDir) + path.sep),
|
||||
`codex skills must resolve inside the sandboxed HOME, got ${dest}`);
|
||||
assert.ok(!dest.startsWith(ambientHome + path.sep),
|
||||
`codex skills must NOT resolve inside the ambient home ${ambientHome}, got ${dest}`);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -16,7 +16,7 @@ const { writeSurface, readSurface, resolveSurface, listSurface, applySurface } =
|
||||
const { loadSkillsManifest, writeActiveProfile, resolveProfile } = require('../gsd-core/bin/lib/install-profiles.cjs');
|
||||
const { resolveRuntimeArtifactLayout } = require('../gsd-core/bin/lib/runtime-artifact-layout.cjs');
|
||||
const { CLUSTERS, allClusteredSkills } = require('../gsd-core/bin/lib/clusters.cjs');
|
||||
const { createTempDir, cleanup } = require('./helpers.cjs');
|
||||
const { createTempDir, cleanup, sandboxHome } = require('./helpers.cjs');
|
||||
|
||||
const REAL_COMMANDS_DIR = path.join(__dirname, '..', 'commands', 'gsd');
|
||||
|
||||
@@ -1140,13 +1140,20 @@ describe('skills-kind destination parity: installer vs surface-apply (#2911)', (
|
||||
function withFakeHome(fakeHome, fn) {
|
||||
const savedHome = process.env.HOME;
|
||||
const savedUserProfile = process.env.USERPROFILE;
|
||||
// #3712: record WHICH home this sandboxed to. src/test-home-guard.cts fails
|
||||
// closed on hosts with no readable passwd entry, and this is what proves a
|
||||
// genuinely-sandboxed caller there. Without it these calls would be refused.
|
||||
const savedMarker = process.env.GSD_TEST_HOME_SANDBOX;
|
||||
process.env.HOME = fakeHome;
|
||||
process.env.USERPROFILE = fakeHome;
|
||||
process.env.GSD_TEST_HOME_SANDBOX = fakeHome;
|
||||
try {
|
||||
return fn();
|
||||
} finally {
|
||||
if (savedHome === undefined) delete process.env.HOME; else process.env.HOME = savedHome;
|
||||
if (savedUserProfile === undefined) delete process.env.USERPROFILE; else process.env.USERPROFILE = savedUserProfile;
|
||||
if (savedMarker === undefined) delete process.env.GSD_TEST_HOME_SANDBOX;
|
||||
else process.env.GSD_TEST_HOME_SANDBOX = savedMarker;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1265,13 +1272,20 @@ describe('codex skills-kind destination: home override (#2911)', () => {
|
||||
function withFakeHome(fakeHome, fn) {
|
||||
const savedHome = process.env.HOME;
|
||||
const savedUserProfile = process.env.USERPROFILE;
|
||||
// #3712: record WHICH home this sandboxed to. src/test-home-guard.cts fails
|
||||
// closed on hosts with no readable passwd entry, and this is what proves a
|
||||
// genuinely-sandboxed caller there. Without it these calls would be refused.
|
||||
const savedMarker = process.env.GSD_TEST_HOME_SANDBOX;
|
||||
process.env.HOME = fakeHome;
|
||||
process.env.USERPROFILE = fakeHome;
|
||||
process.env.GSD_TEST_HOME_SANDBOX = fakeHome;
|
||||
try {
|
||||
return fn();
|
||||
} finally {
|
||||
if (savedHome === undefined) delete process.env.HOME; else process.env.HOME = savedHome;
|
||||
if (savedUserProfile === undefined) delete process.env.USERPROFILE; else process.env.USERPROFILE = savedUserProfile;
|
||||
if (savedMarker === undefined) delete process.env.GSD_TEST_HOME_SANDBOX;
|
||||
else process.env.GSD_TEST_HOME_SANDBOX = savedMarker;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1372,6 +1386,14 @@ describe('installOpencodeFamilySkills destination parity (#2911 sibling coverage
|
||||
const configDir = fs.mkdtempSync(path.join(os.tmpdir(), `gsd-2911-ocfs-${runtime}-`));
|
||||
const fakeHomeOverride = fs.mkdtempSync(path.join(os.tmpdir(), `gsd-2911-ocfs-home-${runtime}-`));
|
||||
t.after(() => { cleanup(configDir); cleanup(fakeHomeOverride); });
|
||||
// #3712 — this row drives a skills-kind `home` override on purpose, which is
|
||||
// exactly what the test-home guard exists to police, so it has to declare the
|
||||
// sandbox rather than rely on the destination happening to sit outside the
|
||||
// real home. On POSIX it does (os.tmpdir() is /tmp or /var/folders); on
|
||||
// Windows os.tmpdir() is under %USERPROFILE%, so without this the guard
|
||||
// correctly refuses and the row fails on Windows only. HOME is the override
|
||||
// itself, which is the home this call actually writes under.
|
||||
sandboxHome(t, fakeHomeOverride);
|
||||
|
||||
const originalResolve = runtimeArtifactLayoutModule.resolveRuntimeArtifactLayout;
|
||||
// Capture the real destSubpath before patching so the assertion below
|
||||
|
||||
Reference in New Issue
Block a user