Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…rator's private group Under Ubuntu's default umask 0002, ~/.local is 0775 and belongs to the user's private group. assertSafeDirectory refused every group-writable, non-sticky ancestor, so the default provider-home root ~/.local/state/ultrafuzz/provider-homes was refused, and OpenRouterAgent, DeepSeekAgent and any agent with a config_dir could not start. A group-writable ancestor is now accepted when the operator owns it, its group is the operator's primary group, /etc/group lists no member of that group other than the operator (under any /etc/passwd name with the operator's UID), and no other /etc/passwd account has it as its primary group. The /etc/passwd half matters: primary-group members are not listed in /etc/group, so a shared primary group such as `users` has an empty member list there. Anything the files cannot show fails closed: an unreadable file, an unrecognized entry, or an operator account or group missing from them (LDAP). World-writable ancestors without the sticky bit stay refused. Both refusals now name the directory and the remedy. Tests pin umask 0002 (test/process-umask.ts, the same file as #1235) and point the adapter at test-written account files by rewriting its /etc/passwd and /etc/group literals. They cover the accepted private group, eight shared or unprovable cases, world-writable ancestors, and the OpenRouter adapter contract with the default root. Closes #1236 Co-Authored-By: Claude Opus 5.5 <[email protected]>
… account source or another owner Review of the private-group check found that it read a directory's group permission bits as the owning group's. With a POSIX ACL they are the ACL mask, so a named user or group entry could grant another account write access to an accepted ~/.local, which main refused. A group-writable ancestor is now accepted only when /bin/ls -ld, run with LC_ALL=C and no other environment, shows that it has no ACL: GNU ls with no mark or only ".", or uutils ls 0.1.0 or later, which Ubuntu 26.04 uses, with no mark at all, since uutils marks any extended attribute with "+" and prints "." in its place under a security context. A "+", another ls, a failed or unparseable listing, or any diagnostic refuses it. Node and Bun cannot read ACLs, and getfacl is not installed by default. The check also fails closed when /etc/nsswitch.conf takes passwd, group or initgroups from anything but files, compat or systemd, since LDAP, SSSD and similar sources can share the group with accounts the files do not list. A member name that another UID also has counts as that account's. Every ancestor must now be owned by root or the operator, since a directory's owner can change its permissions. The boundary test lets a helper's policy declare the system tools it runs, so provider-home.tsx's ls argument lists are not counted as agent argv. The tests use a fake ls and injected account and nsswitch files, pin the owner and primary-group conditions by stubbing the process IDs, and no longer assume the temporary directory is not group writable. Co-Authored-By: Claude Opus 5.5 <[email protected]>
71f7b3f to
8b19c67
Compare
|
Holding this in draft. It fixes #1236 correctly, but by loosening the ancestor check, which takes about 150 lines of account-file, NSS and ACL probing. A simpler root fix is being built instead: the default provider-home root moves to a directory Ultrafuzz creates itself, mode 0700, directly under |
| mark = /^d[-rwxsStT]{9}([.+]?) /u.exec(listing.stdout)?.[1]; | ||
| if (!gnu && !version.stdout.startsWith("ls (uutils coreutils) ")) return undefined; | ||
| if (mark === "+") return true; | ||
| return mark === "" || (gnu && mark === ".") ? false : undefined; |
There was a problem hiding this comment.
If a Linux host's GNU /bin/ls was built without ACL support, it prints no + even when an ACL lets another account write to a group-writable ancestor. This code treats the missing mark as proof that there is no ACL and accepts the ancestor, allowing that account to modify the provider-home path. The GNU version string does not establish whether ACL detection is available.
How this was verified: The acceptance path trusts an unmarked GNU ls listing, while a GNU build without ACL support cannot mark an ACL that grants another account access.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/runtime/src/templates/smithers/agents/provider-home.tsx
Line: 200
Comment:
**Missing ACL mark permits access**
If a Linux host's GNU `/bin/ls` was built without ACL support, it prints no `+` even when an ACL lets another account write to a group-writable ancestor. This code treats the missing mark as proof that there is no ACL and accepts the ancestor, allowing that account to modify the provider-home path. The GNU version string does not establish whether ACL detection is available.
**How this was verified:** The acceptance path trusts an unmarked GNU `ls` listing, while a GNU build without ACL support cannot mark an ACL that grants another account access.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
|
Superseded by #1242, which fixes #1236 by moving the default provider-home root to |
1 participant
Problem
assertSafeDirectoryinpackages/runtime/src/templates/smithers/agents/provider-home.tsxrefused every group- or world-writable provider-home ancestor that is not sticky. Ubuntu's default umask is0002, so~/.localis0775and belongs to the user's private group. On the test host it isdrwxrwxr-x ubuntu:ubuntu, and/etc/grouphasubuntu:x:1000:, with no members. The default provider-home root,~/.local/state/ultrafuzz/provider-homes, was therefore refused.Without
ULTRAFUZZ_PROVIDER_HOME_ROOT, this affectsOpenRouterAgentandDeepSeekAgentalways, because they have no canonical home. It also affectsClaudeAgent,CodexAgentandKimiAgentwhen they setconfig_dir. The campaign fails at its first node that uses one of them.I reproduced it on
mainon this host:resolveProviderHome("openrouter", "probe")frommain's template, run with the realHOME, throwsprovider-home ancestors cannot be group/world writable.Change
provider-home.tsxEach existing directory above a provider home is checked in this order. Each refusal names the directory, the problem and the fix.
mainaccepted a0755ancestor owned by another account.provider-home ancestor <dir> is owned by another account; set ULTRAFUZZ_PROVIDER_HOME_ROOT to a directory outside it… is world writable without the sticky bit; remove that write access or set ULTRAFUZZ_PROVIDER_HOME_ROOT to a directory outside it).process.getuid()and the group isprocess.getgid()./etc/nsswitch.conftakespasswd,groupandinitgroupsonly fromfiles,compatorsystemd. Every line for those databases counts, because glibc versions differ on which one they use. A missingpasswdorgroupline, an empty service list, or an unreadable file refuses. Ubuntu's default,files systemd, passes.compatreads the same files, and the NIS+/-entries it would add are already refused as unrecognized./etc/passwdentry has that group as its primary group, and no other account does. Every/etc/groupentry with that GID lists only names that belong to the operator's UID and to no other UID. Membership goes by name, so a duplicate name counts as the other account's./bin/ls -ld -- <dir>shows that it has no ACL. The adapter runs/bin/ls --versionand/bin/ls -ld -- <dir>with onlyLC_ALL=Cin the environment..(a security context alone). uutils ls 0.1.0 or later must show no mark.+is refused:… is group writable, and ls marks it as having an ACL.… is group writable, and ls cannot show whether it has an ACL: any other ls, a non-zero exit, any stderr, or a listing that does not start with a mode string.Why an ACL probe. With a POSIX ACL, the group bits of
st_modeare the ACL mask, not the owning group's permission. The previous head readmode & 0o020as "the owning group can write" and accepted a directory whose group was private. A named user or group entry could still grant another account write access.mainrefused every such grant, because an ACL write grant works only when the mask includesw, and the mask shows asg+w. Node and Bun have no xattr API.getfaclandgetfattrare not installed by default, and are not on this host.lsis present on every host this check applies to.The adapter runs
/bin/lsby absolute path and does not check who owns the binary, unliketrustedGitExecutable. That function searchesPATH, where a user-writable directory can come first. Here there is no search to hijack, and only root can replace/bin/ls. The environment is onlyLC_ALL=C, soTIME_STYLE,QUOTING_STYLEand similar settings cannot change the listing.Why uutils as well, and why stricter. Ubuntu 26.04 LTS makes uutils coreutils 0.8.0 its default
ls, so a GNU-only probe would leave this issue unfixed on the current Ubuntu LTS. I ran the real release binaries on this host against real ACLs, which I set with Python'sos.setxattr:lsuser.*xattr only--versionfirst line++ls (GNU coreutils) 9.4+++ls (uutils coreutils) 0.8.0+++ls (uutils coreutils) 0.2.2+++<argv0> 0.0.30<argv0> 0.0.23uutils has two quirks, both read from its source:
.instead of+when a file has a security context (alt_access_indicatorinsrc/uu/ls/src/display.rs), so.is refused under uutils.has_aclcounts any extended attribute, and treats a failedlistxattras having none.The
ls (uutils coreutils)prefix only matches 0.1.0 and later, and every such release has the marker, which first appeared in 0.0.24.Boundary test
agent-adapter-boundaries.test.tscounts any array literal that contains a dash-prefixed string as agent argv construction, and a non-adapter helper may carry no such signal. SospawnSync(LS, ["-ld", "--", directory])failed it.A policy can now declare
systemTools. An argument list is exempt when it is the second argument ofspawn,spawnSync,execFileorexecFileSync, and the command is a declared tool, named by a literal or a top-level const.provider-home.tsxdeclares/bin/ls. A new test pins the boundary: an undeclared tool, an argv array that is not passed to such a call, and a call through another function are still flagged.docs/reference/agent-adapter-boundaries.mddocuments the exemption. I did not rebuild the argument lists in a form the detector misses.Template enforcement
Planning admits only the byte-exact packaged
.smithers/agentsclosure, and since #1173 plainultrafuzz initrewrites any stock adapter that differs from it. The closure digest is computed from the packaged templates at runtime (controller-source.ts), and #1173 removed the boundary test's source fingerprints. So no pinned digest or fingerprint needed updating.Tests
test/process-umask.tsis byte-identical to fix(runtime)!: keep the Forge guard under group-writable umasks, warn about old cloud runs' Modal storage in clean, and drop the smithers shim #1235's (blob5cd07bae, checked again against fix(runtime)!: keep the Forge guard under group-writable umasks, warn about old cloud runs' Modal storage in clean, and drop the smithers shim #1235's head40c8855f).test/provider-home-host.tsreplacesaccount-files.ts./etc/passwd,/etc/groupand/etc/nsswitch.confstand-ins for the test process's own UID and GID, plus a fake/bin/ls: ashscript that logsLC_ALL,HOMEand its arguments.groupWritableAncestorslists the directories the check will probe, so no test assumes the temporary directory is not group writable.provider-home.test.ts. Every test runs under umask0002with Ubuntu's layout (home0750,~/.local0775) and the default root reached throughHOME.--versionand-ld -- <dir>for exactly the group-writable ancestors, withLC_ALL=Cand noHOME. Every component the adapter creates is0700.teammatenow has its own passwd entry and primary group, so only the member list can share the group. Also: another account's primary group; a member name another UID also has (new); another primary group for the operator; no operator account; no group entry; an unrecognized entry; no group file. After each case the private files are restored, and the same directories are accepted again.compat, andfiles systemdwith a comment, leading spaces,[SUCCESS=merge]and a spaced action. Refused, 12 cases: SSSD; LDAP groups;initgroupsfrom winbind; a non-local line before or after a local one;GROUP:in capitals; nopasswdline; nogroupline; no service; an unclosed action; no file.+; uutils+; uutils.; uutils 0.0.30's version line; a BusyBox version line;?; exit 2; a diagnostic on stderr;total 0as the listing; no ls. Accepted: uutils 0.8.0 and 1.0.0 with no mark, and GNU../bin/lson the host. A fixture home passes when that ls is GNU's or uutils', and is refused with thecannot showmessage otherwise. Both branches assert, and neither skips.process.getuidreturning another UID, the first non-root ancestor is refused as owned by another account. Withprocess.getgidreturning another GID (as afternewgrp), the group-writable ancestor is refused.0777and0757) is refused, and1777is accepted.0002with the default root, as before, now with the fake ls injected. It asserts that the Bun-run adapter askedls -ldabout exactly the group-writable ancestors.Verification
Every new behaviour test fails on
origin/main. I putmain'sprovider-home.tsxinto this tree and rebuilt.must name /etc/passwd exactly once). The existing test fails onmain's message.main'sprovider-home ancestors cannot be group/world writable, becausemainrefuses the0775~/.local.The review's cases fail on the previous head,
71f7b3f5. I put its template into this tree.a member name another account also has.SSSD accounts, the ls test atan ACL, and the process-ID test at the ownership refusal.Mutation run. 32 mutants against
provider-home.tsx, under umask0002, with the provider-home tests. 30 are killed.getgidcheck, and every nsswitch, ls and account-file rule.owner !== getuidcheck. It only separates a root-owned group-writable directory, which a test cannot create without root.?. This is an equivalent mutant: any mark besides"",+(or.under GNU) already gives "cannot show".The review's ACL reproductions, on this host. Real ACLs, the host's real
/etc/passwd,/etc/groupand/etc/nsswitch.conf, andHOMEa fixture whose other ancestors are0700:main~/.local0775, no ACL~/.local0755+u:nobody:rwxmarks it as having an ACL)~/.local0755+g:adm:rwxd:u:nobody:rwxonHOME, thenmkdir -p ~/.local/stateunder0002~/.localwith only auser.*xattrAfter
chmod g-w, the mask in case (a) isr-x, and the directory is accepted.Group-writable
TMPDIR. WithTMPDIRset to a0775operator-owned directory, the provider-home tests pass (8/8, under umask0002and022), and so does the OpenRouter contract.Probe cost.
resolveProviderHomewalks the root and then the full path, so Ubuntu's layout runsls8 times per call. That is 2.8 ms per call under Bun and 21 ms under Node.Suites and gates
0002and022.bun test dist-test/test/runtime.test.js --test-name-pattern "^Bun adapter contract:"): 51 pass, 1 skip, 0 fail under umask0002and022. The skip is the deliberatetestWhen(false)registration check.node scripts/run-tests.mjs supporting), under umask022: 933 pass, 0 fail.mainat371c04d5(fix(security): move @grpc/grpc-js to 1.14.5 past GHSA-m9gg-hp2v-232j #1241). The CHANGELOG entry sits above fix(security): move @grpc/grpc-js to 1.14.5 past GHSA-m9gg-hp2v-232j #1241's.pnpm -w format:check,pnpm -w lint,CI=1 ESLINT_PLUGIN_DIFF_COMMIT=origin/main pnpm -w lint:strict:ci,pnpm -w knip,pnpm --filter @ultrafuzz/runtime typecheck,node scripts/docs-check.mjs.40c8855f) into this branch in a throwaway worktree. Onlyruntime.test.tsconflicted. After I took this PR's side there, all 17 provider-home and boundary tests passed under umask0002and022, including fix(runtime)!: keep the Forge guard under group-writable umasks, warn about old cloud runs' Modal storage in clean, and drop the smithers shim #1235's new test, and so did the OpenRouter contract.Risk
nsswitch.confandlstogether show that only the operator (and root) can write to it. Remaining gaps:SupplementaryGroups=, systemd userdb records (accepted along with thesystemdNSS source), or a group password or member in the unreadable/etc/gshadow.docs/config.mdsays so.lstreats a failedlistxattras having no extended attributes. A transientlistxattrerror on a directory the operator owns would pass under uutils./etc/groupkeeps the GID in sessions that started before the removal.mainaccepted. It refuses an ancestor owned by an account other than root or the operator, such as a bind mount owned by an unmapped UID in a container. That owner can rename the provider homes' parents, so the refusal is correct. The fix is to setULTRAFUZZ_PROVIDER_HOME_ROOTelsewhere.main, now with a message: hosts with anotherls, such as BusyBox, or a uutils release before 0.1.0; hosts without/bin/ls, such as NixOS; and hosts whose accounts come from LDAP, SSSD, winbind or extrausers.40c8855f).runtime.test.ts(the OpenRouter test) conflicts. Keep this PR's side: fix(runtime)!: keep the Forge guard under group-writable umasks, warn about old cloud runs' Modal storage in clean, and drop the smithers shim #1235's comment "Every provider-home ancestor must refuse group writes" is no longer true, and its private temporary root would bypass the default root this test covers.provider-home.test.tsandCHANGELOG.mdmerge cleanly. fix(runtime)!: keep the Forge guard under group-writable umasks, warn about old cloud runs' Modal storage in clean, and drop the smithers shim #1235's new test uses a0700fixture root, so no ancestor is group writable, and nothing in this PR changes its outcome.chmod g-w ~/.localbecomes obsolete, except on the hosts listed above that fail closed.ultrafuzz initbefore they plan a new run; planning's usual refusal says so. A run launched earlier executes the adapter copy sealed into its execution snapshot.trustedGitExecutablerefuses any group-writable ancestor, so agitfound only under~/.local/binis refused on an Ubuntu umask.Closes #1236
🤖 Generated with Claude Code
The PR should not merge until the ACL check handles GNU
lsbuilds that cannot detect ACLs.Fix with agent prompt
Summary
The PR permits provider homes beneath group-writable private-group ancestors while adding ownership, name-service, and ACL checks and expanding the tests and documentation.
lsbuild that lacks ACL support.Reviews (2) · Last reviewed commit: "fix(runtime): refuse a provider-home anc..."