Give CORA's own background work a real door, not the shrug that names no one - #760
Merged
Conversation
… no one Every real HTTP route and MCP tool already resolves a proper surface_id, the Authorize port's answer to "which door did this arrive through." But roughly ten places in CORA's own composition-root code, agent tick loops, capture readers, one-time operator entrypoints, call a handler directly, in-process, with no surface_id at all, so it silently falls through to the nil sentinel: "unspecified." That makes internal work indistinguishable from any other caller a Policy has not yet classified, and there was no way to write a rule that treats "a person clicking a button" differently from "CORA's own RunWitness agent doing routine background work," because both arrived through the same unlabeled door. This gives that work a real one. A new closed-enum SurfaceKind value, IN_PROCESS, and a fourth seeded Surface alongside the existing three, wired unconditionally at every site that omitted surface_id: RunWitness's four handler calls, the two capture readers it constructs, the scan ingestor, the inference recorder, the reference observation feeder shipped for deployments to copy, and the one operator-run promotion script. No new Settings knob, unlike the Conduit change this is the sibling of: these calls always originated in-process, so naming that fact is a correction, not a behavior change a deployment might want to opt out of. Not named INTERNAL. A naming review caught two problems with that word before it shipped: read aloud inside a transport enum it suggests "internal network," not "no transport," and it collides with an unrelated existing domain value (Operation.acquisitions.trigger_mode's "Internal" camera- trigger source). IN_PROCESS is the phrase this codebase already uses for the same referent in roughly twenty other places, including the new constant's own docstring before the rename. Deliberately excluded from the HTTP-reachability map the observability layer uses to resolve a Surface's kind for HTTP-arriving requests, the same reasoning that already excludes MCP_STDIO: in-process work is by definition never reachable through the HTTP middleware. Mutation-verified in both directions, reverting one wired call site to omitted and reverting the exclusion, and confirmed each turns exactly its paired test red. Under the starter Policy currently shadowing at 2-BM, bound to the HTTP surface, this changes the shadow log's recorded near-miss reason from "surface: nil vs HTTP" to "surface: in_process vs HTTP," still a mismatch, so the Allow/Deny outcome for every existing deployment is unchanged. What changes is that internal work becomes a distinct, nameable category a future Policy can actually govern, instead of being lumped in with every other unspecified caller. Co-Authored-By: Claude Sonnet 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
xmap
added a commit
that referenced
this pull request
Sep 1, 2026
… one (#761) Every in-process call (RunWitness, the capture readers, the inference recorder, promote_seeded_fleet) now carries SYSTEM_IN_PROCESS_SURFACE_ID per #760, but the deployment's one configured Policy is bound to the HTTP door, so every one of those calls strict-denies at the surface check. Reading 2-BM's shadow log confirmed it: 99.998% of a 152,000-event window was exactly this mismatch, drowning out anything a real front-door near-miss would show. Gives the in-process door its own second, optional rulebook instead of widening Policy itself (Policy.surface_id stays a scalar; that general multi-surface case is deferred until MCP traffic exists to validate it against). TrustAuthorize resolves which configured policy governs a call from its surface_id, mirroring the trust_conduit_id shape from #759: Settings.trust_in_process_policy_id, default None, existing deployments and tests unaffected. Two new boot guards close the same "looks wired, governs nothing" gap the conduit knob already guards against: a backdoor policy set without a front one, and a backdoor policy that doesn't actually govern the in-process surface (or, once trust_conduit_id is set, the right conduit). promote_seeded_fleet now always promotes through AllowAllAuthorize rather than the caller's kernel.authz, matching pilot_seed.py's own kernel construction -- it is a rare, explicitly-operator-run bulk recovery outside every request surface, and should not need enrollment in whichever policy ends up governing the in-process door to run at all. Co-authored-by: xmap <16776958+xmap@users.noreply.github.com> Co-authored-by: Claude Sonnet 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.
Item 2 from the deferred list on #759: give CORA's own background work a real, nameable door, so it stops being lumped in with "unspecified" the same way an unclassified caller would be.
The gap
Every real HTTP route and MCP tool resolves a proper
surface_id, theAuthorizeport's answer to "which door did this arrive through." But roughly ten places in CORA's own composition-root code call a handler directly, in-process, with nosurface_idat all, so it silently falls through to the nil sentinel. That makes internal work indistinguishable from any other unclassified caller, and there was no way to write a Policy rule that treats "a person clicking a button" differently from "CORA's own RunWitness agent doing routine background work," because both arrived through the same unlabeled door.What ships
A new closed-enum
SurfaceKind.IN_PROCESS, a fourth seeded Surface alongside the existing three, and every identified in-process call site wired to it unconditionally:RunWitnessRecorder's four handler calls (record_witnessed_run,truncate_run,record_witnessed_run_outcome, thelist_runsinside its open-capture rebuild)CaptureBaselineReader,CaptureProgressFeeder)CaptureScanIngestorDelegatingInferenceRecorder(the productionInferenceRecorderport implementor — fixing it here covers every caller that goes through the port, including two agent subscribers that call the port interface and never touchsurface_idthemselves)SimObservationFeeder(the shipped reference feeder deployments copy for their real EPICS/tomoStream feeders — not wired into the live app today, but worth wiring correctly since it's the example other code follows)promote_seeded_fleet(a one-time operator CLI entrypoint — still the same door as an agent tick loop, just a different principal on the other side; Surface answers "which door," not "who")No new
Settingsknob, unlike the Conduit change in #759 that this is the sibling of. That one added an opt-in because redirecting which conduit governs is a behavior change worth gating. This one is a factual correction: these calls always originated in-process, so naming that fact isn't something a deployment would want to opt out of.A naming correction along the way
I designed this with
SurfaceKind.INTERNAL. A naming reviewer caught two real problems before it shipped: read aloud inside a transport enum, "internal" suggests "internal network," not "no transport" — and it collides with an unrelated existing domain value,Operation.acquisitions.trigger_mode's"Internal"camera-trigger source. The fix,IN_PROCESS, also happens to be the phrase this codebase already uses for the same referent in roughly twenty other places. Renamed everywhere before committing: the enum member, theSYSTEM_IN_PROCESS_SURFACE_IDconstant, the migration (file and content), the new test file, and every reference across production and test code.What this changes and doesn't, for an already-shadowed deployment
Under the starter Policy currently shadowing at 2-BM (bound to the HTTP surface), this changes the shadow log's recorded near-miss reason from
"surface: nil vs HTTP"to"surface: in_process vs HTTP"— still a mismatch, so the Allow/Deny outcome for every existing deployment is unchanged. What changes is that internal work becomes a distinct, nameable category a future Policy can actually govern, instead of being lumped in with every other unspecified caller.Deliberately excluded from
_SURFACE_KIND_BY_UUID(the map the observability layer uses to resolve a Surface's kind for HTTP-arriving requests) — the same reasoning that already excludesMCP_STDIO: in-process work is by definition never reachable through the HTTP middleware.Process note
Designed by Opus, implemented by a Sonnet subagent working from a fully-specified brief (exact call sites, naming decisions, migration pattern, testing bar). The subagent's own naming-review pass caught the
INTERNAL→IN_PROCESSissue above; I applied that rename and ran the full independent verification pass myself before this went anywhere.Verification
Unit + architecture: 49,102 passed. Contract: 3,436 passed. Full integration tier: 1,367 passed, including a new test that applies the migration against a real Postgres testcontainer and confirms the Surface folds to
SurfaceKind.IN_PROCESS. Mutation-verified in both directions myself: reverting one wired call site to omitted, and reverting the HTTP-reachability exclusion, each turns exactly its paired test red.ruff,pyright,tach,make docs-buildall clean. OpenAPI snapshot regenerated (thekindenum and its description text both changed).Also swept the rest of the codebase for any handler call passing
principal_idwithoutsurface_idthat the original brief might have missed: found none. Everything that looked like a candidate on a broader grep turned out to already be either a surface-transparent pass-through (conductor.py, which threads a realsurface_idparameter), a call through theInferenceRecorderport interface (correctly needs nosurface_idof its own now that the implementor is fixed), or a genuinely unwired/dead code path (_experiment_steerer.py'ssteer_experiment, which has no caller anywhere insrc/ortests/today and will need its ownsurface_idthreading whenever it's actually wired to something — noted here for whoever does that, out of scope for this PR).🤖 Generated with Claude Code