Skip to content

Commit dcd9c9e

Browse files
committed
daemon: address Copilot review on checkpoint anchor (#871)
- verify --against-anchor: drop the pre-sort and validate strict monotonicity in anchor FILE ORDER. The emitter only ever writes a head past the highest it already anchored, so file order is strictly increasing by construction and an out-of-order/duplicate record is itself the tamper signal — sorting first laundered "seq 2 then seq 1" into a passing "1,2". The last record in file order is therefore the genuine latest head. Adds an out-of-order regression test. - checkpoint.Verify: correct the docstring to distinguish (false, err) — the artifact could not be evaluated — from (false, nil) — parsed but the signature does not match. Behaviour unchanged; only the contract is now accurate. - syslog sink: strip recordLine's trailing newline before writing, so one checkpoint is one syslog message rather than risking a split/multi-line entry.
1 parent 808b05b commit dcd9c9e

4 files changed

Lines changed: 42 additions & 7 deletions

File tree

daemon/internal/anchor/syslog_unix.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
package anchor
44

55
import (
6+
"bytes"
67
"fmt"
78
"log/syslog"
89
"sync"
@@ -37,13 +38,18 @@ func OpenSyslog(tag string) (*SyslogLog, error) {
3738
return &SyslogLog{w: w, now: time.Now}, nil
3839
}
3940

40-
// Write emits one record line to syslog. A write error means the record was
41-
// not accepted.
41+
// Write emits one record as a single syslog message. recordLine produces a
42+
// newline-terminated NDJSON line for file/git sinks; syslog frames each call as
43+
// its own message, so the trailing newline is stripped — leaving it in can make
44+
// some syslog backends split one record into multiple log entries. The record
45+
// JSON is itself single-line (no interior newlines), so trimming the final byte
46+
// is sufficient to keep one checkpoint == one syslog event.
4247
func (s *SyslogLog) Write(eventType string, payload []byte) error {
4348
line, err := recordLine(s.now(), eventType, payload)
4449
if err != nil {
4550
return err
4651
}
52+
line = bytes.TrimRight(line, "\n")
4753
s.mu.Lock()
4854
defer s.mu.Unlock()
4955
if _, err := s.w.Write(line); err != nil {

daemon/internal/checkpoint/checkpoint.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,17 @@ func Sign(cp Checkpoint, signer Signer) (Signed, error) {
9393

9494
// Verify checks s's signature against the PEM/SPKI Ed25519 public key. It
9595
// re-canonicalises the embedded Checkpoint body (the wrapper fields are never
96-
// signed) and validates the detached signature. A malformed signature, wrong
97-
// key type, or mismatch returns (false, err) — checkpoint verification failure
98-
// is surfaced, never swallowed.
96+
// signed) and validates the detached signature.
97+
//
98+
// The two failure modes are distinct, so callers can tell a malformed artifact
99+
// from a genuine forgery:
100+
// - (false, err): the signature, key, or canonical body could not even be
101+
// evaluated (bad multibase prefix, wrong length, unparseable key, etc.).
102+
// - (false, nil): everything parsed but the signature does not match the body
103+
// (a real verification failure / forgery).
104+
//
105+
// Only (true, nil) means the checkpoint is authentic. Either failure must be
106+
// treated as a hard rejection — never swallowed.
99107
func Verify(s Signed, publicKeyPEM string) (bool, error) {
100108
if len(s.Signature) < 2 {
101109
return false, errors.New("checkpoint signature too short")

daemon/internal/verifycli/anchor.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package verifycli
22

33
import (
44
"fmt"
5-
"sort"
65

76
"github.com/agent-receipts/ar/daemon/internal/checkpoint"
87
)
@@ -43,7 +42,13 @@ func verifyAgainstAnchor(anchorPath, chainID, pubPEM string, headSeq int64, head
4342
return anchorResult{Reason: fmt.Sprintf("no verified checkpoint found for chain %s in anchor %s", chainID, anchorPath)}
4443
}
4544

46-
sort.Slice(cps, func(i, j int) bool { return cps[i].Sequence < cps[j].Sequence })
45+
// Validate strict monotonicity in ANCHOR FILE ORDER, never after a re-sort.
46+
// The daemon appends a chain's checkpoints in strictly increasing sequence by
47+
// construction (the emitter only writes a head past the highest it already
48+
// anchored), so an out-of-order or duplicate record in the file is itself the
49+
// tamper/corruption signal. Sorting first would launder "seq 2 then seq 1"
50+
// into "1, 2" and pass — so the last record in file order is also the genuine
51+
// latest head, not a max() that hides reordering.
4752
for i := 1; i < len(cps); i++ {
4853
if cps[i].Sequence <= cps[i-1].Sequence {
4954
return anchorResult{

daemon/internal/verifycli/anchor_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,3 +173,19 @@ func TestAnchorRejectsNonMonotonicLog(t *testing.T) {
173173
t.Errorf("reason %q does not flag the ordering problem", got.Reason)
174174
}
175175
}
176+
177+
func TestAnchorRejectsOutOfOrderLog(t *testing.T) {
178+
signer, pub := newAnchorSigner(t)
179+
// seq 2 written before seq 1: the monotonic check runs in anchor FILE ORDER,
180+
// so this must fail. Sorting first would launder it into 1,2 and pass, hiding
181+
// a reordered/tampered log — guards against re-introducing a sort.
182+
path := writeAnchor(t, signer, cp("c", 2, "sha256:2"), cp("c", 1, "sha256:1"))
183+
184+
got := verifyAgainstAnchor(path, "c", pub, 2, "sha256:2", true)
185+
if got.OK {
186+
t.Fatal("expected FAIL for an out-of-order checkpoint log")
187+
}
188+
if !strings.Contains(got.Reason, "increasing") {
189+
t.Errorf("reason %q does not flag the ordering problem", got.Reason)
190+
}
191+
}

0 commit comments

Comments
 (0)