From 68e9fa6f6c135e254e22a5623dc02e9676d7139d Mon Sep 17 00:00:00 2001 From: Alex Dunmow Date: Tue, 25 Aug 2026 22:56:49 +0800 Subject: [PATCH] fix: ignore proto output permission drift --- .../adr/0004-ignore-generated-file-permissions.md | 15 +++++++++++++++ proto_freshness.go | 11 ++++++++++- proto_freshness_test.go | 14 ++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 docs/adr/0004-ignore-generated-file-permissions.md diff --git a/docs/adr/0004-ignore-generated-file-permissions.md b/docs/adr/0004-ignore-generated-file-permissions.md new file mode 100644 index 0000000..db2b3bc --- /dev/null +++ b/docs/adr/0004-ignore-generated-file-permissions.md @@ -0,0 +1,15 @@ +# Ignore permission bits when checking generated protobuf freshness + +The protobuf freshness checker introduced by ADR 0003 copies generated outputs into an isolated temporary sandbox, regenerates them, and compares the resulting state. The CMS workspace inherits a default ACL that creates ordinary generated files with group-write permission, while the system temporary directory creates the same files without that permission. Comparing complete file modes therefore reported every generated Go and TypeScript file as stale even when its content was byte-identical. + +The checker now compares generated output type, content digest, and symlink target, but deliberately ignores ordinary permission bits. Generated protobuf artifacts are source files rather than executables, and Git does not preserve the group-write distinction that caused the false positive. File additions, removals, content changes, and regular-file-to-symlink changes remain visible. + +Rejecting the sandbox isolation was not acceptable because it would restore concurrent mutation races in the scanned repository. Reproducing workspace ACLs inside every sandbox was also rejected because ACL support and inheritance vary by filesystem and host. + +Consequences: + +- `proto_freshness.go` no longer treats `0660` versus `0644` as generated drift. +- `proto_freshness_test.go` locks the permission-independent comparison behavior. +- Content, type, symlink-target, addition, and removal checks remain unchanged. + +Keywords: proto freshness, generated protobufs, permission bits, file mode, default ACL, 0660, 0644, sameProtoOutputState, proto_freshness.go, proto_freshness_test.go diff --git a/proto_freshness.go b/proto_freshness.go index cafd39f..d63d192 100644 --- a/proto_freshness.go +++ b/proto_freshness.go @@ -293,10 +293,19 @@ func diffProtoOutputs(before, after map[string]protoOutputState) []string { changes = append(changes, "?? "+path) case !existsAfter: changes = append(changes, " D "+path) - case beforeState != afterState: + case !sameProtoOutputState(beforeState, afterState): changes = append(changes, " M "+path) } } return changes } + +// sameProtoOutputState intentionally ignores permission bits. Generated files +// can inherit different default ACLs in the source workspace and the isolated +// sandbox even when their type and content are identical. +func sameProtoOutputState(left, right protoOutputState) bool { + return left.mode.Type() == right.mode.Type() && + left.digest == right.digest && + left.linkTarget == right.linkTarget +} diff --git a/proto_freshness_test.go b/proto_freshness_test.go index 0d22dc7..32142c3 100644 --- a/proto_freshness_test.go +++ b/proto_freshness_test.go @@ -34,6 +34,20 @@ func TestDiffProtoOutputsIsDeterministic(t *testing.T) { } } +func TestDiffProtoOutputsIgnoresPermissionDifferences(t *testing.T) { + digest := [sha256.Size]byte{1} + before := map[string]protoOutputState{ + "packages/api/src/example.ts": {mode: 0o660, digest: digest}, + } + after := map[string]protoOutputState{ + "packages/api/src/example.ts": {mode: 0o644, digest: digest}, + } + + if got := diffProtoOutputs(before, after); len(got) != 0 { + t.Fatalf("diffProtoOutputs() = %v, want permissions-only change ignored", got) + } +} + func TestCheckProtoGeneratedFreshnessInSandboxLeavesSourceUntouched(t *testing.T) { repoRoot := newProtoFreshnessTestRepo(t, "@printf 'generated\\n' > packages/api/src/example.ts") tempParent := t.TempDir()