fix(access): central project access gate, fail-closed lookups, file policy and origin check (lr-783ba1)

[clagentic-builder]

TASK: lr-783ba1 (lead). Also delivers lr-877be7, lr-bd22ea, lr-85c259 and lr-543036 items 1, 2 and 4 (item 3 goes to PR-3). Close all of them on merge; close lr-543036 only when both PRs have landed.

What changed (final state of the PR)

Authorization was opt-in per handler and a failed lookup counted as allowed. Now:

• lib/project-access.js: one mode-aware resolver (principalFor, canAccess, isAdmin, filterProjectList, canReadSession, authOf). AUTHENTICATION precedes and is separate from PRINCIPAL resolution (round 3 R4): a missing user is the implicit single-user owner only when the caller passes { authenticated: true }, proved by isRequestAuthed(req) (HTTP), the WS upgrade login check (recorded as ws._clagenticAuthenticated, read back with authOf(ws)) or the local MCP bridge. Without the proof a missing user is nobody in every mode. Given the proof, the daemon mode (users.isMultiUser()) decides: single-user is the implicit owner (admin, every project, every session); multi-user is a stranger and is refused. A lookup that cannot be completed (no lookup wired, unknown slug, lookup error, mode unreadable) is a refusal, administrators included. Missing visibility is private. Worktrees take their parent record.
• Every gate, filter and route uses the resolver and passes the proof it has: lib/project-message-gate.js (first thing handleMessage runs; authorizes the current project and any targetSlug, strips the slug), WS upgrade, /p/ and /api/file, root redirect, palette, projects_updated/info/broadcastAll, hub schedules, hub recent sessions, global CLAUDE.md and shared env (admin-only), session rename/delete/search, file history.
• Round 3 R4 (live leak): appHandler has no login check ahead of palette.handleRequest, and principalFor(null) yielded the implicit owner in single-user mode, so an unauthenticated GET /api/palette/search returned every session title and history snippet. The palette now proves authentication itself (isRequestAuthed) and answers 401 without it in every mode.
• Round 3 R5: lib/project-git.js walks up from the project directory to the nearest .git (directory or worktree file) before the ownership stat, so a project in a repository subdirectory keeps git history in os-users mode; no repository found or unreadable fails closed.
• Round 3 R6: test/require-cache-reset-guard.test.js skips regex literals in its brace matcher and counts a catch body of only empty statements or void 0 as empty. The remaining documented limit: code nested inside a template literal substitution is not parsed.
• The WS upgrade listener answers or destroys the socket on every outcome.
• Git (round 2 R1): no safe.directory override; in os-users mode read-only git runs as the uid that owns the repository, never as the viewer; every call passes -c core.fsmonitor=false -c core.hooksPath=/dev/null -c diff.external=, and --no-ext-diff --no-textconv for diff/show/log; only committed blobs are compared; revisions are hex-only; paths project-relative.
• File policy (lib/project-file-scope.js) shared by the WS fs_* handlers, GET /api/file and the watchers; fileBrowser on every fs_* except fs_unwatch; watches per connection (lib/project-file-watch.js); lib/ws-origin.js origin check; lr-543036 items 1, 2, 4 (buildable vendors, prototype-less client-keyed tables, extension and MCP answers accepted only from the originating socket); item 9 no empty catch around require.resolve in tests.

Docs: docs/guides/architecture.md (Access Control and Input Trust), MODULE_MAP.md.

Tests

Round 3 additions: test/authentication-precedes-principal.test.js (real server, the login gate stood in with a switch; in single-user and multi-user mode an unauthenticated palette search and recent list, /p//api/file, /p//, the root, /info and the WebSocket upgrade are all refused; the local MCP bridge works unauthenticated; an authenticated single-user owner is served the palette, file, root, info and socket; a login naming no user in multi-user mode is nobody). test/project-access-units.test.js (proof matrix: only the boolean true counts; authOf; gate refuses an unproven single-user connection). test/project-filesystem-policy.test.js (unproven single-user socket refused on every settings, fs and git message; repository subdirectory, nested .git file, no repository, and a root-only real setuid run in a subdirectory). test/require-cache-reset-guard.test.js (regex literals with quotes and braces, empty-statement and void 0 catch bodies, division kept as division).

Fixture edits to existing tests (the contract they encode changed): fakeSocket in project-filesystem-policy now marks the socket authenticated like the upgrade does; the single-user gate test and the project-access-units single-user cases pass the proof; the hub-recent-sessions wiring regex expects authOf(ws).

Demonstrated failure on 9e63a94 lib with the new tests present: single-user palette returned 200 instead of 401 (unauthenticated), the proof matrix, authOf, unproven gate, unproven socket and findRepositoryRoot tests failed; with the fix they pass.

Failing-test-ID set

• Local npm test at this head: exactly two, lr-29f9 (5) ceiling query at MAX_CONCURRENT and lr-2d91 (4) getMemoryStats returns activeLiveCount, both in test/sdk-bridge-slot-counter.test.js. They fail identically on 9e63a94 and on clean origin/main here because this host exports CLAGENTIC_CONSOLE_MAX_CONCURRENT_SESSIONS=50, which wins over the legacy name the test sets; they were green in CI at 9e63a94.
• CI test check-run at head bc18c87 (check-run 114363426693): concluded success; failing set empty.

CREW_SOP section 6 compliance record (access gate, upgrade check, message gate, git runner)

Shape	Test
Unauthenticated request, single-user and multi-user mode (refused everywhere)	authentication-precedes-principal
Authenticated single-user owner (the live deployment)	authentication-precedes-principal, single-user-null-user, project-filesystem-policy, project-access-units
Single-user mode, owner as an admin account with a login cookie	access-gate-integration
Multi-user without os-users	access-gate-integration, project-filesystem-policy
Multi-user with os-users, git as the repository owner, repository subdirectory	project-filesystem-policy: owner uid from the repo root (recorded exec), fail-closed with no repository, hostile repo inert, real setuid run (root only) in the root and in a subdirectory
Worktree slugs	access-gate-integration, single-user-null-user, project-access-units
WS with and without targetSlug	access-gate-integration, single-user-null-user, project-access-units
Local MCP bridge (isMcpBridgeLocal)	authentication-precedes-principal, access-gate-integration, single-user-null-user
Origin shapes	ws-origin, access-gate-integration

Pending post-merge real run: os-users with real Linux accounts, the live Caddy front end with trustedProxy, and the Chrome extension origin once the operator lists it.

Notes

• class sweep (R4): git grep -nE principalFor|isRequestAuthed|isMcpBridgeLocal -- lib and git grep -nE projectAccess -- lib bin: every non-resolver call site now passes a proof (palette: isRequestAuthed(req); /p/, root and WS upgrade: the login gate that precedes them; connections: authOf(ws); requests: authOf(req)). Two broadcasts with no connection (getHubSchedules(null) in project-loop and project-sessions fallbacks) pass no proof and so send an empty list.
• No new dependency, no new LLM call path.
• Operator-visible: optional allowedOrigins in daemon.json; in os-users mode git history runs as the repository owner.

Not folded (followups)

• The server login gate still requires a login cookie in every mode (single-user PIN mode was removed); a null-user connection reaches the project layer only where the gate is stood in (tests) or on the local MCP bridge.
• Project-management messages that carry a slug of another project (set_project_title, remove_project, schedule_move, ...) are not run through the gate.
• Pre-existing admin-only checks in project-sessions.js still deny a null user.
• safeClaudePath lets any user with fileBrowser read under the daemon home .claude/ in non-os-users multi-user mode.
• Any authenticated client can claim to be the browser extension with browser_tab_list.

🤖 Generated with Claude Code

[clagentic-security]

BOBBIE — blocking (3 finding(s), 1 dropped)

• lib/project-file-watch.js:47 [bobbie.uncat.1] (praise) Watches are now scoped per connection, authorization is re-checked on each event, and reads run as the connection's user. This closes the old broadcast of file contents to every client on the project.
• lib/project-git.js:38 [bobbie.uncat.1] (blocking) safe.directory override makes git trust another user's .git/config; diff.external or textconv in it then runs as the viewing user's uid during git show/diff.
  failure sequence: User A in os-users mode writes diff textconv driver or core.fsmonitor into project .git/config plus a .gitattributes entry -> user B opens a file diff or history, runGit runs git show or diff as B with safe.directory forced to cwd so git trusts A's config -> A's command executes as B
• lib/project-git.js:11 [bobbie.sast.2] (praise) Validating revisions as hex object names before they reach git argv closes option injection via hash/hash2 (e.g. --output=...). Paths are also passed after --.

Dropped candidates (1):

• lib/mcp-local.js:96 [bobbie.uncat.1] semgrep javascript.lang.security.detect-child-process.detect-child-process: Detected calls to child_process from a function argument command. This could lead to a command injection if the input i... (reason: spawn of operator-configured MCP server command from the local config file, not client-supplied; no attacker-controlled path in this PR)

{"reviewer": "bobbie", "review_status": "blocking", "head_sha": "1059eb817b5b8565a6f3cdf6bdc8c83e1006fd14", "pr_number": 430, "failure_sequences": [{"file": "lib/project-git.js", "line": 38, "rule_id": "bobbie.uncat.1", "failure_sequence": "User A in os-users mode writes diff textconv driver or core.fsmonitor into project .git/config plus a .gitattributes entry -> user B opens a file diff or history, runGit runs git show or diff as B with safe.directory forced to cwd so git trusts A's config -> A's command executes as B"}], "dropped": [{"file": "lib/mcp-local.js", "line": 96, "rule_id": "bobbie.uncat.1", "message": "semgrep javascript.lang.security.detect-child-process.detect-child-process: Detected calls to child_process from a function argument `command`. This could lead to a command injection if the input i...", "reason": "spawn of operator-configured MCP server command from the local config file, not client-supplied; no attacker-controlled path in this PR"}]}

[clagentic-reviewer]

PEACHES — blocking (16 finding(s), 2 dropped)

• lib/project-filesystem.js:67 [amos.path-choice.1] (blocking) Admin gate denies when ws._clagenticUser is absent. The removed settings gate allowed that state ('if (ws._clagenticUser)'), so global CLAUDE.md and shared env now fail with no user.
  failure sequence: no-login mode with ws._clagenticUser unset, client sends read_global_claude_md or get_shared_env, admin gate returns Admin access required where the removed settings gate served the request, so global CLAUDE.md and shared env reads and writes fail for every client in that mode
• lib/project-http.js:288 [amos.path-choice.1] (blocking) /api/file now returns 403 'Authentication required' when req._clagenticUser is unset. The old handler had no user requirement, so image previews fail in no-login mode.
  failure sequence: no-login mode, fs_read of a png returns imageUrl api/file, req._clagenticUser is unset, /api/file answers 403 Authentication required, every image preview in the file browser breaks where it was served before
• lib/project-sessions.js:671 [amos.path-choice.4] (nit) search_sessions now coerces non-string msg.query to "", but the sibling search_session_content handler two lines below still passes raw msg.query to searchSessionContent and echoes it back.
• lib/server.js:801 [amos.path-choice.1] (blocking) A null user is now denied (!httpUser || → 302). The old code explicitly allowed when getMultiUserFromReq returned null. Root redirect lines 657/664 now also require reqUser.
  failure sequence: daemon where getMultiUserFromReq returns null, client GETs /p/slug, the !httpUser branch 302s to root, root handler requires reqUser so targetSlug stays unset and no project is reachable over HTTP
• lib/server.js:1025 [amos.path-choice.1] (blocking) wsUser.id is dereferenced unguarded. The removed code gated on wsUser &&, so a null wsAuthedUser now throws inside the upgrade handler instead of proceeding.
  failure sequence: WS upgrade where wsAuthedUser is null, projectAccess.canAccess(wsUser.id) throws TypeError inside the upgrade listener, socket is neither answered nor destroyed and the uncaught throw can take down the daemon
• lib/server-palette.js:34 [amos.path-choice.1] (blocking) paletteUser.id is dereferenced without a null check. The removed code guarded with if (paletteUser && ...) and if (paletteUser). Same site at line 69.
  failure sequence: GET /api/palette/search where getMultiUserFromReq returns null, accessibleProject(paletteUser.id) throws TypeError at line 34 and line 69, the search fails with no results and the throw escapes the request handler
• lib/server.js:1215 [amos.path-choice.1] (nit) filterProjectList(null, ...) returns no projects ("no user, no projects"). Clients with no _clagenticUser, previously shown the full list, now get an empty list here and in broadcastProjectsUpdated.
• test/access-harness.js:188 [amos.code-craft.6] (nit) stop() catches and discards every error from relay.destroyAll() (and ws.terminate() on line 187). A teardown regression, such as a failed or rejected destroy, passes silently instead of failing the...
• test/permission-grant-persist-lr-8b2e.test.js:117 [amos.code-craft.5] (nit) Existing deepEqual assertions are wrapped in plain() to pass after the null-prototype change. Note that Object.assign treats an own __proto__ key as a setter, so plain() would hide such a key.
• test/project-file-watch-per-connection.test.js:140 [amos.code-craft.4] (nit) Vacuous check: no file or dir is changed after the escape attempts, so a watch that wrongly started on ../etc/hostname or '..' would also leave ws.sent empty. Trigger a change, then assert.
• test/project-connection-hydrate-session-model-lr-041af8.test.js:46 [amos.code-craft.6] (praise) Drops the catch-all around require.resolve, so a stale module path in REQUIRE_CACHE_MODULES now fails loudly instead of leaving a cached module that silently skews the test.
• test/prototype-free-tables.test.js:34 [amos.path-choice.4] (nit) The sweep guard only catches an empty {}. A populated literal (handlers = { a: f }) or a name: {} property passes, so a plain-object table can regress unflagged.
• test/prompt-registry.test.js:121 [amos.code-craft.5] (nit) Existing asserts were loosened to Object.keys(...) so they pass with null-prototype tables (also at 401-402). They no longer check prototype; assert Object.getPrototypeOf(...) === null too.
• test/project-loop-message-lr-4a9c.test.js:89 [amos.code-craft.6] (praise) Removes the catch-all around require.cache deletion here and in the sibling loop tests. A failed resolve now surfaces instead of being silently swallowed.
• test/require-cache-reset-guard.test.js:17 [amos.code-craft.6] (nit) The guard's [^{}]* try-body means any try block with nested braces, e.g. if (x) { ... }, escapes detection. readdirSync (line 20) also skips test subdirectories, so empty catches there go unfla...
• test/require-cache-reset-guard.test.js:73 [amos.code-craft.6] (nit) The list scan only matches double-quoted "../" arrays chained directly to .forEach. Single-quoted entries, "./" paths, or a list held in a variable are never resolve-checked, unlike the literal sca...

Dropped candidates (2):

• test/access-gate-integration.test.js:300 [amos.code-craft.4] Test title says 'the owner of a session, and an administrator, can act on it' but only bob (owner) is exercised; the admin path on another user's private session has no assertion. (reason: test title claims admin path coverage, but code-craft.4 covers regression tests for bug fixes and no product defect is shown)
• test/hostile-keys-stores.test.js:138 [amos.code-craft.13] Test reaches into the MCP SDK's private instance._registeredTools.t to get the handler. A SDK internal rename breaks the test; prefer the bridge's exported surface to invoke the tool. (reason: code-craft.13 covers peer-module internals, the reached-into object is the external MCP SDK, and the stated outcome is test fragility rather than a defect)

{"reviewer": "peaches", "review_status": "blocking", "head_sha": "1059eb817b5b8565a6f3cdf6bdc8c83e1006fd14", "pr_number": 430, "failure_sequences": [{"file": "lib/project-filesystem.js", "line": 67, "rule_id": "amos.path-choice.1", "failure_sequence": "no-login mode with ws._clagenticUser unset, client sends read_global_claude_md or get_shared_env, admin gate returns Admin access required where the removed settings gate served the request, so global CLAUDE.md and shared env reads and writes fail for every client in that mode"}, {"file": "lib/project-http.js", "line": 288, "rule_id": "amos.path-choice.1", "failure_sequence": "no-login mode, fs_read of a png returns imageUrl api/file, req._clagenticUser is unset, /api/file answers 403 Authentication required, every image preview in the file browser breaks where it was served before"}, {"file": "lib/server.js", "line": 801, "rule_id": "amos.path-choice.1", "failure_sequence": "daemon where getMultiUserFromReq returns null, client GETs /p/slug, the !httpUser branch 302s to root, root handler requires reqUser so targetSlug stays unset and no project is reachable over HTTP"}, {"file": "lib/server.js", "line": 1025, "rule_id": "amos.path-choice.1", "failure_sequence": "WS upgrade where wsAuthedUser is null, projectAccess.canAccess(wsUser.id) throws TypeError inside the upgrade listener, socket is neither answered nor destroyed and the uncaught throw can take down the daemon"}, {"file": "lib/server-palette.js", "line": 34, "rule_id": "amos.path-choice.1", "failure_sequence": "GET /api/palette/search where getMultiUserFromReq returns null, accessibleProject(paletteUser.id) throws TypeError at line 34 and line 69, the search fails with no results and the throw escapes the request handler"}], "dropped": [{"file": "test/access-gate-integration.test.js", "line": 300, "rule_id": "amos.code-craft.4", "message": "Test title says 'the owner of a session, and an administrator, can act on it' but only bob (owner) is exercised; the admin path on another user's private session has no assertion.", "reason": "test title claims admin path coverage, but code-craft.4 covers regression tests for bug fixes and no product defect is shown"}, {"file": "test/hostile-keys-stores.test.js", "line": 138, "rule_id": "amos.code-craft.13", "message": "Test reaches into the MCP SDK's private `instance._registeredTools.t` to get the handler. A SDK internal rename breaks the test; prefer the bridge's exported surface to invoke the tool.", "reason": "code-craft.13 covers peer-module internals, the reached-into object is the external MCP SDK, and the stated outcome is test fragility rather than a defect"}]}

[clagentic-reviewer]

PEACHES — clean (3 finding(s))

• lib/project-git.js:46 [amos.path-choice.1] (nit) repositoryOwner stats only /.git, so in os-users mode a project whose cwd is a repo subdirectory now throws and file history drops all git commits; it previously worked as the caller.
• test/require-cache-reset-guard.test.js:51 [amos.code-craft.6] (nit) closingBrace does not recognise regex literals, so a quote or brace inside one (/"/, /}/) throws the depth count off. The try or catch bounds then come out wrong and an empty catch can escape the g...
• test/require-cache-reset-guard.test.js:66 [amos.code-craft.6] (nit) isOnlyComments only strips comments, so a catch body holding just ; or void 0; counts as non-empty. That swallows the failure just as well but is not flagged.

{"reviewer": "peaches", "review_status": "clean", "head_sha": "9e63a94d1b5f150c1f64b5490344bdf8eb961b6a", "pr_number": 430}

[clagentic-security]

BOBBIE — clean (1 finding(s), 1 dropped)

• lib/server-palette.js:28 [bobbie.uncat.1] (nit) In single-user mode principalFor(null) is the implicit owner for any request, so this 401 check no longer verifies login. Unlike the WS path, it lacks an isRequestAuthed(req) check.

Dropped candidates (1):

• lib/mcp-local.js:96 [bobbie.uncat.1] semgrep javascript.lang.security.detect-child-process.detect-child-process: Detected calls to child_process from a function argument command. This could lead to a command injection if the input i... (reason: spawn command comes from the operator-owned MCP server config file, not a request; code is pre-existing and not a command-injection path introduced by this PR)

{"reviewer": "bobbie", "review_status": "clean", "head_sha": "9e63a94d1b5f150c1f64b5490344bdf8eb961b6a", "pr_number": 430, "dropped": [{"file": "lib/mcp-local.js", "line": 96, "rule_id": "bobbie.uncat.1", "message": "semgrep javascript.lang.security.detect-child-process.detect-child-process: Detected calls to child_process from a function argument `command`. This could lead to a command injection if the input i...", "reason": "spawn command comes from the operator-owned MCP server config file, not a request; code is pre-existing and not a command-injection path introduced by this PR"}]}

[clagentic-reviewer]

PEACHES — clean (2 finding(s), 1 dropped)

• test/hub-recent-sessions-access-filter-lr-c4da07.test.js:73 [amos.code-craft.4] (nit) Only a source-text regex covers the new login-proof threading in hub_recent_sessions_list. No behavioral test shows an unproven single-user socket gets no cross-project sessions there.
• test/require-cache-reset-guard.test.js:38 [amos.code-craft.6] (nit) REGEX_AFTER_CHAR has + and -, so the / in i++ / 2 or n-- / 2 is read as a regex start. The brace count or catch bounds then go wrong (-1), and an empty catch in that try is skipped silently.

Dropped candidates (1):

• lib/project-message-gate.js:26 [amos.code-craft.3] var DENIED ="Project not found or access denied"; drops the space after = that the previous line had; restore project style (run the linter). (reason: Style a linter owns, out of scope per the rulebook; a spacing nit with no defect shown)

{"reviewer": "peaches", "review_status": "clean", "head_sha": "bc18c87b0f27c14caa9f8df493d4f1ed28a63c07", "pr_number": 430, "dropped": [{"file": "lib/project-message-gate.js", "line": 26, "rule_id": "amos.code-craft.3", "message": "`var DENIED =\"Project not found or access denied\";` drops the space after `=` that the previous line had; restore project style (run the linter).", "reason": "Style a linter owns, out of scope per the rulebook; a spacing nit with no defect shown"}]}

[clagentic-security]

BOBBIE — clean (0 finding(s), 1 dropped)

Dropped candidates (1):

• lib/mcp-local.js:96 [bobbie.uncat.1] semgrep javascript.lang.security.detect-child-process.detect-child-process: Detected calls to child_process from a function argument command. This could lead to a command injection if the input i... (reason: spawn command and args come from the operator's own MCP config file and its include files, not from any request or client input; launching configured MCP servers is the feature, no injectable path)

{"reviewer": "bobbie", "review_status": "clean", "head_sha": "bc18c87b0f27c14caa9f8df493d4f1ed28a63c07", "pr_number": 430, "dropped": [{"file": "lib/mcp-local.js", "line": 96, "rule_id": "bobbie.uncat.1", "message": "semgrep javascript.lang.security.detect-child-process.detect-child-process: Detected calls to child_process from a function argument `command`. This could lead to a command injection if the input i...", "reason": "spawn command and args come from the operator's own MCP config file and its include files, not from any request or client input; launching configured MCP servers is the feature, no injectable path"}]}

[clagentic-merger]

Merged via clagentic-loadout v0.2.0

Field	Value
Gated HEAD SHA	bc18c87b0f27c14caa9f8df493d4f1ed28a63c07
Merged SHA	bc18c87b0f27c14caa9f8df493d4f1ed28a63c07
Reviews	clagentic-reviewer[bot], clagentic-security[bot]
CI status	no-runner-by-design (0 commit-status entries at HEAD)
task_id	lr-783ba1