ci: add protected public domain allowlists - #2111
Conversation
github-actions
Bot
added
the
size/L
📝 WalkthroughWalkthroughAdds approved-domain contract linting with public and fixture allowlists, strict policy validation, incremental changed-line detection, static Go hostname scanning, violation reporting, and options-aware lint integration. ChangesApproved Domain Contract
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant lintcheck
participant ScanRepoWithOptions
participant scanHardcodedEndpoints
participant scanUnapprovedDomains
participant Git
lintcheck->>ScanRepoWithOptions: ScanRepoWithOptions(root, ChangedFrom)
ScanRepoWithOptions->>scanHardcodedEndpoints: Scan endpoint violations
ScanRepoWithOptions->>scanUnapprovedDomains: Scan domain violations
scanUnapprovedDomains->>Git: Read tracked files and added ranges
scanUnapprovedDomains-->>ScanRepoWithOptions: Return domain violations
scanHardcodedEndpoints-->>ScanRepoWithOptions: Return endpoint violations
ScanRepoWithOptions-->>lintcheck: Return sorted combined violations
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
lint/domaincontract/unapproved.go (2)
337-361: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
ast.Exprassertions.composite.Eltsandpair.Keyare alreadyast.Expr; the assertions at Lines 341 and 351 always succeed and just add nesting.♻️ Simplification
case sequenceComposite: for _, element := range composite.Elts { - if valueExpr, ok := element.(ast.Expr); ok { - s.addHostValue(valueExpr, "host collection") - } + s.addHostValue(element, "host collection") } case mapComposite: for _, element := range composite.Elts { pair, ok := element.(*ast.KeyValueExpr) if !ok { continue } - keyExpr, ok := pair.Key.(ast.Expr) - if !ok { - continue - } - s.addMapPair(keyExpr, pair.Value) + s.addMapPair(pair.Key, pair.Value) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lint/domaincontract/unapproved.go` around lines 337 - 361, Remove the redundant ast.Expr type assertions in the composite literal handling: pass each composite.Elts element directly to addHostValue, and pass pair.Key directly to addMapPair after validating the KeyValueExpr. Keep the existing sequence/map branching and non-KeyValueExpr skip behavior unchanged.
97-97: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy lift
loadTypedGoFilesfailures silently weaken the gate.packages.Loaderrors (and per-packagepkg.Errors) are swallowed into anilmap, and any path-normalization mismatch betweenfilepath.Clean(pkg.CompiledGoFiles[i])and the scanned path (e.g. symlinkedTMPDIRon macOS,/varvs/private/var) drops typed info for that file. WithoutInfo,isHostnameStructField/isHostnameSelectorreturnfalseby design (Lines 587-593), so struct-field and selector hostname evidence disappears while the scan still reports success and the unused-entry pass still runs repo-wide. That turns an environment problem into a false-negative CI pass.Consider surfacing typed-load failure as
inventoryComplete = falseplus anincompleteDomainRuleviolation (or returning the error), rather than degrading quietly.As per coding guidelines: "never silently coerce unsupported inputs, ignore unhonored options, default missing identities, or discard writes."
Also applies to: 208-244
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lint/domaincontract/unapproved.go` at line 97, Update loadTypedGoFiles and its callers to surface packages.Load or per-package pkg.Errors instead of returning a nil or partial typed-file map silently. Propagate an incomplete result through the scan by setting inventoryComplete to false and recording an incompleteDomainRule violation (or returning the load error), while preserving existing typed evidence when loading succeeds; also normalize scanned and compiled paths consistently so symlinked or equivalent paths still match.Source: Coding guidelines
lint/domaincontract/policy_test.go (1)
28-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering the remaining
loadDomainListrejection paths."domain list must not be empty"(comment-only/blank file) and the missing-file/open-error branch (policy.goLines 71-74, 100-102) have no test, so removing either guard wouldn't fail this suite.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lint/domaincontract/policy_test.go` around lines 28 - 95, Extend TestLoadDomainPolicyRejectsInvalidLists with cases for a comment-only or blank domain list, asserting the “domain list must not be empty” error, and for a missing/unopenable domain-list file, asserting the open-error path. Use the existing temporary-root setup and loadDomainPolicy flow while preserving all current rejection cases.lint/domaincontract/unapproved_test.go (1)
15-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated helper bodies.
scanDomainEvidenceandscanTypedDomainEvidenceInPackagerepeat identical parse + collect + sort logic; extracting asortDomainEvidencehelper (and a shared parse step) would keep them in sync.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lint/domaincontract/unapproved_test.go` around lines 15 - 65, Refactor scanDomainEvidence and scanTypedDomainEvidenceInPackage to share the duplicated parse, evidence collection, and sorting logic. Extract a sortDomainEvidence helper and a shared parsing/scanning helper where appropriate, while preserving the existing typed-package setup and error handling behavior.lint/domaincontract/unapproved_repo_test.go (1)
15-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the fixture repo hermetic against ambient Git config. Repo-level
user.name/user.emailand-c commit.gpgsign=falseare set, but a developer's globalcore.hooksPath,init.templateDir, orcommit.gpgsigncan still makeinit/commitfail or mutate the fixture. Pinning the environment keeps these tests deterministic on contributor machines.♻️ Suggested isolation
func gitTestCommand(t *testing.T, root string, args ...string) string { t.Helper() cmd := exec.Command("git", args...) cmd.Dir = root + cmd.Env = append(os.Environ(), + "GIT_CONFIG_GLOBAL=/dev/null", + "GIT_CONFIG_SYSTEM=/dev/null", + "GIT_CONFIG_NOSYSTEM=1", + ) out, err := cmd.CombinedOutput()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lint/domaincontract/unapproved_repo_test.go` around lines 15 - 42, Update setupDomainDiffRepo and its gitTestCommand invocations to isolate Git from ambient configuration: disable or override global core.hooksPath and init.templateDir during repository initialization, and explicitly disable commit signing for the commit operation. Keep the fixture’s existing repository-local identity configuration and ensure initialization and commit cannot execute contributor-provided hooks or templates.lint/domaincontract/policy.go (2)
106-126: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winIDN hostnames can never be approved.
validatePolicyHostnameonly acceptsa-z0-9-labels, so an allowlist entry must be punycode, while the scanner'snormalizeCandidateHostname(unapproved.goLine 517) emits the literal Unicode host. A Unicode hostname in Go source is therefore permanently unapprovable even with CODEOWNER sign-off — there is no escape hatch. If that's the intent (asTestUnapprovedDomainDiffContract/IDN hostname is rejectedsuggests), consider stating it explicitly ininternal/qualitygate/config/README.md; otherwise normalize candidates to A-labels (golang.org/x/net/idna) before policy lookup.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lint/domaincontract/policy.go` around lines 106 - 126, Align policy validation with candidate normalization so IDN hostnames can be approved: update the hostname normalization or policy lookup flow around validatePolicyHostname and normalizeCandidateHostname to convert Unicode hostnames to IDNA A-labels using the existing golang.org/x/net/idna dependency before comparison. Preserve current validation for ASCII hostnames and ensure both allowlist entries and scanner candidates use the same canonical form.
91-93: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSort check depends on byte order, which is only safe because
validatePolicyHostnamerestricts to ASCII. Worth a short comment tying the two invariants together so a future relaxation of the charset doesn't silently break ordering validation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lint/domaincontract/policy.go` around lines 91 - 93, Add a concise comment at the ordering check in the policy validation flow, near validatePolicyHostname and the previous/host comparison, documenting that byte-order sorting is valid because hostnames are restricted to ASCII; note that relaxing the charset requires revisiting this comparison.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lint/domaincontract/diff.go`:
- Around line 79-142: Establish the typed-error contract across the diff flow:
in lint/domaincontract/diff.go lines 79-142, make parseChangedGoPaths and
parseAddedLineRanges return typed validation errors with the required metadata;
in lint/domaincontract/diff.go lines 157-170, wrap Git subprocess failures in
the prescribed lower-layer typed error while preserving the cause. Update
lint/domaincontract/diff_test.go lines 29-33 to assert typed metadata for
truncated rename input, and lines 64-68 to assert typed metadata plus
errors.Is-style cause preservation for malformed patch data.
In `@lint/README.md`:
- Around line 33-38: Update the “Adding a new lint domain” example to avoid
assigning an options-incompatible <domain>.ScanRepo directly to scanner.fn.
Document using an options-aware scanner entry point such as ScanRepoWithOptions,
or an adapter that accepts and forwards opts while preserving the existing
scanning behavior.
---
Nitpick comments:
In `@lint/domaincontract/policy_test.go`:
- Around line 28-95: Extend TestLoadDomainPolicyRejectsInvalidLists with cases
for a comment-only or blank domain list, asserting the “domain list must not be
empty” error, and for a missing/unopenable domain-list file, asserting the
open-error path. Use the existing temporary-root setup and loadDomainPolicy flow
while preserving all current rejection cases.
In `@lint/domaincontract/policy.go`:
- Around line 106-126: Align policy validation with candidate normalization so
IDN hostnames can be approved: update the hostname normalization or policy
lookup flow around validatePolicyHostname and normalizeCandidateHostname to
convert Unicode hostnames to IDNA A-labels using the existing
golang.org/x/net/idna dependency before comparison. Preserve current validation
for ASCII hostnames and ensure both allowlist entries and scanner candidates use
the same canonical form.
- Around line 91-93: Add a concise comment at the ordering check in the policy
validation flow, near validatePolicyHostname and the previous/host comparison,
documenting that byte-order sorting is valid because hostnames are restricted to
ASCII; note that relaxing the charset requires revisiting this comparison.
In `@lint/domaincontract/unapproved_repo_test.go`:
- Around line 15-42: Update setupDomainDiffRepo and its gitTestCommand
invocations to isolate Git from ambient configuration: disable or override
global core.hooksPath and init.templateDir during repository initialization, and
explicitly disable commit signing for the commit operation. Keep the fixture’s
existing repository-local identity configuration and ensure initialization and
commit cannot execute contributor-provided hooks or templates.
In `@lint/domaincontract/unapproved_test.go`:
- Around line 15-65: Refactor scanDomainEvidence and
scanTypedDomainEvidenceInPackage to share the duplicated parse, evidence
collection, and sorting logic. Extract a sortDomainEvidence helper and a shared
parsing/scanning helper where appropriate, while preserving the existing
typed-package setup and error handling behavior.
In `@lint/domaincontract/unapproved.go`:
- Around line 337-361: Remove the redundant ast.Expr type assertions in the
composite literal handling: pass each composite.Elts element directly to
addHostValue, and pass pair.Key directly to addMapPair after validating the
KeyValueExpr. Keep the existing sequence/map branching and non-KeyValueExpr skip
behavior unchanged.
- Line 97: Update loadTypedGoFiles and its callers to surface packages.Load or
per-package pkg.Errors instead of returning a nil or partial typed-file map
silently. Propagate an incomplete result through the scan by setting
inventoryComplete to false and recording an incompleteDomainRule violation (or
returning the load error), while preserving existing typed evidence when loading
succeeds; also normalize scanned and compiled paths consistently so symlinked or
equivalent paths still match.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6f46ef56-94a5-4b8f-896a-2f72e5075e71
📒 Files selected for processing (13)
internal/qualitygate/config/README.mdinternal/qualitygate/config/allowlists/fixture-domains.txtinternal/qualitygate/config/allowlists/public-domains.txtlint/README.mdlint/domaincontract/diff.golint/domaincontract/diff_test.golint/domaincontract/policy.golint/domaincontract/policy_test.golint/domaincontract/scan.golint/domaincontract/unapproved.golint/domaincontract/unapproved_repo_test.golint/domaincontract/unapproved_test.golint/main.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@b1fcb46c3adacca0d43a397c7b9678485d1c7047🧩 Skill updatenpx skills add larksuite/cli#feat/domain-allowlist -y -g |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2111 +/- ##
=======================================
Coverage 75.25% 75.25%
=======================================
Files 916 916
Lines 97167 97167
=======================================
Hits 73121 73121
Misses 18433 18433
Partials 5613 5613 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
8fc138f to
0419c26
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lint/domaincontract/policy.go`:
- Around line 69-75: Update loadDomainList to read the allowlist through the
repository VFS, using vfs.Open or vfs.ReadFile instead of os.Open. Translate
scanner and underlying I/O failures through the repository’s established typed
error boundary rather than returning raw fmt.Errorf, while preserving the
existing path construction and domain-list parsing behavior.
- Around line 49-103: The policy-loading functions loadDomainPolicy and
loadDomainList must use the typed-error contract: wrap hostname, ordering,
empty-list, and cross-list duplicate failures as *errs.ValidationError; wrap
os.Open and scanner read failures as *errs.InternalError with errs.SubtypeFileIO
while preserving the underlying cause. Update policy_test.go coverage to assert
these categories with errors.As rather than matching error-message substrings,
including duplicate and file-I/O cases.
In `@lint/domaincontract/unapproved.go`:
- Around line 367-382: Update fileDomainScan.addMapPair so the both-true
keyOK/valueOK case reports both hostname evidences instead of returning; retain
the early return for both-false and the existing single-sided reporting
behavior. Add a regression test in unapproved_test.go covering a
hostname-semantic map literal with hostname-shaped key and value.
- Around line 97-105: Update loadTypedGoFiles and its caller in
scanUnapprovedDomains to propagate packages.Load failures instead of silently
returning nil. When typed loading fails, mark inventoryComplete false and emit
an incompleteDomainRule/incompleteDomainViolation diagnostic, ensuring the
unused-policy rejection pass is skipped while preserving normal typed scanning
when loading succeeds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 392d238d-525b-4fb1-9d7a-ea55421235d9
📒 Files selected for processing (13)
internal/qualitygate/config/README.mdinternal/qualitygate/config/allowlists/fixture-domains.txtinternal/qualitygate/config/allowlists/public-domains.txtlint/README.mdlint/domaincontract/diff.golint/domaincontract/diff_test.golint/domaincontract/policy.golint/domaincontract/policy_test.golint/domaincontract/scan.golint/domaincontract/unapproved.golint/domaincontract/unapproved_repo_test.golint/domaincontract/unapproved_test.golint/main.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/qualitygate/config/README.md
- lint/README.md
0419c26 to
b1fcb46
Compare
github-actions
Bot
added
size/XL
There was a problem hiding this comment.
🧹 Nitpick comments (7)
lint/domaincontract/diff.go (1)
157-171: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueGit invocations have no timeout or
contextsupport. In CI a hunggit(e.g. credential prompt on a bad revision) blocks the lint run indefinitely.exec.CommandContextwith a bounded context would fail fast.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lint/domaincontract/diff.go` around lines 157 - 171, Update gitCommandOutput to create a bounded context and invoke git with exec.CommandContext instead of exec.Command. Ensure the context timeout cancels hung git processes while preserving the existing output and stderr-wrapping behavior.lint/domaincontract/scan.go (1)
86-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd doc comments to the new exported symbols.
ScanOptionsandScanRepoWithOptionsare exported without comments;ChangedFromsemantics (base revision, empty = full inventory) are worth stating since callers inlint/main.godepend on it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lint/domaincontract/scan.go` around lines 86 - 90, Add Go doc comments for the exported ScanOptions type, its ChangedFrom field, and ScanRepoWithOptions. Document that ChangedFrom identifies the base revision and an empty value requests a full inventory, matching the behavior relied on by lint/main.go.lint/domaincontract/policy.go (2)
58-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCross-list duplicate reporting is order-dependent. Iterating
fixtures(a map) makes the reported entry nondeterministic when several hostnames appear in both lists. Sorting the hosts before the check would make CI output stable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lint/domaincontract/policy.go` around lines 58 - 65, Update the cross-list duplicate check to iterate hostnames in deterministic sorted order rather than directly ranging over the fixtures map. Preserve the existing duplicate error construction and return behavior, using the sorted host keys before accessing fixtures and public.
106-126: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valuePolicy hostnames cannot express IDN hosts that the scanner can detect.
validatePolicyHostnameaccepts only[a-z0-9-]labels, whilenormalizeCandidateHostnameinlint/domaincontract/unapproved.go(Lines 711-729) accepts anyunicode.IsLetter/IsDigitlabel. So a Unicode hostname such as例子.公司.cncan be detected as evidence but can never be approved — the only remedy is removing the code. If that is intentional, consider documenting it; otherwise normalize both sides to punycode (or allow non-ASCII labels here).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lint/domaincontract/policy.go` around lines 106 - 126, Update validatePolicyHostname to accept the same IDN hostname forms recognized by normalizeCandidateHostname, while preserving hostname length, label length, non-empty label, hyphen-boundary, and final-label validation. Normalize accepted Unicode hostnames to punycode consistently with the scanner before comparison, or otherwise allow valid non-ASCII letter/digit labels so detected IDN hosts can be approved.lint/domaincontract/unapproved_repo_test.go (1)
15-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winIsolate global Git configuration in these fixtures.
git init/commithere inherit the developer's global config and templates (core.excludesFile,commit.template,core.hooksPath,init.templateDir, signing defaults), which can make these repo-level tests fail or behave differently locally. Pinning the environment makes them hermetic.♻️ Suggested isolation
func setupDomainDiffRepo(t *testing.T, target string) (root, base string) { t.Helper() + t.Setenv("GIT_CONFIG_GLOBAL", os.DevNull) + t.Setenv("GIT_CONFIG_SYSTEM", os.DevNull) + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") root = t.TempDir()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lint/domaincontract/unapproved_repo_test.go` around lines 15 - 42, Isolate Git configuration in setupDomainDiffRepo and its gitTestCommand calls so fixture initialization and commits do not inherit global excludes, templates, hooks, init settings, or signing defaults. Reuse a per-fixture environment or explicit Git configuration for all commands, while preserving the existing repository setup and commit behavior.lint/domaincontract/unapproved_test.go (1)
15-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated sort/scan body.
scanDomainEvidenceandscanTypedDomainEvidenceInPackagediffer only in whetherInfois populated; extracting a sharedcollectSorted(file, fset, info)helper would remove the duplicated comparator.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lint/domaincontract/unapproved_test.go` around lines 15 - 65, Extract the shared scan, evidence collection, and sorting logic from scanDomainEvidence and scanTypedDomainEvidenceInPackage into a collectSorted helper accepting the parsed file, file set, and optional types.Info. Update both callers to perform only their distinct parsing/type-checking setup, then delegate to collectSorted while preserving the existing host and expression-position ordering.lint/main.go (1)
39-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider hoisting
ScanOptionsintolintapi. The scanner signature is keyed toerrscontract.ScanOptions, forcing an adapter per additional domain. A sharedlintapi.ScanOptionswould let both scanners be registered directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lint/main.go` around lines 39 - 51, Move the shared scan-options type from errscontract into lintapi, then update scanner.fn and all ScanRepoWithOptions signatures to accept lintapi.ScanOptions. Adjust errscontract and domaincontract implementations and callers accordingly, and register both scanners directly without the domaincontract adapter closure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lint/domaincontract/diff.go`:
- Around line 157-171: Update gitCommandOutput to create a bounded context and
invoke git with exec.CommandContext instead of exec.Command. Ensure the context
timeout cancels hung git processes while preserving the existing output and
stderr-wrapping behavior.
In `@lint/domaincontract/policy.go`:
- Around line 58-65: Update the cross-list duplicate check to iterate hostnames
in deterministic sorted order rather than directly ranging over the fixtures
map. Preserve the existing duplicate error construction and return behavior,
using the sorted host keys before accessing fixtures and public.
- Around line 106-126: Update validatePolicyHostname to accept the same IDN
hostname forms recognized by normalizeCandidateHostname, while preserving
hostname length, label length, non-empty label, hyphen-boundary, and final-label
validation. Normalize accepted Unicode hostnames to punycode consistently with
the scanner before comparison, or otherwise allow valid non-ASCII letter/digit
labels so detected IDN hosts can be approved.
In `@lint/domaincontract/scan.go`:
- Around line 86-90: Add Go doc comments for the exported ScanOptions type, its
ChangedFrom field, and ScanRepoWithOptions. Document that ChangedFrom identifies
the base revision and an empty value requests a full inventory, matching the
behavior relied on by lint/main.go.
In `@lint/domaincontract/unapproved_repo_test.go`:
- Around line 15-42: Isolate Git configuration in setupDomainDiffRepo and its
gitTestCommand calls so fixture initialization and commits do not inherit global
excludes, templates, hooks, init settings, or signing defaults. Reuse a
per-fixture environment or explicit Git configuration for all commands, while
preserving the existing repository setup and commit behavior.
In `@lint/domaincontract/unapproved_test.go`:
- Around line 15-65: Extract the shared scan, evidence collection, and sorting
logic from scanDomainEvidence and scanTypedDomainEvidenceInPackage into a
collectSorted helper accepting the parsed file, file set, and optional
types.Info. Update both callers to perform only their distinct
parsing/type-checking setup, then delegate to collectSorted while preserving the
existing host and expression-position ordering.
In `@lint/main.go`:
- Around line 39-51: Move the shared scan-options type from errscontract into
lintapi, then update scanner.fn and all ScanRepoWithOptions signatures to accept
lintapi.ScanOptions. Adjust errscontract and domaincontract implementations and
callers accordingly, and register both scanners directly without the
domaincontract adapter closure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 45e92b0a-0ac3-4c0b-805e-6188c8bd86b8
📒 Files selected for processing (13)
internal/qualitygate/config/README.mdinternal/qualitygate/config/allowlists/fixture-domains.txtinternal/qualitygate/config/allowlists/public-domains.txtlint/README.mdlint/domaincontract/diff.golint/domaincontract/diff_test.golint/domaincontract/policy.golint/domaincontract/policy_test.golint/domaincontract/scan.golint/domaincontract/unapproved.golint/domaincontract/unapproved_repo_test.golint/domaincontract/unapproved_test.golint/main.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/qualitygate/config/README.md
- lint/README.md
2 participants
Summary
Add a CI-enforced domain allowlist for Go source so new hostnames must be approved as public endpoints or scoped as test fixtures.
The check targets newly added code and uses hostname-aware contexts to avoid blocking unrelated dotted strings or historical code.
Changes
Host/Domainassignments and absolute URLs.lintcheck.Test Plan
Related Issues
Summary by CodeRabbit
New Features
Documentation
--changed-frombehavior.Tests