From a75532db180c7da223efc0757e7fee9b0092e4fe Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Fri, 23 Oct 2026 15:56:19 -0500 Subject: [PATCH] fix(installer): allow scoped Claude paths and leave unreadable cwd (#1816, Vikunja 614). Untested --- BUILD-LOG.md | 2 + .../gladstone/work/hostcheck-item5/BUILD.md | 63 +++++++++++++++++++ docs/guides/host-run-sheet.md | 4 +- packages/cli/tests/case1-hostcheck.test.mjs | 6 +- packages/cli/tests/case2-unit.test.mjs | 29 +++++++++ packages/cli/tests/stack-copy.test.mjs | 16 +++++ scripts/case1-hostcheck.sh | 2 +- scripts/case2-unit.sh | 2 +- scripts/stack-copy.sh | 3 + 9 files changed, 121 insertions(+), 6 deletions(-) diff --git a/BUILD-LOG.md b/BUILD-LOG.md index bd25808e9..4e91360da 100644 --- a/BUILD-LOG.md +++ b/BUILD-LOG.md @@ -5413,3 +5413,5 @@ Row 299 (573, #1806, Gander, Filbert): before, the P10 band test timed a fault r 2026-10-23T17:32:37Z | Gyro sage-3544 fenton-0016 N1/N2 delta | Before: W7 Gate/final real-conversion list omit the W8 build-order cross-reference and W8 human-view filtering cites agent settings. After: both locations explicitly put W8 landing before human opening as mos-1470 build order, not acceptance; comment/tracked-deletion links are not current story links and never appear in the human Connections view regardless of agent settings. New commit atop approved 5af060fb5, no amend. Static diff and append-only checks pass. Docs only, no repo Node/tests/live action. Fenton checks only this delta. 2026-10-23 | Gladstone | Gitea #1816 / Vikunja 614 hostcheck item 5 and bundle verification | Before: dpkg's correct 750 form falsely fails item 5, and two run-sheet bundle verifies require a missing repository. Draft after: exact root/mosaic/path with octal 0750 equivalence; stat/output unchanged; both verifies use cleaned-up bare scratch repos. Three additive-only test blocks, two planned mutants. Untested under Sage-3738; Sage-3740 authorizes untested branch push for static review. Queue row pending; Sage owns gate and landing. + +2026-10-23 | Gladstone | #1816 / Vikunja 614 scope amendment mos-1534, sage-3741 | F1 narrowed to exact 750/0750 and single spaces; F3 allows @ in absolute unit paths with unsafe-character refusals retained; F4 moves copy_tree to / after path resolution. AS_USER audit found git -C and stack-release's existing checkout cd cover the other steps. Added new unit and unreadable-cwd copy tests only. Untested under Sage's no-Node hold; no live action. diff --git a/agents/gladstone/work/hostcheck-item5/BUILD.md b/agents/gladstone/work/hostcheck-item5/BUILD.md index 3d5e31e54..b9d232305 100644 --- a/agents/gladstone/work/hostcheck-item5/BUILD.md +++ b/agents/gladstone/work/hostcheck-item5/BUILD.md @@ -49,3 +49,66 @@ Push only gladstone/*, with MOSAIC_GITEA_CREDENTIAL_FILE unset; no amend/force. Static draft receipt: `git diff --check` passed. Test-file numstat against the base is `46 0 packages/cli/tests/case1-hostcheck.test.mjs` (added/deleted). + +## Scope and implementation amendment (mos-1534, sage-3741) + +F1 correction: Sage requires precisely `750` or `0750` and exactly single spaces, +so the candidate now uses `0?750` and literal spaces. It refuses 00750/000750, +a doubled space and a tab. This supersedes the initial numeric-equivalence design +above. Only this row's own appended tests change; no base test line changes. + +F3: case2-unit.sh plain() adds @. abs_plain is its only caller and requires an +absolute normalized path, so @ cannot become an ExecStart prefix. In sed @ is +ordinary replacement text (the delimiter is |); &, backslash and | still refuse. +Environment=MOSAIC_CLAUDE_BIN carries @ literally; the parser compares the complete +value, with no shell evaluation. @anthropic-ai also cannot match the residual +@[A-Z_]+@ token check. Tests render/reinstall with a regular executable under +@anthropic-ai, checking Environment and the new copy's WorkingDirectory. Since a +nonroot test cannot create root ownership, a stat fake models root:0755 only for +that file and its ancestor chain. The real file type/executable checks, render, +installed-unit parser, and all other stat calls remain active. A separate block +refuses space, %, $, both quote characters, backslash, & and |. + +F4: copy_tree changes to / before archive/npm/find, after all its path arguments +are absolute. copy_bundle retains its existing relative-bundle resolution before +calling copy_tree. Checkout copies have absolute REPO and COPIES too. Source-only +use from stack-release.sh changes no cwd because copy_tree is not called there. +Audit of every AS_USER command in stack-upgrade.sh: fetch and merge use git -C; +plan, gate, revert, confirm-rollback, stage (activate/rollback), confirm and rehearse +invoke stack-release.sh, which changes to its checkout at line 34. No other +AS_USER command needs a fix. The new test enters a directory mode 0111, verifies +that ls cannot read it, then runs both a relative-bundle copy and checkout copy, +asserting success, no find restore error, and an immutable completed copy. + +Additional mutants (unrun under the same no-Node hold): + +| Mutant | Change | Killer | Result | +|---|---|---|---| +| M3 | Drop @ from plain() | new factory scoped Claude-path test | Unrun | +| M4 | Remove copy_tree's cd / | new unreadable-cwd copy test | Unrun | + +All four fixes remain untested. Follow-up is an additive commit on the pushed +branch, per sage-3741, with no amend or force. Morgana's static F1/F2 read of +f30c74b12 was CLEAN (morgana-0205); F3/F4 and execution remain open. + +Morgana N1: the outer bundle-verification failure now prints +`STOP: bundle verify failed; do not continue` instead of exiting the interactive +root shell. The subshell still returns failure and cleans up. This follows the +sheet's human read-and-stop convention and avoids sending remaining pasted lines +to a parent shell after exiting sudo -i. Inner mktemp failure still exits only the +subshell. This supersedes the initial outer-shell failure behavior noted above. + +## File numstat against 93694c518 (added / deleted) + +| File | Added | Deleted | +|---|---:|---:| +| `BUILD-LOG.md` | 4 | 0 | +| `agents/gladstone/work/hostcheck-item5/BUILD.md` | 114 | 0 | +| `docs/SESSIONS.md` | 1 | 0 | +| `docs/guides/host-run-sheet.md` | 12 | 2 | +| `packages/cli/tests/case1-hostcheck.test.mjs` | 48 | 0 | +| `packages/cli/tests/case2-unit.test.mjs` | 29 | 0 | +| `packages/cli/tests/stack-copy.test.mjs` | 16 | 0 | +| `scripts/case1-hostcheck.sh` | 2 | 2 | +| `scripts/case2-unit.sh` | 1 | 1 | +| `scripts/stack-copy.sh` | 3 | 0 | diff --git a/docs/guides/host-run-sheet.md b/docs/guides/host-run-sheet.md index 3f9aded87..980bf3954 100644 --- a/docs/guides/host-run-sheet.md +++ b/docs/guides/host-run-sheet.md @@ -194,7 +194,7 @@ echo " /root/.bundle" | sha256sum -c - trap 'rm -rf "$bundle_check"' EXIT git init -q --bare "$bundle_check" && git -C "$bundle_check" bundle verify /root/.bundle -) || exit 1 +) || echo "STOP: bundle verify failed; do not continue" # Root's checkout. stack-upgrade.sh and case2-unit.sh run from here, never from a copy. git clone -q -b refactor /root/.bundle /root/mosaic-stack @@ -216,7 +216,7 @@ echo " /root/.bundle" | sha256sum -c - trap 'rm -rf "$bundle_check"' EXIT git init -q --bare "$bundle_check" && git -C "$bundle_check" bundle verify /root/.bundle -) || exit 1 +) || echo "STOP: bundle verify failed; do not continue" git -C /root/mosaic-stack fetch -q /root/.bundle refs/heads/refactor:refs/remotes/origin/refactor git -C /root/mosaic-stack checkout -q --detach install -o mosaic -g mosaic -m 0600 /root/.bundle /srv/mosaic/incoming/.bundle diff --git a/packages/cli/tests/case1-hostcheck.test.mjs b/packages/cli/tests/case1-hostcheck.test.mjs index 99a65df99..69b087d6e 100644 --- a/packages/cli/tests/case1-hostcheck.test.mjs +++ b/packages/cli/tests/case1-hostcheck.test.mjs @@ -335,7 +335,7 @@ test("row 240 pins and usage: the shipped pins are r6's, the script reads the pi }); test("Vikunja 614 check: item 5 accepts dpkg's bare octal mode and leading-zero equivalents", (t) => { - for (const mode of ["750", "0750", "000750"]) { + for (const mode of ["750", "0750"]) { const h = host(t); put(join(h.hc, "statoverride"), `root mosaic ${mode} /usr/bin/bwrap\n`); const r = run(h, "--check"); @@ -359,7 +359,9 @@ test("Vikunja 614 enforce: dpkg's bare 750 leaves bwrap open and writes no DRIFT test("Vikunja 614 check: item 5 refuses other modes, owners, groups, paths and malformed overrides", (t) => { const overrides = [ - ...["755", "0755", "4750", "75", "7500", "075", "0780", "8", "0o750", "+750", "-750", "7x0", "750+0", ""].map((mode) => `root mosaic ${mode} /usr/bin/bwrap\n`), + ...["755", "0755", "4750", "75", "7500", "075", "00750", "000750", "0780", "8", "0o750", "+750", "-750", "7x0", "750+0", ""].map((mode) => `root mosaic ${mode} /usr/bin/bwrap\n`), + "root mosaic 750 /usr/bin/bwrap\n", + "root\tmosaic 750 /usr/bin/bwrap\n", "mosaic mosaic 750 /usr/bin/bwrap\n", "root root 750 /usr/bin/bwrap\n", "root mosaic 750 /usr/bin/other\n", diff --git a/packages/cli/tests/case2-unit.test.mjs b/packages/cli/tests/case2-unit.test.mjs index 78dd77721..f0e5ee006 100644 --- a/packages/cli/tests/case2-unit.test.mjs +++ b/packages/cli/tests/case2-unit.test.mjs @@ -872,3 +872,32 @@ test("inside a session's sandbox a bind below 1024 fails with EACCES, and a bind assert.equal(bindIn(LOW_PORT), "EACCES"); assert.equal(bindIn(high), "bound"); }); + +test("Vikunja 614 factory: scoped @anthropic-ai Claude path renders and passes installed read-back", (t) => { + const h = hub(t); + const claude = script(join(h.root, "opt", "claude-pin", "node_modules", "@anthropic-ai", "claude-code", "bin", "claude.exe"), "exit 0"); + // The suite runs unprivileged. Model root ownership for this regular executable + // and its ancestors only; file type/executable checks and both unit readers are real. + const ancestors = []; + for (let p = claude; p !== "/"; p = dirname(p)) ancestors.push(p); + script(join(h.bin, "stat"), `if [ "$1" = -c ] && [ "$3" = -- ]; then + case "$4" in ${ancestors.map((p) => JSON.stringify(p)).join("|")}) + case "$2" in %u) echo 0; exit ;; %a) echo 755; exit ;; esac ;; + esac +fi +exec /usr/bin/stat "$@"`); + const first = unit(h, { profile: "factory", extra: ["--claude", claude] }); + assert.equal(first.code, 0, first.out + first.err); + assert.ok(lines(read(busFile(h)), "Environment").includes(`MOSAIC_CLAUDE_BIN=${claude}`)); + const again = unit(h, { profile: "factory", copy: COPY_B, extra: ["--claude", claude] }); + assert.equal(again.code, 0, again.out + again.err); + assert.ok(lines(read(busFile(h)), "Environment").includes(`MOSAIC_CLAUDE_BIN=${claude}`)); + assert.deepEqual(lines(read(busFile(h)), "WorkingDirectory"), [copyDir(h, COPY_B)]); +}); + +test("Vikunja 614 factory: plain paths still refuse every named unsafe character", (t) => { + const h = hub(t); + for (const char of [" ", "%", "$", "\"", "'", "\\", "&", "|"]) { + refuses(unit(h, { profile: "factory", extra: ["--claude", `/opt/claude${char}bin`] }), /--claude contains a character the unit cannot carry/); + } +}); diff --git a/packages/cli/tests/stack-copy.test.mjs b/packages/cli/tests/stack-copy.test.mjs index d769bca93..d54abfc78 100644 --- a/packages/cli/tests/stack-copy.test.mjs +++ b/packages/cli/tests/stack-copy.test.mjs @@ -347,3 +347,19 @@ test('row 232: check --units external reads the system units, the default reads rmSync(argv); } }); + +test('Vikunja 614: bundle and checkout copy leave an unreadable caller directory before find',t=>{ + const s=setup(t),cwd=join(s.root,'unreadable');mkdirSync(cwd); + const bundle=join(cwd,'release.bundle');git(s.repo,'bundle','create','-q',bundle,'refactor'); + chmodSync(cwd,0o111); + // Execute permission permits entry and named-file lookup; directory reads must fail. + assert.notEqual(spawnSync('/bin/ls',['-a','.'],{cwd,encoding:'utf8'}).status,0); + for(const [name,args] of [['bundle',['copy','--bundle','release.bundle',s.id]],['checkout',['copy',s.id]]]){ + const copies=join(s.root,`copies-${name}`); + const r=spawnSync(join(s.repo,'scripts','stack-copy.sh'),[...args,'--copies',copies],{cwd,env:s.env,encoding:'utf8',timeout:30000}); + assert.equal(r.status,0,r.stdout+r.stderr); + assert.doesNotMatch(r.stderr,/Failed to restore initial working directory|Permission denied/); + assert.ok(existsSync(join(copies,s.id,'scripts','mosaic'))); + assert.equal(statSync(join(copies,s.id)).mode&0o777,0o555); + } +}); diff --git a/scripts/case1-hostcheck.sh b/scripts/case1-hostcheck.sh index 1e59c4882..032e3caa9 100755 --- a/scripts/case1-hostcheck.sh +++ b/scripts/case1-hostcheck.sh @@ -114,7 +114,7 @@ item5() { local so st so="$(dpkg-statoverride --list "$BWRAP" 2>/dev/null)" st="$(stat -c '%U %G %a' "$ROOT$BWRAP" 2>/dev/null)" - [[ "$so" =~ ^root[[:blank:]]+mosaic[[:blank:]]+0*750[[:blank:]]+"$BWRAP"$ ]] || { fail "5 the statoverride on $BWRAP reads '$so', not 'root mosaic 0750'"; return; } + [[ "$so" =~ ^root\ mosaic\ 0?750\ "$BWRAP"$ ]] || { fail "5 the statoverride on $BWRAP reads '$so', not 'root mosaic 0750'"; return; } [ "$st" = "root mosaic 750" ] || { fail "5 $BWRAP is '$st', not 'root mosaic 750'"; return; } pass "5 $BWRAP is root mosaic 0750, by the statoverride and by stat" } diff --git a/scripts/case2-unit.sh b/scripts/case2-unit.sh index 2479f55d5..2361e0bd0 100755 --- a/scripts/case2-unit.sh +++ b/scripts/case2-unit.sh @@ -59,7 +59,7 @@ WEBUI_UNIT=mosaic-webui.service usage() { echo 'usage: case2-unit.sh --user NAME --home DIR --business ID --copy ID --node PATH [--profile factory|library] [--pm] [--claude PATH] -- SERVE FLAGS' >&2; exit 4; } die() { echo "case2-unit: $*" >&2; exit 1; } # A value the unit carries as itself: no space, %, $, quote, backslash, & or |. -plain() { case "$1" in *[!A-Za-z0-9/._+-]*) return 1 ;; esac; } +plain() { case "$1" in *[!A-Za-z0-9/._+@-]*) return 1 ;; esac; } abs_plain() { case "$1" in /*) ;; *) die "$2 must be an absolute path: $1" ;; esac plain "$1" || die "$2 contains a character the unit cannot carry: $1" diff --git a/scripts/stack-copy.sh b/scripts/stack-copy.sh index 1b0d76daf..adadc0d7d 100755 --- a/scripts/stack-copy.sh +++ b/scripts/stack-copy.sh @@ -59,6 +59,9 @@ copy_commit() { # The copy of commit $2 from repository $1. CLEAN names a bundle's temporary repository, which the # trap removes with the temporary copy. copy_tree() { + # Paths are absolute here (including a bundle resolved by copy_bundle). The caller + # may have dropped uid from a directory this user cannot read; find must start elsewhere. + cd / || die 'cannot change to /; no copy written' local full="$2" target tmp target="$COPIES/$full" [ ! -L "$target" ] || die "refusing: copy target is a symlink: $target"