The signed trail names which account asked for a change, never which one approved it

[vladimirrott]

The transaction chain signs caller_principal, captured in handle_preview at crates/sysknife-daemon/src/dispatcher.rs:2106, so a verified row answers which account asked for an action. ChainIdentity::V3 states why the field exists (crates/sysknife-daemon/src/audit_chain.rs:466):

v2 answers "an Admin did this". On a host with two members of sysknife-admin their signed records were indistinguishable, so the trail could not answer the first question an investigation asks.

The approval chain never got the same treatment. EventContent::canonical_bytes signs six fields, and no account appears among them (crates/sysknife-daemon/src/audit_chain.rs:934):

seq, key_id, kind, transaction_id, receipt_digest, created_at

TransactionStore::approve_transaction takes a transaction id and nothing else (crates/sysknife-daemon/src/transactions.rs:548), while handle_approve holds a CallerAttribution and passes it to authorize_for_transaction a few lines earlier (crates/sysknife-daemon/src/dispatcher.rs:1361). claim_approved_for_execution is the same (transactions.rs:652): the account that spends the receipt is never written down.

What it costs

Two admins on one host, alice at uid 1000 and bob at uid 1001, both in sysknife-admin. Alice's agent previews AddAuthorizedKey, bob types sysknife approve <txid>, alice's agent executes. The verified trail then holds:

record	what it names
transaction row	caller_principal = uid:1000, alice, who previewed
approval_granted	transaction id, receipt digest
approval_consumed	transaction id, receipt digest

Nothing names bob. The receipt exists so that a human authorises what an agent proposed, and the chain cannot say which human that was.

docs/automatic-rollback.md:130 describes the signed fields as "the authorization decision — who approved what, at what risk level". They record who asked.

Why this is more than an ALTER TABLE

transactions versions its signed encoding per row through chain_version, added in migration 2 so rows written by an older binary keep verifying (crates/sysknife-daemon/src/transactions.rs:171). audit_events has no such column in either backend (transactions.rs:179, crates/sysknife-daemon/src/store/postgres.rs:119). Add a field to EventContent without versioning the encoding and every event row already on disk reports Broken, because its signature covers the six-field message.

Suggested shape

1. Migration 4 in both SQLITE_MIGRATIONS and the Postgres MIGRATIONS: audit_events.chain_version defaulting to 1, audit_events.caller_principal nullable, for the reason spelled out in the migration-2 comment.
2. An EventIdentity enum beside ChainIdentity, where LegacyV1 signs today's six fields and V2 appends the principal, with event_message selecting per row.
3. Thread CallerPrincipal into approve_transaction, claim_approved_for_execution and revoke_unconsumed_approval, from the CallerAttribution the handlers already hold.
4. Keep sysknife audit verify correct over a mixed-version event chain.

Acceptance

• An event row written before the change still verifies, against a committed fixture rather than one the test regenerates.
• A grant and a consume by different uids are distinguishable in the chain.
• Editing a stored caller_principal on an event reports Broken.
• Both backends. crates/sysknife-daemon/tests/postgres_store.rs:298 is the pattern for the Postgres side.

Difficulty: hard. Two storage backends, a signed encoding, and a compatibility fixture.

Getting started

CONTRIBUTING.md has the build and test commands, docs/architecture.md covers the trust boundary, and docs/the-audit-chain.md explains what the chain signs today. No CLA and no copyright waiver. The project is MIT.

The Postgres half needs a local database. podman run -d -e POSTGRES_USER=sysknife -e POSTGRES_PASSWORD=sysknife -e POSTGRES_DB=sysknife_test -p 5432:5432 docker.io/library/postgres:17-alpine, then run the contract suite with SYSKNIFE_TEST_POSTGRES_URL and SYSKNIFE_REQUIRE_POSTGRES=1 set and -- --include-ignored.

Comment here before you start. The migration ordering is worth agreeing on before any code exists.

[vladimirrott]

The premise of this issue changed yesterday, and the change is worth recording before someone picks it up expecting the situation described above.

#334 made the daemon refuse an approve, cancel, inspect or execute from any account other than the one that created the transaction. caller_principal is now read back and compared, not merely recorded.

So the question "which account approved this" is currently answerable by inference: the approver is necessarily the requester, and the requester is signed into the transaction row. The scenario in the body, two members of sysknife-admin whose approvals are indistinguishable, cannot happen while that rule holds.

What this issue is now about

Not a hole in what the trail can answer. A dependency on a rule holding.

An investigator reading the chain today derives the approver from a constraint enforced elsewhere in the code. That is weaker than reading it, in three ways:

• Relaxing the rule silently un-answers every historical row. If delegated approval is ever added, and there are reasons to want it, the chain has no record of who approved anything before or after.
• The inference is invisible in the artifact. sysknife audit export gives an auditor rows that do not say who approved; they have to know the daemon's authorization rule, and which version enforced it.
• EventContent::canonical_bytes still signs six fields and no account, so the approval chain cannot corroborate the transaction chain. Two independent records saying the same thing is the point of having two chains.

The implementation sketch in the body is unchanged and still correct. What changed is the argument for doing it, which is now about durability rather than a live gap.

Relabelling from hard to hard (unchanged) but dropping it below #251 in priority, since that one is a live gap and this one is now a hardening step.

[vladimirrott]

@V3RNE42 assigning this to you as the other maintainer, because it is a design decision about the signed format rather than a patch, and it needs someone who can say what the chain is for.

One update since the body was written: #334 landed in v0.12.0 and binds a transaction to the account that created it, so approve, cancel, inspect and execute now require principal equality with the creator. That narrows this issue without closing it. The transaction chain still answers "which account asked", and by construction the approver is now the same account, but EventContent::canonical_bytes still signs six fields and none of them is an account. So a verified approval event remains unable to name anyone on its own, and an investigator has to join it against the transaction row and trust that join.

The open question is whether that join is good enough or whether the event chain should sign the principal itself, which is a format change and a migration. That is your call as much as mine.

[vladimirrott]

@V3RNE42 I am putting this one back in the pool.

It has carried your name since 18 August and the thread has been quiet, and it is help-wanted work that somebody outside can take. The test I used across the four issues sitting on you is whether the work needs our admin access. This one does not.

[vladimirrott]

@Georgefifth, this one is yours if you want it.

You landed #424 last night in store.rs, declaring CHAIN_ROW_COLUMNS once, so you have already been inside the code this touches. #249 is the next question the chain cannot answer. A verified row says which account asked for a change, and nothing in EventContent::canonical_bytes says which account approved it.

It is labelled hard, and the hard part is choosing what to sign without breaking verification of rows written under the current format. ChainIdentity::V3 in crates/sysknife-daemon/src/audit_chain.rs is the precedent for how that was handled last time, including what it cost.

No deadline. If you would rather have something smaller this week, say so and I will find you one.

[vladimirrott]

Rechecked at b2c823e6. Every argument in this issue holds and six of its path:line anchors have moved, most of them a long way. Corrected here rather than in the body, because the body's anchors sit inside prose that still reads correctly with the new numbers.

$ grep -n 'async fn handle_preview' crates/sysknife-daemon/src/dispatcher.rs
2194:async fn handle_preview(
$ grep -n 'caller_principal: caller.principal()' crates/sysknife-daemon/src/dispatcher.rs
2336:        caller_principal: caller.principal(),
$ grep -n 'struct EventContent\|pub fn canonical_bytes' crates/sysknife-daemon/src/audit_chain.rs
534:    pub fn canonical_bytes(&self) -> Vec<u8> {
974:pub struct EventContent<'a> {
989:    pub fn canonical_bytes(&self) -> Vec<u8> {
$ grep -n 'pub fn approve_transaction\|pub fn claim_approved_for_execution\|pub fn revoke_unconsumed_approval' crates/sysknife-daemon/src/transactions.rs
542:    pub fn approve_transaction(
601:    pub fn revoke_unconsumed_approval(
655:    pub fn claim_approved_for_execution(
$ grep -n 'async fn handle_approve\|authorize_for_transaction(' crates/sysknife-daemon/src/dispatcher.rs | head -5
885:async fn authorize_for_transaction(
1533:async fn handle_approve(
1540:    if !authorize_for_transaction(
1730:    if !authorize_for_transaction(

So: handle_preview is dispatcher.rs:2194 and the capture is :2336, not :2106. EventContent::canonical_bytes is audit_chain.rs:989, not :934, and note there is a second canonical_bytes at :534 for a different type, so grep by the struct rather than the method. approve_transaction is transactions.rs:542, claim_approved_for_execution is :655, and handle_approve is dispatcher.rs:1533 with the authorization call at :1540.

ChainIdentity::V3's doc comment is the one anchor that did not move:

$ sed -n '466p' crates/sysknife-daemon/src/audit_chain.rs
    /// `chain_version = 3`. Everything v2 binds, plus the individual account.

The substance is unchanged, and the part that matters most is still exactly six fields with no principal among them:

$ sed -n '989,999p' crates/sysknife-daemon/src/audit_chain.rs
    pub fn canonical_bytes(&self) -> Vec<u8> {
        let mut buf = Vec::with_capacity(256);
        push_field(&mut buf, "seq", &self.seq.to_string());
        push_field(&mut buf, "key_id", self.key_id);
        push_field(&mut buf, "kind", self.kind.as_str());
        push_field(&mut buf, "transaction_id", self.transaction_id);
        push_field(&mut buf, "receipt_digest", self.receipt_digest);
        push_field(&mut buf, "created_at", self.created_at);
        buf
    }
}

Migration 4 is still the next free version on both backends (crates/sysknife-daemon/src/transactions.rs:199 and crates/sysknife-daemon/src/store/postgres.rs:135 are the version 3 entries), and audit_events still carries no chain_version in either. One small correction to the Postgres anchor: the CREATE TABLE IF NOT EXISTS audit_events is crates/sysknife-daemon/src/store/postgres.rs:117, not :119.

[Georgefifth]

Taking this. I've been in store.rs and around the chain in #424, so the V3 precedent in audit_chain.rs is where I'll start: what goes into canonical_bytes for the approver identity, and how a V4 row keeps V3 rows verifiable. Will post the design question here before the patch if the signing boundary has more than one defensible answer.

[Georgefifth]

Design notes before code, per the issue's request to agree the shape first. Following the suggested sketch, with four decisions worth stating explicitly:

1. Migration 4 on both backends — next free version on each (SQLITE_MIGRATIONS and Postgres MIGRATIONS both end at 3):

ALTER TABLE audit_events ADD COLUMN chain_version INTEGER NOT NULL DEFAULT 1;
ALTER TABLE audit_events ADD COLUMN caller_principal TEXT;

Nullable/defaulted for the migration-2 reason: existing rows were signed over the six-field encoding, and any backfill would rewrite their message. DEFAULT 1 makes every historical row LegacyV1 without touching it.

2. EventIdentity beside ChainIdentity, appended suffix, exhaustive match. LegacyV1 signs today's six fields byte-identically; V2 appends event_version + caller_principal fields, same prefix-free framing, same "each arm spells its own fields out" discipline. event_message selects per row, so sysknife audit verify stays correct over a mixed chain.

3. Which events get a principal. approval_granted signs the approver, approval_consumed signs the executor, approval_revoked signs the revoker — the acceptance criterion is that a grant and a consume by different uids are distinguishable. Status events stay LegacyV1: they are written from update_terminal_status inside spawned execution tasks with no CallerAttribution in scope, and the issue's scope threads the principal through exactly the three approval functions. Signing an executor-less status event with the transaction creator's principal would be a claim the code cannot support. If a later change wants status attribution, that is its own encoding decision, not this one's.

4. Guards mirror the transaction chain. A V2 row with an empty/absent caller_principal is Broken (a row naming nobody must not pass for one naming an account), and an unknown chain_version fails the same way the transaction chain fails one — named as un-reproducible rather than guessed at.

Compatibility fixture, per the acceptance wording ("a committed fixture rather than one the test regenerates"): the legacy event row's hex signature is precomputed under the fixed test key and committed as a literal in the test, alongside its exact field values. The test reconstructs the LegacyV1 message and checks it against the committed hash — so if anyone edits the six-field encoding, the committed hash stops matching even though the test could trivially recompute it. That is the point of committing it.

Threading: CallerPrincipal (the type the transaction chain already signs, rendered uid:1000 / token:vsock / none:unattributed) goes into approve_transaction, claim_approved_for_execution, revoke_unconsumed_approval on TransactionStore, the AuditStore trait and both backends, from the CallerAttribution the handlers already hold. The revoke-on-undelivered-response path in handle_approve signs the same caller that granted, which is the account that triggered the revoke.

Starting now; will paste the fixture's committed hash and the mutation runs in the PR.