Skip to content

feat(allowlist): add Bicep (.bicep) support - #525

Merged
lizhengfeng101 merged 2 commits into
alibaba:mainfrom
mittalpk:feat/bicep-allowlist
Jul 27, 2026
Merged

lizhengfeng101 merged 2 commits into
alibaba:mainfrom
mittalpk:feat/bicep-allowlist

Conversation

@mittalpk

Copy link
Copy Markdown
Contributor

Closes #523. Part of #470.

What

Adds .bicep to the code-review allowlist and maps it to a new dedicated bicep.md review-rule doc.

Rule doc coverage

bicep.md follows the same structure as the existing graphql.md/terraform.md (precision-over-recall framing, security findings blocking / style non-blocking):

  • Hardcoded secrets/connection strings instead of Key Vault references, and credential-looking parameters missing @secure()
  • Overly permissive RBAC role assignments (broad built-in roles at wide scope where sibling assignments use narrower scope) and network security group rules open to */Internet on sensitive ports
  • Insecure resource defaults — public network access enabled with no compensating network ACLs, outdated minimumTlsVersion, disabled supportsHttpsTrafficOnly
  • Unpinned module references and inconsistent api-version usage relative to sibling resources
  • Structural issues (unused/undeclared parameters, duplicate resource symbolic names)

Tests

  • allowed_ext_test.go: .bicep/.BICEP allowlist cases
  • system_rules_test.go: rule-resolution case for .bicep resolving to bicep.md
  • Every new assertion confirmed to fail against pre-fix code before the corresponding change landed

Verification

  • go vet, go build, go test on the two touched packages (internal/config/allowlist, internal/config/rules): clean, all passing
  • gofmt -l: clean

@github-actions

github-actions Bot commented Jul 27, 2026 •

Copy link
Copy Markdown
Contributor

🔍 OpenCodeReview found 1 issue(s) in this PR.

  • ✅ Successfully posted inline: 1 comment(s)

Comment thread internal/config/rules/system_rules.json
@CLAassistant

CLAassistant commented Jul 27, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@lizhengfeng101 lizhengfeng101 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review: feat(allowlist): add Bicep (.bicep) support

Critical Bug: Julia (.jl) support accidentally removed

The PR replaces .jl with .bicep in supported_file_types.json instead of appending .bicep:

-  ".jl"
+  ".bicep"

Additionally, all Julia-related test cases are removed:

  • allowed_ext_test.go: .jl/.JL assertions replaced by .bicep/.BICEP
  • TestIsExcludedPath: Julia test-file exclusion patterns deleted entirely
  • system_rules_test.go: Julia resolution tests replaced with Bicep tests

Meanwhile, system_rules.json still maps "**/*.jl": "julia.md" and the julia.md rule doc remains on disk — creating an inconsistency where the rule mapping exists but .jl files would be filtered out at the allowlist stage and never reach rule resolution.

Fix: append .bicep after .jl in supported_file_types.json, restore all Julia test cases, and add Bicep test cases alongside them (net addition, not replacement).

Minor

  • The bicep.md rule doc itself is well-written and follows the existing precision-over-recall structure — no issues there.
  • Consider explicitly listing common database ports (3306, 5432, 1433, 27017) in the "sensitive port" clause for precision.

Summary

Request changes due to the unintentional Julia regression. The Bicep additions are good — they just need to be additive rather than replacing existing language support.

@mittalpk

Copy link
Copy Markdown
Contributor Author

Thanks for catching this — confirmed the root cause: my local main was stale (missing #501, the Julia support merge) when I built this branch, so restoring my working changes onto a freshly-cut branch from the current main silently replaced the newer Julia content with my older snapshot instead of merging.

Fixed in 490fd3d:

  • Restored .jl in supported_file_types.json alongside .bicep (both present now, not one replacing the other)
  • Restored the .jl → julia.md mapping in system_rules.json alongside .bicep → bicep.md
  • Restored the .jl/.JL cases in allowed_ext_test.go's TestIsAllowedExt, and the three Julia entries in TestIsExcludedPath that were dropped, alongside the Bicep cases
  • Restored both Julia rows in system_rules_test.go's TestResolve_DefaultRules, alongside the Bicep row
  • Applied the minor suggestion too — bicep.md's sensitive-port clause now explicitly lists MySQL/3306, PostgreSQL/5432, SQL Server/1433, MongoDB/27017 alongside SSH/RDP

Verified the diff against upstream/main is now purely additive across all four files (every line is a +, nothing removed) and go test ./internal/config/allowlist/... ./internal/config/rules/... passes for both Julia and Bicep cases together.

mittalpk added a commit to mittalpk/open-code-review that referenced this pull request Jul 27, 2026
Same root cause as the sibling Bicep PR (alibaba#525): the local main used to
build this branch's working changes predated the Julia support merge
(alibaba#501), so restoring those changes onto a branch freshly cut from the
current upstream/main silently replaced the newer .jl allowlist entry
and its test cases with the stale pre-Julia state instead of adding
HCL/Terraform alongside them.

Restores .jl in supported_file_types.json, the TestIsAllowedExt and
TestIsExcludedPath Julia cases in allowed_ext_test.go, and the Julia
resolution cases in system_rules_test.go -- system_rules.json's
.jl -> julia.md mapping was already restored in a prior commit on
this branch. Now a pure addition of HCL/Terraform support, not a
replacement of Julia support.
lizhengfeng101 pushed a commit that referenced this pull request Jul 27, 2026
* feat(allowlist): add HCL/Terraform (.hcl, .tfvars) support

Adds .hcl and .tfvars to the allowlist and maps them, along with the
already-supported .tf, to a new dedicated terraform.md review rule
doc covering hardcoded secrets/credentials, overly permissive
network/IAM access, state file hygiene, and lifecycle protection on
stateful resources. .tf previously fell through to the generic
default rule set with no Terraform-specific guidance despite already
being allowlisted; bringing all three extensions under one doc gives
consistent coverage rather than leaving .tf behind.

Part of #470, closes #522.

* Update internal/config/rules/system_rules.json

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* fix: restore Julia (.jl) support accidentally removed

Same root cause as the sibling Bicep PR (#525): the local main used to
build this branch's working changes predated the Julia support merge
(#501), so restoring those changes onto a branch freshly cut from the
current upstream/main silently replaced the newer .jl allowlist entry
and its test cases with the stale pre-Julia state instead of adding
HCL/Terraform alongside them.

Restores .jl in supported_file_types.json, the TestIsAllowedExt and
TestIsExcludedPath Julia cases in allowed_ext_test.go, and the Julia
resolution cases in system_rules_test.go -- system_rules.json's
.jl -> julia.md mapping was already restored in a prior commit on
this branch. Now a pure addition of HCL/Terraform support, not a
replacement of Julia support.

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

@lizhengfeng101 lizhengfeng101 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

rebase remote main

mittalpk added 2 commits July 27, 2026 14:55
Adds .bicep to the allowlist and maps it to a new dedicated bicep.md
review rule doc covering hardcoded secrets/connection strings, missing
@secure() on sensitive parameters, overly permissive RBAC role
assignments, and insecure resource defaults (public network access,
missing TLS enforcement).

Part of alibaba#470, closes alibaba#523.
…explicitly

The previous commit on this branch was built from a local main that
predated the Julia (.jl) support merge (alibaba#501), so restoring stashed
changes onto a freshly-branched, up-to-date main silently replaced the
newer .jl allowlist entry, its system_rules.json mapping, and its
allowed_ext_test.go / system_rules_test.go cases with the stale
pre-Julia state instead of adding Bicep alongside them.

Restores every removed Julia entry (allowlist, TestIsExcludedPath
cases, system_rules.json mapping, and both test files) so this is now
a pure addition of Bicep support, not a replacement of Julia support.

Also lists common database ports (MySQL/3306, PostgreSQL/5432, SQL
Server/1433, MongoDB/27017) explicitly in bicep.md's sensitive-port
clause per review feedback.
@mittalpk
mittalpk force-pushed the feat/bicep-allowlist branch from 490fd3d to 63056a6 Compare July 27, 2026 12:57
@mittalpk

Copy link
Copy Markdown
Contributor Author

rebased

@lizhengfeng101 lizhengfeng101 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@lizhengfeng101
lizhengfeng101 merged commit 0804075 into alibaba:main Jul 27, 2026
7 checks passed
Githab-capibara added a commit to Githab-capibara/open-code-review that referenced this pull request Sep 17, 2026
* feat(allowlist): add HCL/Terraform (.hcl, .tfvars) support

Adds .hcl and .tfvars to the allowlist and maps them, along with the
already-supported .tf, to a new dedicated terraform.md review rule
doc covering hardcoded secrets/credentials, overly permissive
network/IAM access, state file hygiene, and lifecycle protection on
stateful resources. .tf previously fell through to the generic
default rule set with no Terraform-specific guidance despite already
being allowlisted; bringing all three extensions under one doc gives
consistent coverage rather than leaving .tf behind.

Part of alibaba#470, closes alibaba#522.

* Update internal/config/rules/system_rules.json

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* fix: restore Julia (.jl) support accidentally removed

Same root cause as the sibling Bicep PR (alibaba#525): the local main used to
build this branch's working changes predated the Julia support merge
(alibaba#501), so restoring those changes onto a branch freshly cut from the
current upstream/main silently replaced the newer .jl allowlist entry
and its test cases with the stale pre-Julia state instead of adding
HCL/Terraform alongside them.

Restores .jl in supported_file_types.json, the TestIsAllowedExt and
TestIsExcludedPath Julia cases in allowed_ext_test.go, and the Julia
resolution cases in system_rules_test.go -- system_rules.json's
.jl -> julia.md mapping was already restored in a prior commit on
this branch. Now a pure addition of HCL/Terraform support, not a
replacement of Julia support.

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Githab-capibara added a commit to Githab-capibara/open-code-review that referenced this pull request Sep 17, 2026
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.

feat(allowlist): add Bicep (.bicep) support

3 participants