Skip to content

feat(runner): SEA-1443 bound the Runner runtime dir under the AF_UNIX socket-path budget - #25

Merged
mattwilkinsonn merged 2 commits into
mainfrom
compass-runner-1443-runtime-dir-budget
Jul 30, 2026
Merged

feat(runner): SEA-1443 bound the Runner runtime dir under the AF_UNIX socket-path budget#25
mattwilkinsonn merged 2 commits into
mainfrom
compass-runner-1443-runtime-dir-budget

Conversation

@mattwilkinsonn

@mattwilkinsonn mattwilkinsonn commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Ports sealedsecurity/sealed#992. Rejects at startup a --runtime-dir that cannot fit the
longest agent socket path the Runner will build under it, so a misconfigured deployment
fails to boot with a message naming the overshoot instead of failing at the first
provision with a bare EINVAL.

Stacked on compass-runner-sea-1440-sunpath-cap (#12) — it collides with it, and the
collision is the interesting part.

The collision: two guards on one operand, both kept

Both branches add a NamePrefix check at the same startup edge in NewConfigSpecBuilder,
which is why the patch does not apply to #12 (patch failed: go/internal/runner/spec.go:51).
They are independent properties of the same value:

#12    strings.Contains(NamePrefix, "/")            SHAPE   — a separator escapes RuntimeDir
                                                             through filepath.Join's clean
#25    len(NamePrefix) > len(AgentContainerNamePrefix)  LENGTH — a wider prefix builds a path
                                                             past what the budget cleared

A shape check cannot catch an over-long prefix and a length check cannot catch a
traversing one
, so keeping either alone would half-enforce the model. Same for the two
test-table rows.

The test the budget model was missing

run_test.go's own comment already named this gap:

the tests above derive their expectations from the same constants as the code under test,
so shrinking the model (id width, name prefix, socket dir) leaves them green while the
real budget is wrong

TestSunPathMaxMatchesTheKernel closes that for sunPathMax by binding a real socket.
Nothing closed it for agentAccountIDWidth — the budget clears every path using
strings.Repeat("0", agentAccountIDWidth), and if the minting site ever changes width, the
model silently clears a path the kernel refuses.

TestAgentAccountIDWidthMatchesTheMintingSite pins it against the actual construction
(hex.EncodeToString of a 16-byte array, which is what store.newID does) rather than
against the literal 32. Asserting 32 == 32 would restate the constant instead of testing
it.

Verified red-capable rather than merely observed green:

const agentAccountIDWidth = 40   (mutation)
--- FAIL: TestAgentAccountIDWidthMatchesTheMintingSite
    agentAccountIDWidth = 40, but store.newID mints 32 chars (hex of 16 bytes);
    the socket-path budget in validateRuntimeDir is derived from this width and is now wrong

Restored and re-verified.

Open Question — the third operand is still unchecked

The socket path has three operands. This PR and #12 together bound two of them; the third
is asserted by a comment.

NamePrefix   operator config   bounded at startup   #12 (shape) + this PR (length)
id shape     request input     bounded at BuildSpec #12
id WIDTH     server-minted     ASSERTED, NOT CHECKED

validateRuntimeDir clears a runtime dir by modelling the id as exactly 32 chars. Nothing
verifies an incoming id is that wide — validAccountID constrains the shape only. A
path-safe id of 200 chars passes every guard and builds a path 168 bytes past what the
budget cleared: boot passes, bind fails EINVAL
— the exact failure the budget exists to
prevent.

I implemented the guard, measured its blast radius, and reverted it. len(id) != 32
reddens three tests in spec_test.go and would break four test files that provision ids
like "acct-1" and "a". That makes it a change to the Runner's contract with every
caller, not a fixture fix
— a design call rather than something to decide by rewriting
tests until they agree with me.

The comment at spec.go:118-132 now states the gap and why it is open, rather than the
earlier version which implied the width was covered. A false stated reason is worse than a
stated gap.

Tracked as SEA-1443.

Gate: run locally, because this repo has no CI

sealedsecurity/compass has no workflows — a CLEAN PR here means nothing ran. Local
testimony at 5446670, local == remote == PR head confirmed by rev-parse:

go test ./... -count=1        15/15 ok, uncached
go vet ./internal/runner/     clean
gofmt -l (banner-filtered)    0 files

-count=1 is deliberate: moon run compass-go:test --force bypasses moon's cache but not
go test's per-package cache, and reports every package (cached) — a green that executed
nothing.

moon run :ci cannot currently complete in an agent shell: compass-go:fmt fails on clean
origin/main
because ~/.proto/shims/gofmt prints a 165-byte NDJSON banner on stdout that
out=$(gofmt -l .) captures. Pre-existing, reported to compass-repo, not fixed here.

Base verification

All 5 files compared between sealed's merge-base (fe4e8424) and compass origin/main:
5/5 byte-identical. Against #12's branch, 3/5 identical and 2 divergent — which is the
collision above, found by measurement rather than discovered at apply time.

Refs SEA-1443

@linear-code

linear-code Bot commented Jul 28, 2026

Copy link
Copy Markdown

SEA-1443

mattwilkinsonn commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

@seal-agent seal-agent changed the title feat(runner): SEA-1443 bound the runtime dir under the socket-path budget feat(runner): SEA-1443 bound the Runner runtime dir under the AF_UNIX socket-path budget Jul 28, 2026
Comment thread go/internal/runner/run_test.go
@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown

Greptile Summary

This PR validates the Runner runtime directory against the platform AF_UNIX socket-path limit at startup.

  • Derives the socket-path cap from the platform's sockaddr_un.
  • Rejects relative or over-budget runtime directories before dialing the server.
  • Centralizes the production container-name prefix and rejects configured prefixes wider than the budgeted value.
  • Adds boundary, kernel-binding, and startup-path tests for the new validation.

Confidence Score: 4/5

The PR appears safe to merge, with only the previously reported regression-test coupling issue remaining.

The runtime validation and prefix constraint are internally consistent, but the new minting-width regression test still duplicates the minting implementation and therefore will not detect a future width change at the actual minting site.

Files Needing Attention: go/internal/runner/run_test.go

Important Files Changed

Filename Overview
go/cmd/compass-runner/main.go Replaces the inline production container-name prefix with the shared Runner constant.
go/internal/runner/run.go Adds platform-derived AF_UNIX path budgeting and validates RuntimeDir before network enrollment.
go/internal/runner/run_test.go Adds validator boundary tests, a real kernel socket-bind check, and verification that Run rejects invalid configuration before dialing.
go/internal/runner/spec.go Rejects name prefixes wider than the prefix included in the startup socket-path model.
go/internal/runner/spec_test.go Adds coverage for rejecting a name prefix that exceeds the socket budget allocation.

Reviews (4): Last reviewed commit: "feat(runner): SEA-1443 bound the runtime..." | Re-trigger Greptile

Base automatically changed from compass-runner-sea-1440-sunpath-cap to main July 29, 2026 16:50
@seal-agent
seal-agent force-pushed the compass-runner-1443-runtime-dir-budget branch 2 times, most recently from 1872eb0 to 8dfef67 Compare July 29, 2026 17:50
…dget

Ports sealed#992. Rejects at startup a --runtime-dir that cannot fit the
longest agent socket path the Runner will build under it, so a
misconfigured deployment fails to boot with a legible message naming the
overshoot instead of failing at the first provision with a bare EINVAL.

Stacked on the sun_path cap branch, which it collides with in spec.go and
spec_test.go: both add a NamePrefix guard at the same startup edge. They
are independent properties of one operand and both are kept -- the cap
branch rejects a prefix containing a path separator (shape), this one
rejects a prefix wider than the budget reserves (length). A shape check
cannot catch an over-long prefix and a length check cannot catch a
traversing one.

Adds the test the budget model was missing. run_test.go's existing tests
derive their expectations from the same constants as the code under test,
which its own comment notes leaves them green if the model shrinks. The
kernel test binds sunPathMax that way; nothing bound agentAccountIDWidth.
TestAgentAccountIDWidthMatchesTheMintingSite pins it against the actual
construction -- hex.EncodeToString of a 16-byte array -- rather than
against the literal 32, so it tests the constant instead of restating it.
Verified red-capable: widening the constant to 40 fails with the message
naming both widths.

The id width itself is still unchecked at provision time, and that is
recorded as an open question rather than silently fixed: asserting it
would change the Runner's contract with every caller (four test files
provision ids like "acct-1"), which is a design call.

Refs SEA-1443

Co-Authored-By: seal <noreply@sealedsecurity.com>
@seal-agent
seal-agent force-pushed the compass-runner-1443-runtime-dir-budget branch from 8dfef67 to 3bd6161 Compare July 29, 2026 17:54
The id becomes a path component (container name + agent socket path), so
constrain it to its minted shape — exactly agentAccountIDWidth lowercase
hex — at the provision edge, subsuming the prior shape-only check.

Refs SEA-1505.

Co-Authored-By: seal <noreply@sealedsecurity.com>
@mattwilkinsonn
mattwilkinsonn merged commit a07f2c8 into main Jul 30, 2026
3 checks passed
@mattwilkinsonn
mattwilkinsonn deleted the compass-runner-1443-runtime-dir-budget branch July 30, 2026 02:07
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