Non-blocking findings from the review of #1766 (subject access logs). Line numbers refer to 8ae9467.
- Exemption audit can record an entry that was never stored.
crates/registry-breg/src/subject_access_log.rs:183-205 writes the exemption audit ("begin", then "authorized") before the log INSERT, inside the read transaction. If that transaction rolls back, the operational audit shows a delayed entry that never landed.
- Nothing checks trusted intermediaries at authoring time.
crates/registry-breg/src/evidence_source.rs:440 enables forwardAccessAttribution whenever the entity declares accessLog, but nothing checks that the Evidence connection's client is listed in trustedIntermediaries. If it isn't, BReg refuses the forwarded headers and every lookup fails at runtime. That fails safe, but bregctl or evidencectl source add could catch it before deployment.
- The Evidence export protocol change needs a changelog entry. The exported protocol moves from
breg-evidence-lookup-v1 to -v2, and access-attribution-v1 is added to readSemantics for every entity, whether logged or not (evidence_source.rs:519). This changes the behaviorRevision of every exported Evidence source and needs a breaking-change entry in the release notes.
ACCESS-LOG.md doesn't cover every read path. It doesn't say whether these reads are logged:
- change-request views
- reads inside Rhai action handlers
- hook and webhook payloads
- exports
Non-blocking findings from the review of #1766 (subject access logs). Line numbers refer to 8ae9467.
crates/registry-breg/src/subject_access_log.rs:183-205writes the exemption audit ("begin", then "authorized") before the logINSERT, inside the read transaction. If that transaction rolls back, the operational audit shows a delayed entry that never landed.crates/registry-breg/src/evidence_source.rs:440enablesforwardAccessAttributionwhenever the entity declaresaccessLog, but nothing checks that the Evidence connection's client is listed intrustedIntermediaries. If it isn't, BReg refuses the forwarded headers and every lookup fails at runtime. That fails safe, butbregctlorevidencectl source addcould catch it before deployment.breg-evidence-lookup-v1to-v2, andaccess-attribution-v1is added toreadSemanticsfor every entity, whether logged or not (evidence_source.rs:519). This changes thebehaviorRevisionof every exported Evidence source and needs a breaking-change entry in the release notes.ACCESS-LOG.mddoesn't cover every read path. It doesn't say whether these reads are logged: