From caeeaa5cb707f590d98fdbe71b25dcac7e00929b Mon Sep 17 00:00:00 2001 From: ChethanUK Date: Thu, 23 Jul 2026 19:01:57 +0200 Subject: [PATCH] feat(allowlist): add Protocol Buffers (.proto) support Allow .proto files through the extension allowlist and map them to a dedicated protobuf review rule focused on wire compatibility. Part of #470. --- internal/config/allowlist/allowed_ext_test.go | 2 + .../allowlist/supported_file_types.json | 3 +- internal/config/rules/rule_docs/protobuf.md | 39 +++++++++++++++++++ internal/config/rules/system_rules.json | 1 + internal/config/rules/system_rules_test.go | 2 + 5 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 internal/config/rules/rule_docs/protobuf.md diff --git a/internal/config/allowlist/allowed_ext_test.go b/internal/config/allowlist/allowed_ext_test.go index fa153d5f..aea7132b 100644 --- a/internal/config/allowlist/allowed_ext_test.go +++ b/internal/config/allowlist/allowed_ext_test.go @@ -46,6 +46,8 @@ func TestIsAllowedExt(t *testing.T) { {".TFVARS", true}, {".bicep", true}, {".BICEP", true}, + {".proto", true}, + {".PROTO", true}, {".txt", false}, {".md", false}, {".png", false}, diff --git a/internal/config/allowlist/supported_file_types.json b/internal/config/allowlist/supported_file_types.json index be73b443..c976805d 100644 --- a/internal/config/allowlist/supported_file_types.json +++ b/internal/config/allowlist/supported_file_types.json @@ -77,5 +77,6 @@ ".jl", ".hcl", ".tfvars", - ".bicep" + ".bicep", + ".proto" ] diff --git a/internal/config/rules/rule_docs/protobuf.md b/internal/config/rules/rule_docs/protobuf.md new file mode 100644 index 00000000..5d06a342 --- /dev/null +++ b/internal/config/rules/rule_docs/protobuf.md @@ -0,0 +1,39 @@ +> Favor precision over recall: only raise an issue when you are confident it is a real defect, and stay silent when the surrounding context is unclear — a false alarm costs more reviewer trust than a missed minor issue. Treat security and correctness findings as blocking, and style or idiom suggestions as non-blocking. + +#### Obvious Typos or Spelling Errors +- Spelling errors in message, field, enum, enum-value, service, or rpc names at their declaration sites; do not report spelling errors at reference sites +- Comments or option strings with spelling errors that affect readability of the public API surface + +#### Field Numbers and Wire Compatibility +- Reused or renumbered field tags that break existing clients or servers (Wire Compatibility) +- Changing a field's type, label (`optional`/`repeated`/`required`), or oneof membership in a way that breaks wire or JSON compatibility +- Deleting a field without adding both its number and name to `reserved` +- Renaming a field without `json_name` consideration when JSON clients depend on the old name +- Do not flag purely additive new fields with fresh numbers, or documentation-only comment changes + +#### Message and Field Design +- Missing `optional` (proto3) where absence must be distinguishable from the zero value +- `map` used where order matters, or `repeated` used where key lookup would be clearer +- oneof fields that leave an invalid zero-state representable when an explicit sentinel was intended +- Nested messages that re-encode the same domain concept already modeled elsewhere in the package +- Do not report stylistic preference for `message` vs `group` (groups are legacy) when the schema is already consistent + +#### Enums and Defaults +- First enum value is not a zero `*_UNSPECIFIED` (or equivalent) sentinel +- Relying on implicit zero defaults across schema versions when clients treat zero as meaningful data +- Inserting new enum values in the middle of an existing numeric range used by older clients +- Do not flag additive enum values appended at the end with new numbers + +#### Services and RPC Design +- Non-idempotent methods modeled as if they were safe to retry without client-visible side effects +- Multiple rpcs sharing the same request or response message type when distinct contracts would prevent accidental field coupling +- Unbounded client/server streaming without documented flow control, page size, or deadline expectations +- Missing request or response message wrappers that force primitive/scalar request bodies +- Do not flag standard google.api annotations or well-known types used correctly + +#### Security and Resource Limits +- `google.protobuf.Any` accepted from untrusted input without type allowlisting +- Unbounded `repeated`/`map` fields or recursive message depth on untrusted payloads with no application-level limits +- Secrets, tokens, or credentials embedded in field defaults, examples, or comments +- File paths, URLs, or SQL fragments carried as unconstrained strings without validation guidance at the service boundary +- Do not report when limits are enforced outside the schema and that boundary is clearly documented diff --git a/internal/config/rules/system_rules.json b/internal/config/rules/system_rules.json index 98b6c262..0dd10066 100644 --- a/internal/config/rules/system_rules.json +++ b/internal/config/rules/system_rules.json @@ -24,6 +24,7 @@ "**/*.c": "c.md", "**/*.py": "python.md", "**/*.{php,phtml}": "php.md", + "**/*.proto": "protobuf.md", "**/*.po": "po.md", "**/*.pot": "pot.md", "**/*.{graphql,gql}": "graphql.md", diff --git a/internal/config/rules/system_rules_test.go b/internal/config/rules/system_rules_test.go index 60a77881..10aca01b 100644 --- a/internal/config/rules/system_rules_test.go +++ b/internal/config/rules/system_rules_test.go @@ -102,6 +102,8 @@ func TestResolve_DefaultRules(t *testing.T) { {"modules/network/vpc.hcl", "Overly Permissive Access"}, {"envs/prod.tfvars", "Hardcoded Secrets"}, {"infra/main.bicep", "Hardcoded Secrets"}, + {"api/v1/user.proto", "Wire Compatibility"}, + {"service.proto", "Wire Compatibility"}, } for _, tt := range tests {