Skip to content

relay: audit every request line on a kept connection - #197

Merged
CMGS merged 2 commits into
mainfrom
relay/audit-lines
Sep 16, 2026
Merged

CMGS merged 2 commits into
mainfrom
relay/audit-lines

Conversation

@CMGS

@CMGS CMGS commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Second step of #195, the sandboxd side of a persistent data-plane connection; independent of #196 in code, motivated by it.

What changes

The audit tee in sandboxd/server/relay.go recorded the first client line and then degraded to a pass-through, on the premise that one connection carried one RPC. With silkd serving RPCs back to back on one connection (#196) it now scans every client line:

  • request frames are recorded, as before;
  • the input frames of the RPC in flight (stdin, stdin_close, data, data_end) are skipped by their canonical head, so an upload's 256 KB chunks never reach the journal;
  • a request frame that outgrows the 4 KB AuditLineCap is recorded once as oversized, the same event the cap produced before;
  • a trailing partial line is never recorded.

wire.IsContinuation names the head check on the shared wire package, next to the encoders that emit that head, so the relay and the SDKs agree on which frames open an RPC.

Docs: protocol/wire, the root README and docs/sandboxd-api.md drop the one-connection-per-RPC wording; the agent endpoint's doc now says that an open relay holds the sandbox's idle clock, which a client keeping a connection warm has to know for idle_hibernate_seconds to apply. Two stale comments (engine/silkd.go, pool/telemetry.go) follow.

Nothing else in the relay changes: the hold, the wake-on-dial, the drain grace and the half-close handling are as before.

Hot path

Without audit: unchanged, the tee is not installed. With audit: one IndexByte per relayed chunk and at most 4 KB copied per line, where only the first line was scanned before; the post-first-line io.Copy pass-through is gone, so an audited upload now flows through the tee's Read for its whole length.

Evidence

$ (cd protocol/wire && GOWORK=off go test -race -count=1 ./...)
ok  	github.com/cocoonstack/sandbox/protocol/wire	1.398s
$ (cd sandboxd && GOWORK=off go test -race -count=1 ./server/ ./pool/ ./engine/)
ok  	github.com/cocoonstack/sandbox/sandboxd/server	2.479s
ok  	github.com/cocoonstack/sandbox/sandboxd/pool	4.017s
ok  	github.com/cocoonstack/sandbox/sandboxd/engine	4.079s
$ make go-lint | grep -c "0 issues."
10
$ (cd protocol/wire && GOWORK=off asl ./...)          # clean
$ (cd sandboxd && GOWORK=off asl ./server/ ./pool/ ./engine/)
pool/hibernate.go:329, pool/pool.go:203, pool/pool.go:697   # pre-existing forwarder advisories, files untouched here

TestAuditTeeRecordsRequestLines drives the tee whole and one byte at a time: requests back to back, an upload with a 12 KB data chunk, stdin frames, an oversized request, a partial tail. TestIsContinuation pins the predicate to the package's own encoders.

The audit tee recorded the first client line and then passed bytes
through, on the premise that a connection carried one RPC. silkd now
serves RPCs back to back on one connection (#196), so the tee scans
every line: request frames are recorded, the input frames of the RPC
in flight (stdin, stdin_close, data, data_end) are skipped by their
canonical head, and a request frame past the 4 KB cap is recorded once
as oversized. wire.IsContinuation names that head check for the relay
and the SDKs alike.

The Go-side docs drop the one-connection-per-RPC wording, and the
agent endpoint's doc says that an open relay holds the sandbox's idle
clock, which is what a client keeping a connection warm must know.

Hot path: nothing changes without audit; with audit, one newline scan
per relayed chunk and at most 4 KB copied per line, where only the
first line was scanned before.
IsContinuation matched four canonical heads by prefix, which misread a
producer that orders its keys differently and restated the op strings
the encoders own; it now takes the op through frameTag, the canonical
fast path with the token-walk fallback, and switches on the request
types' own Op(). The audit tee regains a WriteTo with a bulk-sized
buffer, since without it io.Copy's 32 KB buffer split every upload
frame into eight reads, and it copies at most the cap plus one byte of
an oversized frame instead of the whole chunk. The relay test builds
its frames with wire.EncodeRequest so the predicate is tested against
the encoders, not against literals.
@CMGS
CMGS merged commit 55b0c1a into main Sep 16, 2026
4 checks passed
@CMGS
CMGS deleted the relay/audit-lines branch September 16, 2026 17:48
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