Conversation
|
Field evidence for the running-image lane, from a deploy on our own fleet this morning — worth adding because it fired unprompted, hours after the PR was written. An insula module deploy ( That's the exact state the Linux test in this PR constructs deliberately: spawn a copy, atomically replace it on disk, then assert (Adjacent, from the same deploy, in case it's useful for the operator docs rather than this PR: copying directly over a running binary gives One limitation of the mechanism I should state plainly, since the above could be read as a stronger claim than it is: |
936c722 to
34e5079
Compare
|
Sequencing note so the review happens exactly once: I'll run the full review-by-execution pass (twin matrix, mutation fences on the never-merge property, live drive against a real daemon) as soon as two known-incoming changes land in the branch: (1) the |
34e5079 to
bceb381
Compare
|
Both incoming changes are pushed — 1.
|
|
Review by execution on the merged tree (master + branch, the 0.5/0.6 subc-control conflict resolved to 0.6). One blocking revision, everything else verified green. The revision: What I verified, all green:
Also needed in the same push: rebase over current master — #60's pid-reuse window is correctly out of scope here per its own filing; the captured-facts-never-config property reads exactly as the #58 disposition specified. Green matrix on the twin after the platform fix + rebase = merge. |
|
Taking the revision — with one correction to the fix shape, because the suggested form still fails Windows.
|
CONSUMER-IMPACT: Old daemons ignore the additive optional provenance block during manifest decode, so an absent or present block does not make HELLO incompatible. Rust path-dependency consumers that construct ModuleManifest by literal must update that source promise (16 explicit constructors in-tree add provenance: None; out-of-tree literals can fail E0063 after the protocol bump, or the insula-class E0560 when source and dependency move in the opposite order).
The manifest block introduced earlier in this branch named its commit field build_commit, while master already ships build_git_sha and build_lock_digest on server.describe, consumed by ck's dashboard and its build-skew detector. Two names for one fact inside a single response is a reader trap, and renaming the shipped field would have broken an older ck against a newer daemon by making the skew check fail open. Align the unreleased name onto the shipped vocabulary instead. CONSUMER-IMPACT: Renames ManifestProvenance.build_commit to build_git_sha in subc-protocol. The field is unreleased -- it was introduced earlier in this same branch and has never appeared in a published crate -- so no out-of-tree consumer can have decoded or constructed it under the old name, and there is no deprecation window to honour. Net effect against master is unchanged from the introducing commit: one additive optional block whose fields old daemons ignore during manifest decode, with Rust literal constructors of ModuleManifest still needing the source update that commit described. Wire field names on server.describe and the daemon provenance record are untouched by this commit and remain byte-identical to master.
The sweep enumerated ManifestProvenance's fields by hand, so a field added later would escape it silently. Destructuring makes that a compile error (E0027) at the sweep instead.
CONSUMER-IMPACT: Manifests carrying non-printable or over-long provenance values are now refused where they were previously accepted; this changes behavior for any module already sending such values.
CONSUMER-IMPACT: Manifest provenance values that are empty are now rejected during HELLO decoding; modules previously sending empty declarations must omit the field or provide a non-empty value.
The CK_BUILD_REV rename replaced names that already ship on master (build.rs emission, server.describe build fields, the ck skew check) — subconscious's own provenance skew detector. Reverting per the #58 ruling keeps the rename from silently retargeting it. The two names now mark two different provenance sources rather than one name meaning either: SUBC_BUILD_* is this repo's build.rs git probe, while CK_BUILD_REV is injected as a process env by release scripts in other fleet repos. Blast radius is four in-crate sites; cargo:rustc-env never escapes the crate build, so no consumer sees the change. CONSUMER-IMPACT: none — reverts to the names already shipped on master.
…pelling The provenance human-output test pinned linux_proc_sha256, so it failed honestly on macOS (which reports macos_spawn_inode) and on Windows (which has no probe and correctly reports unavailable). The cfg split now lands on capability rather than spelling: probe-capable platforms assert a match plus membership in the closed RunningImageEvidence set, and platforms without a probe assert the unavailable verdict positively rather than skipping. CONSUMER-IMPACT: none — test-only.
bceb381 to
8b70e02
Compare
|
Revision pushed at The fix follows the capability split from my prior comment:
Load-bearing RED proofs from the implementer:
Local gates after the rebase + fix: Linux arm executed. macOS and Windows remain compile-reasoned until twin #63 runs them; I am not claiming those arms green before the matrix does. One correction from the rebase: |
|
Twin matrix on |
Windows CI rejected `subc-core` (lib) under -D warnings: both RunningImageEvidence constructors are cfg-gated to their probing platform (LinuxProcSha256 under target_os="linux", MacosSpawnInode under any(target_os="macos", test)), so on a platform with no probe the unqualified import had no user. The gate is the union of its users, including `test`, and that term is load-bearing rather than defensive: compare_spawn_inode is compiled under cfg(test) on every platform, so a Windows *test* build does construct the type. Gating on any(linux, macos) alone would have traded the lib break for a test break on the same platform. Verified by mutation: narrowing the gate to target_os="macos" fails the Linux build at both constructor sites (E0433, 2 errors). Windows itself remains unverifiable locally -- no rustup, so no cross-target check -- and is left to the twin. CONSUMER-IMPACT: none -- compile-gating only, no behavior or wire change.
|
What actually failedBoth Worth stating plainly: I read the earlier Windows failure as the platform pin because that was the finding in hand, and the platform pin was real on macOS. But on Windows the job died before the tests, so that leg's red was always this compile error. I should have pulled the log rather than inheriting the diagnosis. CauseBoth evidence constructors are cfg-gated to their probing platform — The gate is now the union of its users: #[cfg(any(target_os = "linux", target_os = "macos", test))]
use subc_control::RunningImageEvidence;The Proof, and its limitMutation: narrowing the gate to I cannot compile-verify Windows here: no rustup, distro cargo, no cross-targets, and the MSVC target needs a toolchain this box does not have. So the Windows arm is reasoned, not executed — the twin is the only proof. I did audit for the same class rather than fixing one instance and re-running blind, since this is the second Windows-only break in this file (the first was Linux gates on
|
|
Our comments crossed by about a minute — yours at 06:28:05Z, mine at 06:29:49Z — so two things to reconcile. You already answered the macOS question I closed my comment by asking. You reported Linux and macOS green on We landed on the same root cause independently, and I took your option 1. You offered gating the import versus referencing the type in the One detail on the gate that is easy to get wrong, and I only found it because the naive version was my first instinct: the union has to include Carrying your macOS green forward to
|
…greement (supervisor.provenance) iceteaSA's #58 build: modules declare build facts at HELLO (optional ManifestProvenance), the daemon overlays process-identity evidence it alone can attest (linux proc sha256, macos spawn inode, windows typed unavailable), served via supervisor.provenance. Carried on the CI twin with three fixes of mine folded: SupervisorProvenance dispatch-op arm (semantic conflict with the timing instrumentation), platform-gated test imports, and the validate-in- Deserialize witness pattern verified by mutation at both tiers. CONSUMER-IMPACT: subc-control 0.6.0, subc-core 0.8.0, subc-client-rs 0.8.0 — additive control op + optional manifest field; no existing decode changes.
|
Merged to master at e71d189 via the CI twin (fork PRs skip the Rust matrix here, and master had moved twice under your head — the twin carried your commits rebased plus three fixes noted in the review thread: the SupervisorProvenance dispatch-op arm from the timing-instrumentation collision, platform-gated test imports for the macOS/Windows clippy legs, and nothing else touched). Full matrix green at 3e3d984; 753 workspace tests on the merge. Your validate-in-Deserialize shape — invalid provenance unconstructable from wire bytes — is the pattern the next wire type copies, and the split-attestation boundary (modules declare build facts, daemon attests process identity) landed exactly as ruled in #58. subc-control 0.6.0 / subc-core 0.8.0 / subc-client-rs 0.8.0. |
…greement (supervisor.provenance) iceteaSA's #58 build: modules declare build facts at HELLO (optional ManifestProvenance), the daemon overlays process-identity evidence it alone can attest (linux proc sha256, macos spawn inode, windows typed unavailable), served via supervisor.provenance. Carried on the CI twin with three fixes of mine folded: SupervisorProvenance dispatch-op arm (semantic conflict with the timing instrumentation), platform-gated test imports, and the validate-in- Deserialize witness pattern verified by mutation at both tiers. CONSUMER-IMPACT: subc-control 0.6.0, subc-core 0.8.0, subc-client-rs 0.8.0 — additive control op + optional manifest field; no existing decode changes.
Implements the #58 disposition. Closes #58.
Modules self-report build facts at HELLO; the daemon independently observes process facts. The two travel as separate typed members and never merge — a module's declaration can be false, so it is never presented as something the daemon attested.
What's here
ModuleManifest.provenance— optional block, four optional fields:build_git_sha,build_lock_digest,wire_crate_version,store_schema_version. Absent is fine and stays fine: the module registers normally and reportsunverifiable.supervisor.provenance— new channel-0 op, whole-box or exact-module. Each entry carriesmodule_declared(reportedwith the block, orunverifiable) anddaemon_observed(pid, spawn time, exact spawned-from path, running-image evidence). The daemon's own identity rides in the same response, so one call answers "what is this daemon, and what is it supervising".Spawn-fact capture — the supervisor retains pid, spawn timestamp, the exact program path it executed, and the spawn-time file identity, captured around
spawn(). Provenance answers come from those captured facts, never from current configuration: a rescan can repoint config while the old process runs, and reading config would attest a path the child was never spawned from. Every exit path clears the facts through one helper.Executable identity — on Linux,
/proc/<pid>/exeand the captured spawned-from path are opened at read time and hashed through the open handles (never reopened after metadata, so the file can't be swapped between the stat and the hash). Digests are cached by(dev, inode, size, mtime), bounded at 64 entries. macOS compares spawn-time and current inode — deliberately weaker than a hash, and the response says which method produced the evidence. Elsewhere: typed unavailable. No failure path ever produces a placeholder digest.ck provenance <module>— human output keeps source labels (MODULE-DECLARED/DAEMON-OBSERVED) so a declared value can't be read as an observed one;--jsonforwards the typed response unchanged.What proves it
fs as its build sha, plus distinct sentinels in the other three fields. The test asserts none of them appears anywhere in the daemon-observed record or the daemon's own identity. Mutation: routing a daemon field to read a declared value reddens it./proc/<pid>/exehashes the still-running old inode while the path hashes the replacement →Mismatch. This is the deploy-skew case as a regression test.spawned_from.Wire compatibility
No breaks. The
server.describegolden differs from master by one additive op-list entry;build_git_shaandbuild_lock_digestkeys and values are byte-identical. The new manifest block adopts those same field names rather than introducing synonyms, so one vocabulary coversserver.describe, the daemon record, and the module declaration. Ten golden fixtures total: five new, five regenerated op lists. TS client tests pass unchanged — it doesn't parse this op in this PR.Declared values are untrusted input
They're module-controlled strings that reach an operator's terminal, so they're bounded at the daemon boundary — non-empty, ≤128 bytes, printable ASCII — and a manifest violating that is refused at HELLO. Absent and malformed are deliberately different outcomes.
ckalso escapes non-printable bytes at render, so a value from an older daemon can't emit control sequences into the terminal region where the daemon's verdict prints. Both halves are independently load-bearing (severing either reddens a different test).Deliberately not here
No
origin_delta, nobuildable_at_head, no git, network, or deploy logic entered the daemon — verified mechanically: the only added dependency issha2, andCargo.lockgains no packages.Two things for you to decide
1.
CK_BUILD_REVvsSUBC_BUILD_GIT_SHA. Asked in #58 and unanswered, so the branch currently carries the rename. My disposition proposed "standardizeCK_BUILD_REV" without checking thatbuild.rsalready emittedSUBC_BUILD_GIT_SHAfor the same purpose — so what you approved was a rename, and that wasn't visible from what I wrote. It's self-contained (four sites, all insubc-core,cargo:rustc-envonly, nothing in scripts or CI). I lean toward reverting to your existing names: it isn't load-bearing for #58 and it's diff noise in a PR you'll read for the attestation logic. Say the word and I'll revert it; the wire field names are unaffected either way.2. The SDK helper has no first-party user.
subc-client-rs::build_provenanceis unit-tested but every in-tree module hardcodesprovenance: None, so the "modules self-report" half is demonstrated only by the test stub. Wiring it intoecho-modulewould make the reference module teach the feature, but that changes a reference module's behavior and wasn't in the plan, so I left it. Happy to add it here or as a follow-up.Known gap, filed rather than fixed
Between
wait()returning and the exit handler clearing spawn facts there's a scheduler-bounded window where a probe could read a recycled pid. Worst case is a falseMatch; exploiting it needs a hostile fork inside that window and a coincidental hash match. Closing it properly means widening the probe signature for a liveness gate, which didn't belong in this PR. I'll file it separately unless you'd rather have it here.Verification
14 commits, each reviewed independently by a cross-family model at 0 blocking findings, plus a security review and two whole-branch passes. At the merge with master: fmt, clippy
-D warnings, 742 tests, both fleet scripts,git diff --check, andbun testall pass. Gates were also run on the merge result, not just the branch — master moved 20 commits during the work and one of them touchedfake-aft-stub.rs, which this PR also extends.Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.