J3: optional off-box audit event sink - #21
Merged
Merged
Conversation
`audit.log` is written by the same process that makes the change, to a local file, so the box that made an unauthorised change is the box that can edit the only record of it. `VPCOPILOT_AUDIT_SINK` ships each entry to a collector as it is written — https:// (POST), syslog:// (RFC 3164 over UDP or a unix datagram socket), stdout, or off — with `VPCOPILOT_AUDIT_SINK_TOKEN` for bearer auth. Hooked inside `audit.record`, the one chokepoint, so all 30 call sites and all three surfaces (CLI, console, MCP) inherit it with no call-site change and no new `record()` parameter. No new dependency. The local log stays authoritative: the line is serialized ONCE and handed to both destinations, so the shipped copy is byte-identical to the line on disk, and a failed local write delivers nothing. Delivery is fail-soft — a dead collector costs one stderr warning and one timeout, then a 60s cooldown, so an apply writing several entries is never stalled by their sum. Misconfigured never renders as clean: an unusable value is reported with its reason on every surface, a 3xx is a failure (the body was never re-sent, and following it would forward the bearer token to an unconfigured host), and UDP reports `sent, unconfirmed` because a datagram to a dead port succeeds at the send call. `vpcopilot audit-sink [--send]` and GET/POST /api/audit-sink are the check; both new keys are on the ⚙ Setup page. Docs in AUDIT.md §10 and USAGE.md. Also fixed here, all pre-existing and called out rather than slipped in: - runmeta._save named its temp file by PID only, so two threads writing a fresh out dir shared it and the loser of the os.replace got FileNotFoundError out of `audit.record` — a change already made to a load balancer that could not be recorded. Reproduced 12/12 with 8 threads and no sink configured. - runmeta.run_id minting was an unguarded read-modify-write, so those same concurrent first writes produced eight different run_ids — seven audit entries carrying a join key run.json does not contain. - The console's `.bad` CSS class was used to mark failures in the H2 dependency preview and defined nowhere, so "could not check" rendered as ordinary text. - tests/conftest.py now clears the sink vars: with one exported, a full run shipped 267 fabricated audit records to the developer's real collector. 854 tests (was 785), coverage 80%, ruff clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the J3 roadmap item.
audit.logis written by the same process that makes the change, to a local file — so the box that made an unauthorised change is the box that can edit the only record of it. This ships each entry to a collector as it is written, so the local file stops being the only copy.Hooked inside
audit.record— the one chokepoint — so all 30 call sites and all three surfaces (CLI, console, MCP) inherit it with no call-site change and no newrecord()parameter (**detailswallows unknown kwargs andjson.dumpshas nodefault=, so a straylog=would be serialized into the entry or raise after the LB was already mutated). No new dependency: httpx is already a base dep, syslog is a raw socket.Acceptance, as met
audit-backfillagainst a black-holed sink exited 0, wrote its sidecar and its audit entry, and emitted exactly one warning.envand the ⚙ Setup page/api/configin a running console and watching it take effect with no restartaudit.logstays authoritative"Logs one warning" is a design constraint, not a log line: a hung collector at a 5 s timeout times the several entries one apply writes is a stall the criterion plainly forbids, so a failure starts a 60 s cooldown — one timeout and one warning per outage, not per entry. The warning goes to stderr, never stdout, so it cannot corrupt the MCP protocol stream even where K1's stdout swap is not in force.
Refusing to guess, applied to a transport
A run succeeds whether or not the sink works, so a misconfigured sink would otherwise be invisible. Every "not delivering" state is distinguishable from "delivering":
Locationto an unconfigured host is how a credential travels;sent, unconfirmed, because a datagram to a port with nothing bound succeeds at the send call. Same three-state honesty asexport --verify'spresent-unverified.Run for real
content-type, bearer token, and a body byte-identical to the on-disk line. Driven end to end by a realvpcopilot audit-backfill.<134>(local0.info), space-padded day, hostname,vpcopilot[pid], JSON payload intact.drift_detectedwith a 60-field diff measured 5,481 bytes — 2.7× the macOS/var/run/syslogSO_SNDBUFof 2048, which refuses more withEMSGSIZE. Nothing is hardcoded; the kernel sets the limit and only a refusal triggers a reduced envelope. A 16,139-byte entry was refused on UDP and a 363-byte valid-JSON envelope arrived naming the action,finding_id, true byte count and where the full record lives, while the on-disk entry kept all 120 changes.local0.infoby default, and the control proves it is the box and not the frame —/usr/bin/logger -p local0.infoalso produces zero hits.Adversarial review
35 raised across six failure dimensions; 4 confirmed after independent skeptics tried to refute each. Every one was the same shape — not delivering, rendering as delivering:
urlsplitraises on a dropped IPv6 bracket (https://[2001:db8::1/x) — the typo this item's own IPv6 support invites. Unguarded that became a CLI traceback and a console 500, whileemit's catch-all swallowed it with no warning and nolast_error.sent,delivered 1, exit 0, redirect target never contacted.sentover UDP meant "handed to the kernel" —audit-sink --sendagainst a dead collector printedsentand exited 0.apply_waf/retire/rollback_failedentries, indistinguishable at the collector from real changes. Now cleared structurally inconftest.py; verified with a counter (1 deliberate record registers, the full suite adds zero).Five more were found by self-review before the reviewers reported, including a password in the sink URL being printed on every surface (
urlsplitputs userinfo innetloc) and an IPv6 collector being unreachable (hardcodedAF_INET).Pre-existing fixes, called out rather than slipped in
None is caused by this change; the first is a raise inside
audit.record, which no audit-integrity item should ship past.runmeta._savenamed its temp file by PID only, so two threads writing a fresh out dir shared it and the loser of theos.replacegotFileNotFoundErrorout ofaudit.record— a change already made to a load balancer that could not be recorded. Reproduced 12 times out of 12 with 8 threads and no sink configured at all. The console starts every apply on its own daemon thread with no job lock.runmeta.run_idminting was an unguarded read-modify-write, so those same concurrent first writes produced eight different run_ids — seven audit entries carrying a join keyrun.jsondoes not contain. Now thread-atomic; single-threaded behaviour byte-identical, and the remaining cross-process case is stated rather than pretended away..badCSS class was used and never defined, so the H2 dependency preview's "could not check" and manifest-parse errors rendered as ordinary body text — a warning styled like content, in the preview built to prevent exactly that. Found while validating the Setup card; the fix repairs three pre-existing call sites.No regressions
audit.record's output is unchanged — no new key, no reordering, no envelope — pinned by a test comparing an entry written with and without a sink. 854 tests (was 785), coverage 79% → 80%, ruff clean, suite run the way CI runs it.🤖 Generated with Claude Code