indifferentketchup
b52c5df705
When the agent needed context from another repo, pathGuard rejected every read
with no recovery path. This batch adds a reactive request_read_access flow:
pathGuard's error now hints at the tool, the model emits a structured request,
the inference loop pauses (same mechanism as ask_user_input), the user picks
Allow/Deny via inline chips, and subsequent reads under the granted root succeed
for the rest of the session.
Schema: sessions.allowed_read_paths TEXT[] NOT NULL DEFAULT ARRAY[]::TEXT[]
(idempotent ADD COLUMN IF NOT EXISTS).
Grant unit (design D1): nearest registered projects.path ancestor →
nearest repo-shaped ancestor (.git/ / package.json / go.mod / Cargo.toml)
under PROJECT_ROOT_WHITELIST → else refuse. grant_resolver.ts walks
ancestors with a per-iteration whitelist invariant check so symlinked
input can't escape the whitelist mid-walk (Sam's checkpoint-1 ask).
Path-guard: optional extraRoots arg threaded from session.allowed_read_paths
through executeToolCall to view_file / list_dir / grep / find_files. The
ToolDef.execute signature gets an optional third param; non-FS tools
ignore it. view_file re-anchors the secret-guard check on basename(real)
whenever a relative path starts with "../" so .env / id_rsa* etc. still
deny across grant roots.
Endpoint: POST /api/chats/:id/grant_read_access mirrors /answer_user_input.
On 'allow' it re-resolves the grant root (state may have changed since
prompt — auto-falls to denial reason text on failure, not 500), array_appends
to sessions.allowed_read_paths with in-memory dedup, then publishes
tool_result + session_updated frames and enqueues the next assistant turn.
PATCH /api/sessions/:id allowed_read_paths supports revocation only. Zod
refines absolute + no traversal markers; runtime findUnauthorizedAdditions
guard rejects any entry not already present in the row, so a malicious
curl -X PATCH -d '{"allowed_read_paths":["/etc"]}' returns 400 instead of
bypassing the grant flow (Sam's compliance-review action item).
Frontend: RequestReadAccessCard renders pending (path + reason + Allow/Deny)
and answered (granted/denied summary with the resolved root) variants;
MessageList.flatten/group special-cases the tool name; SettingsPane adds a
per-session grants list with per-row revoke that PATCHes the shortened
array.
Tests: 11 grant_resolver, 8 path_guard, 8 sessions PATCH subset, including
explicit cases for symlink escape mid-walk, walk-bound termination at
whitelist root, /etc bypass attempt via PATCH, and nearest-project
disambiguation. 292 total server tests green.
Pairs with v1.13.16-xml-parser — the model now self-recovers from both
a wrong tool name AND from a refused path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
71 lines
2.9 KiB
TypeScript
71 lines
2.9 KiB
TypeScript
// v1.13.17-cross-repo-reads: PATCH /api/sessions/:id allowed_read_paths
|
|
// subset enforcement. Sam flagged in the compliance review that without a
|
|
// runtime subset check, a malicious client could POST
|
|
// {"allowed_read_paths":["/etc"]}
|
|
// and bypass the user-consent grant flow entirely. The findUnauthorizedAdditions
|
|
// helper is the guard; tests pin its behavior so a regression in the helper
|
|
// or its callsite (PATCH handler in sessions.ts) trips CI before prod.
|
|
|
|
import { describe, it, expect } from 'vitest';
|
|
import { findUnauthorizedAdditions } from '../sessions.js';
|
|
|
|
describe('findUnauthorizedAdditions — PATCH allowed_read_paths subset guard', () => {
|
|
it('returns no extras when requested is empty (full revoke)', () => {
|
|
expect(findUnauthorizedAdditions(['/opt/forks/foo'], [])).toEqual([]);
|
|
});
|
|
|
|
it('returns no extras when requested is a strict subset (single revoke)', () => {
|
|
expect(
|
|
findUnauthorizedAdditions(['/opt/forks/foo', '/opt/forks/bar'], ['/opt/forks/foo']),
|
|
).toEqual([]);
|
|
});
|
|
|
|
it('returns no extras when requested equals prior (no-op PATCH)', () => {
|
|
expect(
|
|
findUnauthorizedAdditions(['/opt/forks/foo', '/opt/forks/bar'], [
|
|
'/opt/forks/foo',
|
|
'/opt/forks/bar',
|
|
]),
|
|
).toEqual([]);
|
|
});
|
|
|
|
it('flags an unauthorized addition when prior is empty', () => {
|
|
// The /etc bypass attempt — Sam's specific concern from the compliance
|
|
// review. Without this guard, the PATCH would have written /etc directly.
|
|
expect(findUnauthorizedAdditions([], ['/etc'])).toEqual(['/etc']);
|
|
});
|
|
|
|
it('flags a single unauthorized addition mixed in with valid revokes', () => {
|
|
// The attacker still tries to be sneaky: keep one legit entry, drop
|
|
// another, slip in a new one. The guard catches the addition regardless
|
|
// of how the rest of the array shrinks.
|
|
expect(
|
|
findUnauthorizedAdditions(['/opt/forks/foo', '/opt/forks/bar'], [
|
|
'/opt/forks/foo',
|
|
'/var/secrets',
|
|
]),
|
|
).toEqual(['/var/secrets']);
|
|
});
|
|
|
|
it('flags every unauthorized addition when there are multiple', () => {
|
|
expect(
|
|
findUnauthorizedAdditions(['/opt/forks/foo'], ['/opt/forks/foo', '/etc', '/root']),
|
|
).toEqual(['/etc', '/root']);
|
|
});
|
|
|
|
it('treats requested duplicates correctly (each occurrence checked)', () => {
|
|
// If the requested array has duplicates of an unauthorized entry, the
|
|
// guard surfaces each one. (A frontend would never send duplicates, but
|
|
// the guard's contract shouldn't assume that.)
|
|
expect(findUnauthorizedAdditions([], ['/etc', '/etc'])).toEqual(['/etc', '/etc']);
|
|
});
|
|
|
|
it('does not flag entries present in prior even if requested has duplicates', () => {
|
|
// Duplicate of an authorized entry passes — the membership check is by
|
|
// value, not by index. Settled by Set.has semantics.
|
|
expect(
|
|
findUnauthorizedAdditions(['/opt/forks/foo'], ['/opt/forks/foo', '/opt/forks/foo']),
|
|
).toEqual([]);
|
|
});
|
|
});
|