Skip to content

feat(allowlist): add HCL/Terraform (.hcl, .tfvars) support - #524

Merged
lizhengfeng101 merged 3 commits into
alibaba:mainfrom
mittalpk:feat/hcl-terraform-allowlist
Jul 27, 2026
Merged

feat(allowlist): add HCL/Terraform (.hcl, .tfvars) support#524
lizhengfeng101 merged 3 commits into
alibaba:mainfrom
mittalpk:feat/hcl-terraform-allowlist

Conversation

@mittalpk

Copy link
Copy Markdown
Contributor

Closes #522. Part of #470.

What

Adds .hcl and .tfvars to the code-review allowlist and maps them, along with the already-supported .tf, to a new dedicated terraform.md review-rule doc.

.tf was already allowlisted but had no dedicated rule doc — it fell through to the generic default.md rule set. Since .tf/.hcl/.tfvars are all the same HCL syntax family, this brings all three under one consistent, language-specific ruleset rather than adding rules only for the two new extensions and leaving .tf behind.

Rule doc coverage

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

  • Hardcoded secrets/credentials in resource arguments or .tfvars values, and credential-looking variables missing sensitive = true
  • Overly permissive network rules (0.0.0.0/0 on sensitive ports) and wildcard IAM policies
  • Committed .tfstate/.tfstate.backup files (state can contain secrets in plaintext)
  • Weakened/missing lifecycle { prevent_destroy = true } on stateful resources
  • Unbounded provider/module version constraints, inconsistent with sibling pinned versions
  • Structural issues (duplicate resource labels, unused/undeclared variables)

Tests

  • allowed_ext_test.go: .hcl/.HCL/.tfvars/.TFVARS allowlist cases
  • system_rules_test.go: rule-resolution cases for .tf, .hcl, .tfvars all resolving to terraform.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

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.
@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
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@CLAassistant

CLAassistant commented Jul 27, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

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.
@mittalpk

mittalpk commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

my local main was stale (missing #501, the Julia support merge) when I built this branch's working changes, so restoring them onto a freshly-cut branch from the current main silently replaced the newer Julia content instead of merging.

Fixed in 94f8f1d (on top of the system_rules.json mapping already restored in 29a7a9b):

  • Restored .jl in supported_file_types.json alongside .hcl/.tfvars
  • Restored the .jl/.JL cases in TestIsAllowedExt and the three Julia entries in TestIsExcludedPath
  • Restored both Julia rows in system_rules_test.go's TestResolve_DefaultRules

Verified the diff against upstream/main is now purely additive across all four files, and go test ./internal/config/allowlist/... ./internal/config/rules/... passes for Julia, HCL/Terraform, and the existing suite together.

@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 6f6a296 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 Aug 23, 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>
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 HCL/Terraform (.hcl, .tfvars) support

3 participants