Skip to content

Cancel and the TTL sweep end an approval without revoking it or writing an event #251

Description

@vladimirrott

revoke_unconsumed_approval ends an unspent approval properly: it deletes the row from transaction_approvals and appends ApprovalRevoked in the same database transaction (crates/sysknife-daemon/src/transactions.rs:607). One caller uses it, the undelivered-response path in handle_approve (crates/sysknife-daemon/src/dispatcher.rs:1426).

The two paths an operator reaches do neither:

  • cancel_queued flips status to Canceled in one UPDATE (transactions.rs:738), which is what handle_cancel calls (dispatcher.rs:1556).
  • cleanup_stale_queued cancels every queued row past the 15-minute TTL in one UPDATE (transactions.rs:715), on a 300-second ticker started at crates/sysknife-daemon/src/main.rs:133.

Neither touches transaction_approvals, and neither appends an event. Postgres matches: crates/sysknife-daemon/src/store/postgres.rs:725 and :707.

Approve a transaction and then cancel it, and the event chain holds approval_granted with no terminal event, while the row stays in transaction_approvals with consumed_at IS NULL for good. docs/the-audit-chain.md:222 sets the expectation those events exist to meet:

The transaction chain records what the daemon authorized. It says nothing about whether a human then approved it, whether that approval was spent, or whether it was retracted.

A reader of the verified chain cannot separate "granted, still valid, the operator is deciding" from "granted, then cancelled". The two look identical.

The sweep hits approved transactions too. Its comment at main.rs:128 says it "only touches expired Queued rows (never Running/Approved)", and there is no Approved state in JobState (crates/sysknife-types/src/lib.rs:503): approval writes a row in a separate table and leaves status at Queued. So a preview that a human approved and nobody executed is swept on the next tick, and the sweep logs a count without naming what it cancelled.

What this is not

The receipt stays unspendable after a cancel. claim_approved_for_execution requires status = Queued (transactions.rs:667-678) and nothing moves a row back to Queued. This costs completeness of the trail, not privilege, and the fix should not be sold as closing an escalation.

Suggested shape

  1. Factor the delete-plus-ApprovalRevoked body out of revoke_unconsumed_approval into a helper that runs inside a caller-supplied transaction.
  2. cancel_queued opens an Immediate transaction, applies the status UPDATE, and calls that helper when the UPDATE affected a row.
  3. cleanup_stale_queued does the same per row it cancels, so the event count matches the sweep count.
  4. Mirror both in crates/sysknife-daemon/src/store/postgres.rs, where append_event is already available.

Acceptance

  • Approve then cancel: no unconsumed row survives in transaction_approvals, and the event chain ends in approval_revoked for that transaction.
  • Cancelling a transaction that was never approved writes no event.
  • A TTL sweep over three approved and expired transactions writes three events, and sysknife audit verify reports the event chain intact.
  • Both backends.

Difficulty: medium.

Getting started

CONTRIBUTING.md has the build and test commands, and docs/developer-guide.md covers the setup steps and how to reproduce each required check locally. No CLA and no copyright waiver. The project is MIT.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

bugSomething isn't workingclaimedSomeone has said in the thread that they are working on thishelp wantedExtra attention is neededmediumDifficulty: needs familiarity with one subsystem

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions