fix(tl): serialize ingestion, recover leaves, and derive certificate status - #127
csnitker-godaddy wants to merge 3 commits into
Conversation
…atus Signed-off-by: Connor Snitker <csnitker@godaddy.com>
kperry-godaddy
left a comment
There was a problem hiding this comment.
The ingest gate, startup and pre-read recovery, migration 006, the writer lock and the family-max expiry rule fix real defects, and the tests exercise them against a real Tessera log; that part I would merge. What shapes my read is checkAgentState. It makes the Transparency Log a second lifecycle arbiter, with semantic dedup that ANS-4 §8 does not define, and its rejections are permanent 422s. That interacts badly with two things on the RA side: the inline activation seal rebuilds its leaf with a fresh now on retry instead of replaying persisted bytes, and the outbox has no per-agent ordering at this base. I would split the policy layer out (or hold it) and fix the seal replay in the RA, where the root cause lives.
Four things I'd settle before this ships, all inline. Non-blocking notes below.
Not blocking, worth a look
- Badge and status token disagree on a malformed
notAfter:certExpiresAtfalls back toexpiresAt(badge ACTIVE) whilecurrentCertFingerprintsreturns an error (token 500, permanently, for that agent). Ingest never validatesnotAfter, andCertificateExtendedleaves it optional although the arrays may now carry lapsed certificates. One shared parser with one policy, plus RFC3339 validation inEvent.Validate, closes it. WithAntispamcannot fire: every leaf carries a fresh UUIDv7 logId and a fresh attestation signature, so no two appends share Tessera's identity hash. IfIsDupever came back true,StoreEventwould be called with the original leaf index and hitUNIQUE(leaf_index). Remove it and theIsDuplicateplumbing, or honour it before the mirror write.- The conflict tests assert only
err != nil; three of the six scenarios are rejected by the new INVALID_EVENT name checks rather than by the state logic they are named for, andSTALE_AGENT_EVENT, the foreign-producer rule and the DEPRECATED transitions have no test at all. AwantCodecolumn and a second producer key in the testbed cover that. - Operator steps exist only in the PR body: stop the old writer, back up, one writer per data directory, the one-time
tl_eventsrebuild and its disk cost, recovery before the listener opens, what "index extends beyond the integrated log" means. Compose's health check marks the TL unhealthy after roughly 55s of startup work. An Upgrading section in README or docs/ carries them past the merge. - Contract text: state conflicts sit under 422 "Body fails validation" while the handler already maps conflicts to 409; the raId ownership rule is undocumented; the identity route still says "same dedup / same codes" although
checkAgentStateskips identity envelopes;/v2/internal/agents/eventis registered but absent from the spec.LatestAgentStatealso binds a versioned name to its first agentId forever, across producers and past revocation, and nothing says so.
| if rec.EventType == string(event.TypeAgentRevoked) || | ||
| env.EventType() == string(event.TypeAgentRegistered) || | ||
| (rec.EventType == env.EventType() && env.EventType() != string(event.TypeAgentRenewed)) || | ||
| (rec.EventType == string(event.TypeAgentDeprecated) && env.EventType() != string(event.TypeAgentRevoked)) { | ||
| return nil, domain.NewValidationError("AGENT_STATE_CONFLICT", "event duplicates or reverses an existing lifecycle state") | ||
| } | ||
| oldTime, oldErr := time.Parse(time.RFC3339, prior.Timestamp) | ||
| newTime, newErr := time.Parse(time.RFC3339, env.Timestamp()) | ||
| if oldErr != nil || newErr != nil { | ||
| return nil, domain.NewValidationError("INVALID_EVENT", "event timestamp is invalid") | ||
| } | ||
| if newTime.Before(oldTime) { | ||
| return nil, domain.NewValidationError("STALE_AGENT_EVENT", "event predates the current agent state") | ||
| } |
There was a problem hiding this comment.
These rejections assume per-agent in-order delivery and apply to AGENT_REVOKED. STALE_AGENT_EVENT compares producer-supplied timestamps with no skew window and no exemption for revocation, so an RA clock step backwards between two events makes the revocation unappendable and the agent stays ACTIVE here. At this base the RA outbox has no per-agent ordering (a row in backoff is overtaken by later rows) and the TL client treats every non-429 4xx as permanent, so once #128 emits AGENT_RENEWED through the outbox, one transient error followed by a later event turns a signed, RA-committed statement into a permanent absence from the log, with the row retrying at max backoff forever.
Worth settling before this ships, because it decides what the log is: a witness that seals every verified statement, or an arbiter. Revocation should never be rejected on ordering or time; the identity lane already treats it as terminal by existence (HasIdentityRevoked). Either seal every verified same-producer statement and derive current state at read time (any AGENT_REVOKED on the stream wins, otherwise the latest producer timestamp with leaf_index as tiebreak), or, if rejection stays, return 409 with a distinct code so the producer can retry after its predecessor, add a bounded skew window, and land per-agent FIFO plus a dead-letter state in the RA outbox first. The minimum change if nothing else moves:
if env.EventType() != string(event.TypeAgentRevoked) {
oldTime, oldErr := time.Parse(time.RFC3339, prior.Timestamp)
newTime, newErr := time.Parse(time.RFC3339, env.Timestamp())
if oldErr != nil || newErr != nil {
return nil, domain.NewValidationError("INVALID_EVENT", "event timestamp is invalid")
}
if newTime.Before(oldTime) {
return nil, domain.NewValidationError("STALE_AGENT_EVENT", "event predates the current agent state")
}
}| // checkAgentState checks the database before any append. An unchanged state | ||
| // retried with fresh timestamps maps to its original leaf. Renewal is repeatable | ||
| // when its certificate/attestation state changes; a UNIQUE(FQDN,version,status) | ||
| // constraint would incorrectly suppress every renewal after the first. | ||
| // | ||
| //nolint:nilnil // A nil record and nil error mean the new state is eligible for append. | ||
| func (s *LogService) checkAgentState(ctx context.Context, env event.Signable, canonical []byte) (*sqlitetl.EventRecord, error) { | ||
| if _, ok := env.(*identityevent.Envelope); ok { | ||
| return nil, nil | ||
| } |
There was a problem hiding this comment.
checkAgentState strips timestamp and issuedAt to decide that two differently-signed events are the same state, re-encodes domain.ValidTransitions as a four-clause predicate, and pins each agent to the raId of its first event, all inside the I/O orchestrator and testable only through the Tessera testbed. ANS-4 §4 and §8 define dedup as byte-identical only, and none of these rules has a spec home yet. The semantic dedup exists for one producer behaviour: sealActivationEvent rebuilds the activation leaf with a fresh now on retry instead of persisting and replaying the signed bytes, which is the rule the outbox-replay invariant already imposes on worker rows. When a retry after a seal-then-crash differs in anything beyond the timestamps (a DNSSEC AD bit, an optional record), it gets a permanent AGENT_STATE_CONFLICT: the agent stays PENDING_DNS, the RA refuses the name, this log holds it, and registration.go still calls the orphaned leaf benign residue.
Worth settling before this ships. Fix the root cause in the RA (persist {innerEventCanonical, producerSignature} for the activation leaf before calling SealAgentEvent and replay it verbatim on retry) and reduce this function to what only the log can check: canonical name and version (which belong in Event.Validate) plus the binding guard that a versioned FQDN is not bound to a different agentId. eventStateBytes, the raId rule, the transition predicate and both new codes can then go, or be held until the ANS-4 amendment lands and the transition rule is a pure table-driven function shared with the domain. That also lets migration 006, the gate and the expiry work merge on their own.
| func (s *LogService) RecoverIndex(ctx context.Context) error { | ||
| select { | ||
| case s.ingestGate <- struct{}{}: | ||
| defer func() { <-s.ingestGate }() | ||
| case <-ctx.Done(): | ||
| return ctx.Err() | ||
| } | ||
| s.indexReady = false | ||
| if err := s.recoverIndex(ctx); err != nil { | ||
| return err | ||
| } | ||
| s.indexReady = true | ||
| return nil | ||
| } | ||
|
|
||
| // lockIndexedRead linearizes current-state reads with append/index writes. | ||
| // After a failed mirror write, a healthy SQLite read must not mint an ACTIVE | ||
| // token from an older row while a revocation is already committed in Tessera. | ||
| func (s *LogService) lockIndexedRead(ctx context.Context) (func(), error) { | ||
| select { | ||
| case s.ingestGate <- struct{}{}: | ||
| case <-ctx.Done(): | ||
| return nil, ctx.Err() | ||
| } | ||
| unlock := func() { <-s.ingestGate } | ||
| if !s.indexReady { | ||
| if err := s.recoverIndex(ctx); err != nil { | ||
| unlock() | ||
| return nil, fmt.Errorf("log: recover event index before read: %w", err) | ||
| } | ||
| s.indexReady = true | ||
| } | ||
| return unlock, nil | ||
| } |
There was a problem hiding this comment.
internal/tl has no logger, and this change adds the paths that most need one: startup and in-request recovery, the fail-closed indexReady flips in log.go, the lifecycle rejections, and the writer lock. After a failed mirror write every read and write returns 500 while the TL's own log shows only "listening"; a restored leaf, which is evidence of an earlier failed write, leaves no trace; a cross-producer conflict is visible only in the RA's outbox log. The startup failure is logged once, generically, by main.
Worth fixing before this ships since CLAUDE.md treats a silent component as a defect, and the wiring is small: a logger zerolog.Logger field on LogService (default zerolog.Nop()) with a WithLogger option that tags component=tl-log, applied from cmd/ans-tl/main.go. Then INFO on recovery start and end with firstUnindexedLeaf, indexedSize, logSize, the restored count and duration; WARN or ERROR with leafIndex, logId, agentId where indexReady flips; WARN with agentId, ansName, eventType, existingLeafIndex before each AGENT_STATE_CONFLICT and STALE_AGENT_EVENT; INFO with the path on writer-lock acquire. For this function:
s.indexReady = false
s.logger.Info().Msg("recovering event index")
if err := s.recoverIndex(ctx); err != nil {
s.logger.Error().Err(err).Msg("event index recovery failed; reads and ingest stay fail-closed")
return err
}
s.indexReady = true
s.logger.Info().Msg("event index recovered")
return nil| select { | ||
| case s.ingestGate <- struct{}{}: | ||
| defer func() { <-s.ingestGate }() | ||
| case <-ctx.Done(): | ||
| return nil, ctx.Err() | ||
| } | ||
| if !s.indexReady { | ||
| if err := s.recoverIndex(ctx); err != nil { | ||
| return nil, fmt.Errorf("log: recover event index: %w", err) | ||
| } | ||
| s.indexReady = true | ||
| } |
There was a problem hiding this comment.
The gate acquired here is held across KMS signing, s.log.Append (whose Tessera future has no context and, with BatchSize: 1, fsyncs entry bundle, tiles and tree state per leaf) and StoreEvent, and every current-state read takes the same single slot through lockIndexedRead. Reads therefore exclude each other and wait on write-path disk I/O; a stalled tiles filesystem wedges badges, status tokens, receipts and audit behind the 30s timeout while /v2/admin/health and /ready keep returning 200, because indexReady is unexported. Before this change reads served from SQLite through such a stall. The property the gate needs is narrower: no read while the mirror may lag Tessera.
Worth fixing before this ships. A writer mutex serializes check-then-append; an RWMutex (or weighted semaphore) covers only the mirror-lag window, that is recovery and the gap between a resolved append and its StoreEvent, and is never held across future(); readers take the shared side and upgrade only to run recovery. Bound the Tessera wait with the request context and treat an abandoned append as an uncertain write that forces one more recovery. Export a Ready(ctx) error (index ready plus a 2s-bounded IntegratedSize) and have /v2/admin/ready return 503 while it errors, so an orchestrator can see the state instead of routing to an instance that answers 500 on every real route.
Signed-off-by: Connor Snitker <csnitker@godaddy.com>
Signed-off-by: Connor Snitker <csnitker@godaddy.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical recovery, lifecycle-state, and status-token correctness findings remain.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 4
Open (4)
What changed in this PR
This PR hardens TL ingestion and recovery, aligns certificate-derived status behavior, and makes RA activation replay durable without changing public wire shapes.
Changes:
- Adds serialized ingestion, recovery, readiness checks, antispam, and writer locking.
- Adds lifecycle validation and durable activation seals.
- Updates certificate status handling, schemas, migrations, documentation, and tests.
| File | Summary |
|---|---|
spec/api-spec-v2.yaml |
Documents activation conflicts. |
spec/api-spec-tl-v2.yaml |
Documents lifecycle deduplication and validation codes. |
internal/tl/service/statustoken.go |
Filters expired certificates and bounds token lifetime. Critical finding (1 vote): duplicate fingerprints can bypass expiry filtering. |
internal/tl/service/statustoken_internal_test.go |
Updates status-token tests. |
internal/tl/service/schemas/V2.json |
Documents lapsed certificate evidence. |
internal/tl/service/schemas/V1.json |
Documents legacy certificate evidence. |
internal/tl/service/receipt_test.go |
Extends receipt fixtures. |
internal/tl/service/readiness.go |
Adds index readiness checks. Moderate finding (1 vote): readiness can remain healthy while the writer is stalled. |
internal/tl/service/readiness_test.go |
Tests readiness locking. |
internal/tl/service/log.go |
Serializes appends and fences indexed reads. Critical finding (1 vote): latest-state reads can report an active state after a terminal event; nit (1 vote): successful appends lack structured INFO logging. |
internal/tl/service/ingest.go |
Adds lifecycle validation and leaf recovery. Critical findings (1 vote each): V0 recovery is unsupported, and duplicate historical leaves can resurrect terminal states. Moderate finding (1 vote): noncanonical host spellings can cause state conflicts. |
internal/tl/service/ingest_test.go |
Tests ingestion, recovery, and lifecycle behavior. |
internal/tl/service/envelope_wrapper.go |
Derives family-based certificate expiry. Moderate finding (1 vote): malformed V1 singleton expiry values can make badge and token paths disagree. |
internal/tl/service/envelope_wrapper_test.go |
Tests certificate overlap handling. |
internal/tl/service/certificate_status_test.go |
Tests badge and token expiry consistency. |
internal/tl/service/badge.go |
Shares lifecycle status derivation. |
internal/tl/receipt/statustoken.go |
Caps token expiration by certificate validity. |
internal/tl/logstore/writerlock.go |
Adds process-level writer locking. |
internal/tl/logstore/writerlock_test.go |
Tests writer-lock exclusivity. |
internal/tl/logstore/log.go |
Integrates locking and antispam. |
internal/tl/logstore/log_test.go |
Tests duplicate suppression. |
internal/tl/event/v1/event.go |
Validates V1 expiry evidence. |
internal/tl/event/event.go |
Validates V2 expiry evidence. |
internal/ra/service/registration.go |
Persists and replays activation evidence. |
internal/ra/service/registration_test.go |
Updates outbox test doubles. |
internal/ra/service/lifecycle.go |
Supports durable activation replay. |
internal/ra/service/activation_replay_test.go |
Tests exact activation replay. |
internal/domain/attested_expiry.go |
Adds shared expiry parsing. |
internal/domain/attested_expiry_test.go |
Tests expiry parsing. |
internal/adapter/store/sqlitetl/migrations/006_recover_duplicate_leaves.sql |
Preserves duplicate leaves for recovery. |
internal/adapter/store/sqlitetl/migrations/005_agent_state_index.sql |
Adds lifecycle indexing. |
internal/adapter/store/sqlitetl/migration_test.go |
Tests migration preservation. |
internal/adapter/store/sqlitetl/events.go |
Adds gap recovery and lifecycle queries. |
internal/adapter/store/sqlite/migrations/014_activation_seals.sql |
Adds activation-seal storage. |
internal/adapter/store/sqlite/activation_seals.go |
Implements activation-seal persistence. |
internal/adapter/store/sqlite/activation_seals_test.go |
Tests seal persistence across restart. |
internal/adapter/docsui/openapi/tl.yaml |
Mirrors TL contract documentation. |
internal/adapter/docsui/openapi/ra.yaml |
Mirrors RA contract documentation. |
go.mod |
Adds synchronization and system dependencies. |
docs/operations/tl-recovery-upgrade.md |
Documents upgrade and recovery procedures. |
cmd/ans-tl/main.go |
Wires recovery, locking, and readiness. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| oldTime, oldErr := time.Parse(time.RFC3339, prior.Timestamp) | ||
| newTime, newErr := time.Parse(time.RFC3339, env.Timestamp()) |
| case header.SchemaVersion == eventv1.SchemaVersion: | ||
| env = &eventv1.Envelope{} | ||
| default: | ||
| return nil, fmt.Errorf("unsupported stored schema %q", header.SchemaVersion) |
| unlock, err := s.lockIndexedRead(ctx) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer unlock() |
| if fp == "" || seen[fp] { | ||
| continue | ||
| } | ||
| if raw, ok := m["notAfter"]; ok { | ||
| value, _ := raw.(string) |

Repeated submissions of the same event return the original leaf. A timestamp-only repeat of the latest agent state is also a duplicate. Conflicting or stale lifecycle changes are rejected, while a renewal with changed certificate evidence can append for the same FQDN/version.
Committed leaves remain recoverable after an interrupted write. Recovery preserves historical duplicate leaves and their signed evidence. Current status cannot be served from an out-of-date event index.
Badges and status tokens use
min(max(server notAfter), max(identity notAfter))for the required certificate families. Expired overlap certificates do not expire their replacements. Expired agents receive no status token; tokens include only unexpired certificate fingerprints and cannot outlive any certificate they authorize.Contract comparison
The existing
AppendResponseinspec/api-spec-tl-v2.yamlremains:No response, event-envelope, checkpoint, receipt, or status-token fields change. The existing CBOR
expclaim is bounded by the certificates it authorizes. The OpenAPI changes document duplicate/lifecycle behavior and theAGENT_STATE_CONFLICT/STALE_AGENT_EVENTvalidation codes; the event-schema edits describe lapsed-family evidence.Expiry and lapsed-family semantics depend on agentnameservice/ans-registry#68. The additional semantic lifecycle checks go beyond the current ANS-4 content-hash deduplication text and need corresponding prose clarification. This PR does not claim that clarification is already merged.
Validation
make build,make check, andmake test-racepass on this branch. Coverage is internal 90.83%, domain 100.00%, crypto 98.45%.Regressions cover concurrent V1/V2 submissions, repeated events, conflicting lifecycle changes, successive renewals, interrupted writes, restart recovery, historical duplicate leaves, and preservation of signed evidence. Certificate tests cover both family orderings, V1/V2 overlap, expiry boundaries, signed status, and token lifetime.
Third PR in the deployment-fix stack. Back up the log data and stop the existing TL process before upgrading. Only one TL writer may use a log data directory. The upgrade preserves the Merkle log's history.
Fixes #123.
Depends on #126; this PR targets
fix/acme-owner-validationto keep its review diff scoped.AI assistance
Assisted-by: Codex (GPT-6), under Connor Snitker's direction.