From 16505f6054439de18f5e2ab3745012e5c8672991 Mon Sep 17 00:00:00 2001 From: Catherine Date: Sat, 30 May 2026 14:31:55 +0000 Subject: [PATCH] [security] Actually check result of `appendNewAuditRecord`. Before this commit, if `backend.AppendAuditLog` failed, the request would proceed anyway. This is contrary to the explicit contract and design intent. If Go had `#[must_use]`, this would not have happened :/ V12-Ref: F-77170 --- src/audit.go | 43 ++++++++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/src/audit.go b/src/audit.go index 630027e..1aa454c 100644 --- a/src/audit.go +++ b/src/audit.go @@ -363,65 +363,78 @@ func notifyAudit(ctx context.Context, id AuditID) { func (audited *auditedBackend) CommitManifest( ctx context.Context, name string, manifest *Manifest, opts ModifyManifestOptions, -) (err error) { +) error { domain, project, ok := strings.Cut(name, "/") if !ok { panic("malformed manifest name") } - audited.appendNewAuditRecord(ctx, &AuditRecord{ + + if err := audited.appendNewAuditRecord(ctx, &AuditRecord{ Event: AuditEvent_CommitManifest.Enum(), Domain: proto.String(domain), Project: proto.String(project), Manifest: manifest, - }) + }); err != nil { + return err + } return audited.Backend.CommitManifest(ctx, name, manifest, opts) } func (audited *auditedBackend) DeleteManifest( ctx context.Context, name string, opts ModifyManifestOptions, -) (err error) { +) error { domain, project, ok := strings.Cut(name, "/") if !ok { panic("malformed manifest name") } - audited.appendNewAuditRecord(ctx, &AuditRecord{ + + if err := audited.appendNewAuditRecord(ctx, &AuditRecord{ Event: AuditEvent_DeleteManifest.Enum(), Domain: proto.String(domain), Project: proto.String(project), - }) + }); err != nil { + return err + } return audited.Backend.DeleteManifest(ctx, name, opts) } -func (audited *auditedBackend) ExpireManifest(ctx context.Context, name string) (err error) { +func (audited *auditedBackend) ExpireManifest(ctx context.Context, name string) error { domain, project, ok := strings.Cut(name, "/") if !ok { panic("malformed manifest name") } - audited.appendNewAuditRecord(ctx, &AuditRecord{ + + if err := audited.appendNewAuditRecord(ctx, &AuditRecord{ Event: AuditEvent_ExpireManifest.Enum(), Domain: proto.String(domain), Project: proto.String(project), - }) + }); err != nil { + return err + } return audited.Backend.ExpireManifest(ctx, name) } -func (audited *auditedBackend) FreezeDomain(ctx context.Context, domain string) (err error) { - audited.appendNewAuditRecord(ctx, &AuditRecord{ +func (audited *auditedBackend) FreezeDomain(ctx context.Context, domain string) error { + if err := audited.appendNewAuditRecord(ctx, &AuditRecord{ Event: AuditEvent_FreezeDomain.Enum(), Domain: proto.String(domain), - }) + }); err != nil { + return err + } return audited.Backend.FreezeDomain(ctx, domain) } -func (audited *auditedBackend) UnfreezeDomain(ctx context.Context, domain string) (err error) { - audited.appendNewAuditRecord(ctx, &AuditRecord{ +func (audited *auditedBackend) UnfreezeDomain(ctx context.Context, domain string) error { + if err := audited.appendNewAuditRecord(ctx, &AuditRecord{ Event: AuditEvent_UnfreezeDomain.Enum(), Domain: proto.String(domain), - }) + }); err != nil { + return err + } return audited.Backend.UnfreezeDomain(ctx, domain) }