Fix dejima-allow and dejima-project, and teach the agent briefing the project scope #73
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: dejima/dejima#73
Reference in New Issue
Block a user
Summary generated by LLM agent
Fixes the five defects in #61, and closes a privilege-scope escalation that the third of them opened up.
The five asks
--projectmeans project-level, not container-level. Container work now happens only when no scope flag was given, or--containerwas passed explicitly.--project/--globalalone never consult Docker — no container listing, no "No running dejima container found", no reachability probe.--project --container <id> <domain>still does both, because both were asked for.Flags first, required positional last.
The old order is removed rather than deprecated; it fails loudly.
dejima-depis in scope by choice — two sibling CLIs driven by the samedejima-project approveand described in the same agent briefing should not disagree about what--projectmeans.The optional path needs one precise rule, because
--project Xis ambiguous on its own:--projectconsumes the next token as its path only if doing so still leaves the command its required trailing positional(s). The obvious simplification — "never consume the last token" — silently sendsdejima-dep list --project ~/pto the current directory, becauselisthas no trailing positional to protect. That case has its own test.--projectwrites the declaration too. It records the approval under~/.config/dejima(which grants) and a declaration in the project's own.dejima/<kind>(committed, grants nothing). Without the second, a domain approved with--projectwas invisible todejima-project status, to the agent briefing, and to everyone else on the project.--globalwrites only the approval — there is no project to declare in.dejima-project statusshows approvals, not just declarations. It now lists the union, with each entry's source and a marker when an approval has no declaration behind it — the case that was invisible. The header labels each store rather than printing a bare path beside a project path.The agent briefing names the project scope. This is the defect that generated the others' symptoms: the briefing offered only
--containerand--global, so an agent asked for a project-specific domain to be added container-wide and globally, and could not name the project-level command. It now gives all three with a decision rule, because a bare flag list would not have changed that choice.Security fix, found by the whole-branch review
Making the CLIs write declarations inverted the trust direction on a path nobody guarded.
<project>/.dejima/is on a bind mount the sandboxed session can write, so a session could pre-plant a symlink there. Three reproductions against the real binaries:.dejima/allowed-domains→ the global approval store turned a--projectapproval into a global one, allowed in every project on the machine, while the output claimed it was declared in the project..dejima/allowed-domains→ the globaldependenciesfile corrupted it, soapproved_entries' refuse-rather-than-proceed contract fired against the global store andbin/dejimacould build a derived image for no project until the file was hand-edited..dejima→ any directory gave an arbitrary host-file append, becausemkdir -pfollows a directory symlink.The chain was closed by this branch: the new briefing tells the agent to have the user run
dejima-allow --project, so a prompt-injected session could plant the symlink and request the command that trips it.declare_entrynow refuses to write when.dejimaor.dejima/<kind>is a symlink or any non-regular file, and the refusal is the open, not a test before it. A check would not have been enough here: the briefing tells the agent to have the user rundejima-allow --projectfrom a terminal outside the sandbox, so the session is normally still live — and it picks the moment, so it can spin a rename loop across any window between a check and the write. The write is therefore done throughpython3(already a hard dependency):O_NOFOLLOW|O_DIRECTORYon.dejima, thenO_NOFOLLOW|O_CREAT|O_APPENDon<kind>relative to that directory fd, with the duplicate check read back through the same fd. A swapped path fails the open instead of redirecting it, and neither path can be swapped for the other's sake afterwards.O_NONBLOCKplus anS_ISREGcheck keeps a planted FIFO from wedging the CLI, and the read-back is capped at 1 MiB —grep -qxFstreamed in constant memory, so reading this session-owned file into a variable would have swapped the symlink for a host OOM (truncate -s 8Gis sparse and costs the session nothing).Both guards were run against a live swapper: the old check-then-write appended to the victim file within a few hundred attempts on both the file and the directory path; the new one never does, and the two tests that demonstrate it are in the suite.
Reads of
.dejima/deliberately have no symlink guard: following a link on a read only lets a session show itself its own files. The asymmetry is intentional, not an oversight.Behaviour changes worth knowing
dejima-project approveexits non-zero when any approval failed. It previously exited 0 while printing that nothing was recorded, soapprove && dejimawalked into a session with none of the approvals. Relevant if you script it.dejima-project approveno longer applies anything to a running session — it records approvals and reports persistence failures without stopping at the first one. The dependency half had no such collector and aborted the whole run underset -e; it does now.dejima-deprejects--container(exit 1) instead of silently ignoring it. Adopting the shared parser had given it a flag it has no concept of, so a user who thought they were scoping to a container silently got project scope.DEJIMA_ALLOW_SKIP_SESSIONis removed. The scope split made it dead — every use passed a scope flag that never reaches its guard. It was never a documented interface.dejima-dep remove --projectwithdraws the approval but leaves the declaration. Editing a file the whole team shares is the project's editorial decision, not a side effect of one machine withdrawing its approval — but it meansdejima-project approvewill offer the entry again, soremovenow says so.dejima-allow --projecthas no remove at all; the README notes the gap.Known limitations, deliberately not fixed
--project <proj>/<subdir>where<subdir>is a symlink creates.dejima/inside the link target. The leaf guards still fire, so the worst case is an attacker-chosen directory gaining one validated entry — no approval-store write, no append into an existing file. Not reachable through the briefing, which only ever emits the workspace root.bin/dejimarefuses a read-write mount overlapping the config root.Testing
bats test/project.bats test/static.bats test/config.bats test/providers.bats→ 409 passing, 0 failures (baseline onorigin/mainwas 349). Whole fast suite includingaudit,proxyandremote→ exit 0, no failures.runtime.batsnot run (needs Docker).bash -nclean on every changed script;shellcheckwas not available in this environment.Each dimension of the declaration guard was mutation-checked rather than assumed: dropping
O_NOFOLLOWfails all five symlink tests and both swap tests; droppingO_NONBLOCK/S_ISREGhangs the FIFO test out totimeout's 124; dropping the read cap fails the oversized-file test while its just-under-the-cap neighbour still passes.Reviewed task by task with an adversarial pass after each, plus a whole-branch review. Defects those caught and fixed here rather than shipped, beyond the symlink escalation above:
set -eloop-abort guard fordejima-project approve; proven by mutation on two commits — deleting the collector left all eight approve tests green.approveprinted "Approved for project, but not applied to a running session" and exited 0 in a case where nothing was written at all.declare_entryfailure aborted past an already-recorded approval, so the grant happened and the user saw only an error that read as "nothing happened".while ... done < <(cmd) || return 1swallowed the failure whencmderrored before printing, sostatusproceeded with exit 0 against a corrupt approval store — defeating a deliberate refuse-rather-than-proceed contract.resolve_ws-style detection treat any snapshotted workspace as having reports.Fixes #61
The atomic rewrite replaced `grep -qxF`, which streams the duplicate check in constant memory, with a plain fh.read() of the whole file. That file is the session-writable one this guard exists for, and a session can make it arbitrarily large for nothing: truncate -s 8G .dejima/allowed-domains # sparse, du reports 0 Then it asks the user to run the command the briefing tells it to ask for, and the host reads 8 GiB into a variable. Measured with the read contained by ulimit -v: a raw MemoryError traceback. Uncontained it is the OOM killer, aimed at whatever else the machine is running. It failed closed - no wrong write, the approval was already recorded - so this was availability rather than privilege, but it was the one dimension of the new code with no bound at all, and a traceback is the wrong register for a file every other refusal here explains. The read now stops at LIMIT + 1 bytes and refuses above LIMIT, inside the same fd. LIMIT + 1 rather than an fstat size check because the file can grow between the fstat and the read. 1 MiB is orders of magnitude above any real declaration - DEJIMA_MAX_DECLARED entries of a domain's 253 bytes is tens of KiB - and that constant is passed in from the shell rather than duplicated, so the two cannot drift. The test plants the 8G sparse file and runs the CLI under ulimit -v, so a regression fails the test rather than the machine it runs on; a just-under-the-cap neighbour proves the cap is a ceiling on a hostile file and not a new way to refuse a large real one. Also narrows the README's "cannot swap either path" to the two leaves the session actually controls - an intermediate component of an operator-supplied --project path is still resolved through, as the residuals already say.