Skip to content

feat(allowlist): add Protocol Buffers (.proto) support - #473

Open
chethanuk wants to merge 2 commits into
alibaba:mainfrom
chethanuk:fix/issue-470-featallowlist-expand-supported-file-types-language
Open

feat(allowlist): add Protocol Buffers (.proto) support#473
chethanuk wants to merge 2 commits into
alibaba:mainfrom
chethanuk:fix/issue-470-featallowlist-expand-supported-file-types-language

Conversation

@chethanuk

Copy link
Copy Markdown
Contributor

Summary

Adds Protocol Buffers as the first language-group contribution for tracking issue #470.

.proto files were skipped by both review paths because they were missing from the extension allowlist. This PR:

  1. Allows .proto / .PROTO via supported_file_types.json
  2. Maps **/*.proto → a dedicated protobuf.md rule doc (wire compatibility focus)
  3. Adds table-driven tests for allowlist + rule resolution

No Go source changes. No exclude patterns (proto has no standard test-file convention per #470).

Why this was needed

diff / scan path
  → user include/exclude
  → allowedext.IsAllowedExt(ext)   ← .proto returned false → ExcludeExtension
  → allowedext.IsExcludedPath
  → SystemRule.Resolve             ← unmapped ext fell back to default.md

After this PR:

.foo.proto
  → IsAllowedExt(".proto") = true
  → Resolve("**/*.proto") → rule_docs/protobuf.md  (Wire Compatibility)

Changes

File Change
internal/config/allowlist/supported_file_types.json append .proto
internal/config/allowlist/allowed_ext_test.go IsAllowedExt cases for .proto / .PROTO
internal/config/rules/system_rules.json "**/*.proto": "protobuf.md"
internal/config/rules/rule_docs/protobuf.md new — wire compat, enums, RPC, security
internal/config/rules/system_rules_test.go Resolve cases for .proto paths

Rule doc focus

  • Field numbers / wire + JSON compatibility
  • Message/field design (optional, map/repeated, oneof)
  • Enum zero sentinels
  • RPC/streaming design
  • Any / unbounded payloads / secrets in defaults

Test plan

  • go test -run 'TestIsAllowedExt|TestResolve_DefaultRules' ./internal/config/allowlist/ ./internal/config/rules/
  • go test ./internal/config/...
  • make check
  • make test
  • grep -l "Wire Compatibility" rule_docs/* → only protobuf.md

Out of scope

Part of #470.

Pin govulncheck to v1.6.0 instead of @latest. The argument is
reproducibility, not supply chain: golang.org/x/vuln is the Go team's
own module and go install is already checksum-verified via GOSUMDB.
What @latest costs is a new govulncheck release turning CI red on an
unchanged tree. This gate is load-bearing - e6e5da0 bumped the Go image
to fix GO-2026-5856 after govulncheck caught it - so protecting it from
unrelated churn is worth a pin.

Pinning costs nothing in scan freshness: the vulnerability database is
fetched at runtime, independently of the binary version. Verified:
Scanner govulncheck@v1.6.0, DB updated 2026-07-08, no vulnerabilities,
exit 0.

Narrow release.yml's workflow-level permissions from contents: write to
contents: read. Only the build job inherits it, and it only checks out
and uploads artifacts - upload-artifact authenticates with
ACTIONS_RUNTIME_TOKEN, not GITHUB_TOKEN. The release and npm-publish
jobs declare their own job-level permissions, which replace the
inherited set entirely, so both are unaffected.

Verified with actionlint (clean across all workflows) and by confirming
the release.yml still parses as YAML.
Allow .proto files through the extension allowlist and map them to a
dedicated protobuf review rule focused on wire compatibility.

Part of alibaba#470.
@github-actions

Copy link
Copy Markdown
Contributor

OpenCodeReview: No comments generated. Looks good to me.

@lizhengfeng101

Copy link
Copy Markdown
Collaborator

Review

The proto support itself looks good — clean changes, well-structured rule doc, and solid test coverage. Happy to see this land as part of #470.

However, this PR includes two unrelated CI changes that should be removed:

1. .github/workflows/release.yml — permissions downgrade (blocking)

permissions:
-  contents: write
+  contents: read

This will break the release workflow. Creating GitHub Releases and uploading artifacts on tag push requires contents: write. Please revert this change.

2. .github/workflows/ci.yml — govulncheck version pin (out of scope)

-  go install golang.org/x/vuln/cmd/govulncheck@latest
+  go install golang.org/x/vuln/cmd/govulncheck@v1.6.0

Pinning is good practice, but it's unrelated to proto support. Please submit this separately if desired.

Verdict

Remove the two CI file changes and the proto part is ready to merge.

@lizhengfeng101 lizhengfeng101 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants