Skip to content

fix(antigravity): refuse a read reservation that names no absolute violations path - #1824

Open
joelmitz wants to merge 1 commit into
fujibee:mainfrom
joelmitz:fix/read-guard-require-absolute-violations
Open

joelmitz wants to merge 1 commit into
fujibee:mainfrom
joelmitz:fix/read-guard-require-absolute-violations

Conversation

@joelmitz

Copy link
Copy Markdown
Contributor

Summary

bridge-read-guard.mjs check appends a read-denied row to reservation.violations and takes ${reservation.violations}.lock on every refusal. If the reservation has no violations field, or a relative one, those paths are the literal string undefined / undefined.lock (or a file relative to the caller's cwd), so the guard writes stray files into whatever directory it was run from.

The existing test the mjs read-guard: a stale reservation is judged on every POSIX host does exactly this: its read-reservation.fixture__worker.json fixture has no violations, so running it leaves undefined and undefined.lock in the repository root.

Reproduction

On main (ca587b3f):

$ bats -f "a stale reservation is judged on every POSIX host" tests/test_antigravity_transport.bats
1..1
ok 1 the mjs read-guard: a stale reservation is judged on every POSIX host
$ git status --short
?? undefined
?? undefined.lock
$ cat undefined
{"event":"read-denied","pid":1}

undefined is 32 bytes (that one row), undefined.lock is empty. Observed on Linux (Node 22). I did not run it on macOS or Windows.

Cause

check reads the reservation and, when the caller is not authorized, writes the refusal to reservation.violations. Nothing checks first that the field is present and absolute. The two writers of a reservation in this repo (antigravity-bridge.mjs and the TUI supervisor) record an absolute path, so I do not expect this in normal operation; the guard just is not defensive about it, and the test fixture above is a reservation that omits it.

Change

  • scripts/drivers/types/antigravity/bridge-read-guard.mjs: right after reading the reservation, throw if violations is not a string holding an absolute path. The existing blanket catch turns that into the existing refusal (exit 13, nothing written), so the fail-closed contract and the exit code callers branch on are unchanged.
  • tests/test_antigravity_transport.bats:
    • new case a reservation without an absolute violations path refuses and writes nothing: runs the guard from an empty directory with (a) no violations and (b) a relative violations, and asserts exit 13 and that the directory is still empty.
    • the stale-reservation fixture now names a violations sidecar like a real reservation, and its guard call gets </dev/null (the guard reads stdin before it decides, so the call blocks when stdin is an open pipe).

Tests

  • The new case fails on main without the source change ([ -z "$(ls -A "$cwd")" ] fails because undefined was written) and passes with it.
  • bats tests/test_antigravity_transport.bats: 14 passed, 2 skipped (16 cases), exit 0.
  • Full suite on Linux, bats --jobs 4 tests/ (3036 tests): 3029 ok (33 skipped), 7 not ok. Six of the seven also fail on unmodified main (ca587b3f) when those files are run there: test_agmsgd_daemon_sh (Node fallback), test_node_resolve (version-manager node), test_registry_lock (mkdir error text), test_remote set-endpoint feat(remote): set-endpoint moves a connected team's address, identity-checked #739 P1-1, test_terminal_registry set -u canary, test_watch --max-seconds. The seventh, test_sync_cipher "bulk seal fan-out, worker failure, and per-request errors", failed once in the parallel run and passed in 3 of 3 standalone runs on this branch; I have not determined why, and I did not run the full suite on main for comparison.
  • node --test tests/antigravity_bridge.test.mjs: 17 of 18 pass, exit 1. The failure, a SIGTERM exit of verify while idle is treated as a normal stop (a wait timeout), is identical on unmodified main (ca587b3f), so it is not caused by this change. I have not determined its cause.
  • No stray undefined / undefined.lock is left in the working tree after the full run (git status clean).

…olations path

bridge-read-guard.mjs check appends to reservation.violations and takes
`${violations}.lock` on every refusal. A reservation with no violations
field, or a relative one, made those paths the literal `undefined` /
`undefined.lock` (or a file relative to the caller's cwd), so the guard
wrote stray files into whatever directory it ran from.

Refuse such a reservation (exit 13, unchanged) before anything is
written. Add a regression test, and name the violations sidecar in the
existing stale-reservation fixture, which was the source of the stray
files when the suite ran.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant