Repository navigation
feat(canonical): admit JSON on the bytes before it is hashed or signed - #204
Merged
graywolf336 merged 23 commits intoSep 30, 2026
Merged
Conversation
jcs::canonicalize takes a serde_json::Value, and four conformance cases cannot be decided from one. A repeated object member is gone before canonicalize is reached, because the parse kept the last of the two. A document nested past a bound has already been built by the time anything could decline to build it. RFC 8785 section 3.2.2.3 defers number formatting to ECMAScript, which has one numeric type, so the specification admits an integer past 2^53 and a fractional number and writes the double each rounds to; refusing either is the RFC 7493 profile rather than a canonicalization rule. Add jcs::admit_document(&[u8]) -> Result<Value>. It runs jcs-admit over the raw bytes under RFC 8785 plus the RFC 7493 profile tightened to integers only, then parses the same bytes. The value comes from the bytes the document arrived in rather than from the admission's canonical output, so an accepted document keeps its value, its content hash and its proof, and refusal is the only new behaviour. CanonicalError::Admission carries the fault itself rather than a rendered string, so a caller can tell a repeated member from a depth bound. Two call sites move onto it: decode_from_transport, which receives the Atomic-Delegation header, and load_for_delegate, which reads documents back out of the identity store. maxChanges is the only numeric field the vocabulary carries and it is a count, so the integers-only tightening costs nothing. Nine tests in atomic-canonical/tests/ingest_boundary.rs. Four are the cases above, copied byte for byte under tests/vectors/ingest/ with their vector ids, manifest paths, suite commit and digests recorded in PROVENANCE.md, each asserting the refusal variant rather than a message. Five are accept controls over a minted certificate: admitted in both the compact and the indented serialization, still round-tripping through encode_for_transport and verifying, and still found in a real identity store.
atomic identity grant reads a certificate from a file, from stdin or out of the identity store and then verifies it, so the bytes it parses are verification input and belong behind the same admission as the header and store paths in atomic-canonical. Route load, list, push, verify and revoke through jcs::admit_document, and delegate_from_request with them: grant new --request countersigns a self-signed request that arrived from the far end, and its signature is checked over the canonical form of whatever those bytes denote. No new dependency: the crate already depends on atomic-canonical, and jcs-admit stays a dependency of atomic-canonical alone. A document that fails admission is refused where it was previously parsed, which for load, verify, revoke and the request read is an error naming the fault and for list and push is the skip those paths already had. The typed reads elsewhere in the workspace are left alone on purpose. A struct deserialization already answers 'duplicate field' for a repeated member, while the same bytes into a serde_json::Value answer Ok and keep the last one, so the untyped reads are the ones that needed a decision.
…tomicdotdev#201) The opencode plugin attributes working-copy changes via in-memory ownership claims and sends them as record_files manifests. A plugin restart drops every claim; deletions of files the session itself recorded earlier then never appear in any later manifest and strand as pending deletions forever (observed as 16 pending deletions after an ownership lockout forced a plugin restart mid-session). record_turn now augments explicit record_files manifests with Deleted status entries for paths in the session's persisted files_touched before scoping and validation. Attribution stays conservative: files the session never recorded remain out of scope. fix(agent): re-negotiate recording mode when the plugin changes it The plugin declares its recording mode (recording_scope) on every session-start, but an existing session short-circuited before the declaration was read: a session that opted into explicit-files under an older plugin kept explicit_record_files=true forever. Once a newer plugin stops sending record_files, every Stop fails with 'explicit record_files manifest required' and the session's view silently stays empty — observed live when a session resumed after the plugin switched to whole-tree recording. Apply the declared scope on BOTH session-start paths (fresh and re-entered). An explicit-files session follows a plugin that no longer declares the scope; an unknown declared scope refuses session-start instead of silently recording under the wrong mode.
…olver (atomicdotdev#209) Replace change/command.rs's local, case-SENSITIVE prefix scan with Repository::find_change_by_prefix, the same case-insensitive resolver used by 'atomic insert' and 'atomic unrecord'. Removes the last divergent hash-addressing rule in the CLI.
Two depth limits were in force on the admission path and they disagreed by exactly one document. The admission options passed no explicit max_depth, so jcs_admit used its own default of 128 and accepted a document nested in 128 containers. The parse on the very next line, serde_json::from_slice, then applied a second limit of its own, also 128, but undeclared and refusing AT that depth rather than past it. A document sitting exactly on the cap was therefore admitted and then rejected one line later as one that "does not deserialize". The result is a validity split inside a single crate: it encodes a certificate it will not read back. encode_for_transport writes such a document into the Atomic-Delegation request header without complaint, and decode_from_transport refuses it on the way in, so the split is reachable by any caller that sends the header rather than only from a test. The error surfaced blames deserialization, pointing at the bytes instead of at the limit that actually fired, which is the wrong place to look. The cap is now declared once as jcs::MAX_DEPTH, passed to the admission explicitly, and serde_json's recursion limit is switched off for the parse. That is sound only because the admission has already walked the same bytes and refused anything nested deeper, so the parse never sees an input the cap did not already bound. The ordering is load-bearing and the code says so. What the crate admits is now exactly what it reads back, and the cap a caller relies on is the one that is written down. The three new vectors are wire bytes at 127, 128 and 129 containers, checked in as files so the boundary is exercised from both sides rather than only from past it. Each test asserts its file's length before reading it, so an editor that appends a newline cannot quietly change the document under test.
…heckout (atomicdotdev#215) `atomic agent enable` always re-clones the skills-source package (atomic-skills) from Atomic storage on every install, even when --from is used. The manifest's [skills-source], [skills] and [agent-definition] inputs are only reachable through that remote sync, so offline installs of a local integration package still fail. Add --from-skills <PATH>: point the skills inputs at a local atomic-skills checkout. No network access happens, and the manifest and agent definition are read from the given path. Without the flag, behavior is unchanged (remote sync, as before).
…otdev#218) `atomic diff` conflates two questions: working copy vs. recorded state (no -c), and the state before vs. after a change (-c). It also had no machine-readable mode, making it the only core VCS command without one. - Add `--json`, emitting a versioned document (per-file status and paths, hunks with typed lines, insertion/deletion rollups). It takes precedence over --stat/--name-only/--name-status so consumers get one parseable document instead of formatted output to re-parse. A `-c` diff carries the change header under `change`; a working-copy diff carries `view`. An empty diff still emits valid JSON. - When the working copy is clean, name real copy-pasteable `-c` commands for the most recent changes on the current view. A clean working copy was a dead end for anyone expecting to see a change. Uses include_inherited so a freshly forked draft does not claim it has no recorded changes.
…tdev#219) * feat(agent): select a delegated identity for hook recording `atomic agent identity set <name>` makes every hooked agent on this machine record under a delegated identity's own key instead of the plus-tag of the default identity. The selection is global — one identity per machine, deliberately not per repository — written to a new top-level `agent_identity` in ~/.atomic/config.toml. Hooks resolve the identity on every invocation: ATOMIC_AGENT_IDENTITY env var > global agent_identity setting > active server profile's agent_identity binding > plus-tag fallback The server-profile link closes a promise that was already written down: `bind_agent_identity` documents the binding as "so hooks use it by default", but only push auth ever read it — recording never did. `set` validates eagerly (the identity must resolve and be an agent/delegated type; human identities are refused; a missing delegation certificate warns) while the record path degrades softly on a bad name — recording a turn must never fail over identity selection. `show` and `agent status` (human + JSON) report the effective identity and its source. With nothing configured anywhere, recorded turns are indistinguishable from before, and `atomic record` is untouched. atomic-agent never reads config: the resolved name crosses as plain data through TurnRecordOptions into build_agent_author and active_delegation_urn, so change headers carry the agent's public key and envelopes name the active delegation certificate. * chore(agent): fmt and rustdoc fixes for identity selection cargo fmt across the touched crates, and the `key()` doc comment linked to `Self::fmt` — a trait impl method rustdoc cannot resolve — which failed the Documentation job under -Dwarnings. * fix(cli): serialize database-owner integration tests Every test in database_owner_integration_test spawns real atomic subprocesses that hold the redb lock and burn CPU. On 2-core Windows CI runners, the default test-threads parallelism lets the process-heavy tests (the eight-process session-start hammer, the failpoint owners) starve whichever sibling tests overlap them past their database-wait budgets. The failure signature is exactly that: different tests fail on different runs — concurrent_session_starts…, crashing_second_checkpoint…, owner_death_after_checkpoint_prepare… in this PR's runs; concurrent_stops_publish… on another PR the same day — all in this file, all contention-shaped (one with an explicit 'Database already open. Cannot acquire lock'), while macOS and ubuntu pass the same suite. #[serial] trades a few minutes of wall time for runs that only fail when something is actually broken. Locally the serialized suite passes in 53s (macOS); CI is the only place the starvation reproduced.
…ent (atomicdotdev#217) `triage review` and `triage candidates` required both a source view and `--into`, so the whole command had to be retyped from memory on every invocation. The common gesture — "is the view I'm working on ready to promote?" — is now a bare `atomic triage review`. - `<VIEW>` is optional and defaults to the current view; the metavar drops `FEATURE` for `VIEW`, since `feature` read as a literal view name. - `--into` is optional and defaults to the view's *direct* parent, matching bare `atomic insert`. Deliberately not `nearest_shared_ancestor`, so feature-login -> service-auth -> dev targets service-auth. - Both args get view-name shell completion, which neither had. `--help` also showed zero examples: the `# Examples` doc blocks became `long_about`, which `apply_agent_help` strips tree-wide. Moved to `after_help` — the one slot the agent template preserves — with `--walkthrough` led on, as it was the undiscoverable flag. An unknown view name now explains itself instead of dead-ending on "not found", naming the current view as the value to drop in or omit: No view named 'feature' — <VIEW> takes a real view name, not a placeholder. Current view is 'feature-x'; omit <VIEW> to triage that instead, or run 'atomic view list' to see all views. Resolution goes through the cheap parent_change_count() rather than get_view_info(), which materialises the parent's whole visible change set.
Contributor
Author
|
Hey @graywolf336, the CI runs on #202 and this pull request are waiting for approval. I ran the Ubuntu jobs against each merge commit, and Format, Check, Clippy, Documentation, Test with the CLI harness, and MSRV all pass on both. #202 needed one rustfmt fix, now pushed as af313df, and the macOS and Windows tests are the part I did not run myself. Would you approve the runs when you have a moment? |
Contributor
|
@astrogilda sure thing! Just approved them. Thanks for the contributions. I'll take some time tomorrow or Monday to review them and test them out |
* feat(change): Ed25519 signing for recorded changes
Recorded changes previously carried only an unverifiable claim of
authorship: the identity's public key was copied into the header, but
nothing ever touched the private key. Anyone could author a change
claiming any author and any key.
This adds real signatures:
- New SIGNATURE section (0x04) in the V3 change format carrying the
signer's did:atomic fingerprint, an Ed25519 signature over the change's
content hash, the signed hash, and a timestamp.
- The SIGNATURE section is unhashed, so the change hash remains a pure
function of content+header+deps: re-signing with a different key never
changes the change's identity.
- Signing happens at the Repository::record seam (covers CLI record and
revise content-mode); revise --reword signs via Change::sign_with since
it bypasses the record pipeline.
- Verification is strictly out-of-band: verify_change_signature takes a
caller-resolved public key and never treats the DID embedded in the
change as a trust root. A forged change (attacker key + claimed
identity) fails against the victim's key.
- Signature is domain-separated (atomic.change.signature.v1) so a change
signature cannot be transplanted from or accepted as a signature over
another object kind (attestations, intents, captures).
- Backward compatible: unsigned changes load, apply, and display
unchanged; a missing signature is "no signature claim", not an error.
- When no signing identity is available, record warns and records
unsigned instead of silently claiming an author without a key.
Tests: 5 integration tests (record->parse->verify byte-level, hash
stability, forge scenario, unsigned compat, signed+unsigned coexist) plus
unit tests for the signing core. Full workspace suite passes (53 test
binaries, 0 failures).
* fix(ci): resolve clippy, fmt, and test failures on change-signing branch
- revise.rs: replace single-arm `match` with `if let` (clippy::single_match)
- change.rs: remove leftover debug eprintln statements from investigation
- tests_signing.rs: drop unused glob import, replace debug printlns with
assertions, verify the SIGNATURE section round-trips at section level
- change_signing_test.rs: remove unnecessary `mut` on repositories that
are only read
- formatting across all touched files (cargo fmt)
Verified locally against the exact CI commands:
- cargo fmt --all -- --check: clean
- cargo clippy --workspace -- -D warnings: clean
- cargo test --workspace: 51 test binaries, 0 failures
- CLI semantic diff harness (run_all.sh 09): 1/1 suites passed
* feat(agent): sign hook-recorded turns with the effective identity
The signing seam covered `atomic record` and `revise` only. Harness turns
(opencode, Claude Code, Gemini CLI, ...) go through record_turn, which
passed `signing_identity: None`: a turn's header claimed the agent's
public key while nothing proved possession of it — exactly the
'unverifiable claim' problem the signing work set out to close.
record_turn now resolves a signer through the same levels as the header
author, so the signature always proves the header's key claim:
1. A selected delegated agent identity signs with its own key. If the
identity resolves but its keypair is not on disk, the turn records
unsigned rather than signing as someone else — a signature by any
other key would contradict the claim.
2. No selection: plus-tag attribution claims the default identity's
key, and the turn signs with it, exactly like `atomic record`.
3. No identities at all: unsigned, legacy behavior unchanged.
Attribution and signing share one selected-identity loader, so they can
never name different identities. TurnRecordOptions gains an identity_dir
override (mirroring AgentAuthorOptions) threading to the author, the
signing key, and the envelope's delegation URN — set for testing, None
in production.
End-to-end tests drive record_turn the way hooks do against a real repo
and store: a selected agent identity yields a change whose SIGNATURE
section carries the agent's DID and verifies against the agent's public
key (and not the human's); no selection signs with the default identity;
no store records unsigned.
…ectors Replace the hand-written RFC 8785 walker in atomic-canonical/src/jcs.rs with a call into serde_json_canonicalizer. Member ordering and string escaping were already correct; number formatting was not. Section 3.2.2.3 requires the ECMAScript Number::toString algorithm, and serde_json's formatter crosses between decimal and exponent notation at different magnitudes and writes negative zero as -0.0, so two conforming implementations hashed the same logical document to different digests. The delegate formats through ryu_js, which is the variant the section names, and serializes through an explicit heap stack rather than the call stack. canonicalize keeps its signature, so no call site changes anywhere in the workspace. Eleven measured cases are pinned as fixtures under atomic-canonical/tests/vectors/ with a harness in tests/jcs_vectors.rs. Three conformance cases this entry point cannot decide are recorded in atomic-canonical/tests/vectors/INGEST-BOUNDARY.md rather than tested here: a repeated object member is gone before canonicalize is reached, and RFC 8785 admits both an integer past 2^53 and a fractional number. All of them want a strict decoder on the raw bytes at the boundary where documents arrive.
The Format job runs cargo fmt --all -- --check, which rejected two statements in atomic-canonical/tests/jcs_vectors.rs. No behaviour change.
Contributor
|
@astrogilda I just merged the other PR and now it looks like there is a conflict. Would you like to resolve them? I'm happy to do it if not. |
Contributor
Author
|
@graywolf336 traveling for a bit, don't have access to my laptop. Could you please do it? |
Contributor
|
@astrogilda you got it! I'll notify you to review once I get to it. |
Two conflicts, both additive: dev delegated canonicalize to serde_json_canonicalizer, this branch added admit_document ahead of the parse. Neither displaces the other, so both are kept and no functional code differs from what git had already placed. - jcs.rs: the conflict was the module doc alone. dev's account of the delegation is kept, with its reason the ECMAScript number algorithm cannot be hand-written; this branch's claim that numbers come out of serde_json's formatter is dropped, because dev replaced that formatter. dev's forward reference to a strict decoder at the boundary becomes a description of the decoder that now exists. - Cargo.lock: serde_json_canonicalizer and tempfile both belong, sorted. INGEST-BOUNDARY.md was dev's in-tree brief for this branch, so "what closes all four" is retensed to past. The title still holds: canonicalize still cannot decide those cases from a parsed value. cargo test --workspace passes, including dev's 11 JCS vectors alongside this branch's 14 boundary tests. clippy and doc add no new warnings.
Contributor
|
Wait, no. Something went wrong...looking into this |
Every reader of a stored grant admits it under the I-JSON profile before verifying anything: load_for_delegate, and grant list, verify, push and revoke. The writers did not. mint signs a maxChanges of 2^53 or more, or a Unicode noncharacter in a name or description, and the grant is then stored, exported or printed and skipped by every reader on the machine. Add delegation::encode_for_storage, which renders the indented form the store has always held and admits those bytes before returning them, and use it in grant new, agent create and agent renew. agent create now encodes before it saves the agent identity, so a refusal leaves no identity behind without its certificate. Tests: the largest safe count is stored and found by load_for_delegate; 2^53 and a noncharacter description are refused with the reader's own fault. Deleting the admission call in encode_for_storage fails exactly the two refusal tests.
The comment promised a warn log for every stored certificate the function skips, and there is no logging call in it or a logging dependency in the crate. Say what happens instead.
… number canonicalize expects serde_json_canonicalizer never to fail on a Value. That holds only while serde_json's arbitrary_precision feature is off: with it on, a Value keeps 1e400 as text, the delegate parses it to infinity and errors, and the expect panics on input anyone can supply. Features unify across the workspace, so any new dependency could turn it on. Pin the premise with a test.
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
5 participants
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.
Pull request 202 moved canonicalization onto serde_json_canonicalizer, and this is the reading half, branched from dev at bcbe2c5. The canonicalize entry point takes a value somebody already parsed, and four conformance cases cannot be decided from one.
v0f4f2093061d303fserde_jsonkeeps the last of the two, so the repeat is gone beforecanonicalizerunsvd94ac70c9f0d84bfcanonicalizereturnsString, so it has nowhere to put a refusalv679f56481420e45a9007199254740993v97f5d8777e514257412.5in a signed fieldSo I added the jcs-admit dependency and one function beside canonicalize, which admits the raw bytes and then parses those same bytes. Eight call sites move onto it: the delegation header, the identity store, and six grant reads in the CLI. An accepted value still comes from the bytes that arrived, so a certificate keeps its content hash and its proof. Nine tests cover the four cases, two boundary refusals, and three accept controls; cargo test, workspace clippy, and cargo doc are clean.
The four vectors above come from https://github.com/probityai/agent-evidence-vectors, and jcs-admit is at https://github.com/probityai/jcs-admit.