Repository navigation
fix: recognize Slack webhook hostname case - #1232
Conversation
Normalize generated Slack webhook hostnames to paired case so lower-, upper-, and mixed-case hosts are redacted. Preserve lowercase path prefixes, token suffix boundaries, literal dots, lookalike rejection, and the empty flags value. Regenerate from the pinned Gitleaks source and cover the generator and compiled-rule contracts. Focused tests, typecheck, touched-file ESLint, diff checks, and mutation evidence pass. No dependencies or Codecov classification changed. Signed-off-by: Bernardo Donadio <[email protected]>
Bring the Slack hostname fix onto the current protected-branch head before exact-candidate review. The intervening merges address Bugs 1174 and 1140 in disjoint runtime paths. Retaining them here avoids reviewing a stale publication candidate. Signed-off-by: Bernardo Donadio <[email protected]>
Build Slack path-negative fixtures on the valid mixed-case hostname so the tests isolate path casing from hostname casing. Separate escaped Sidekiq identity from already-paired Slack idempotence to make each generator contract explicit. Keep this correction test-only and issue-scoped. Signed-off-by: Bernardo Donadio <[email protected]>
Bring the adjudicated Slack hostname test corrections onto the current protected-branch head before the second exact-candidate review. The intervening Bugs 1132 and 1155 merges are disjoint from the Bug 1139 files, so this integration preserves the reviewed issue behavior. Signed-off-by: Bernardo Donadio <[email protected]>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![greptile-apps[bot]](/web-api/iframe-proxy?url=https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Changes recommended
Repo policy requires a fresh exact-head pnpm run test:ci pass (including 100% coverage gates), which is not yet confirmed in the PR metadata.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes Slack webhook redaction so it recognizes hooks.slack.com hostnames in any ASCII case (lower/upper/mixed) without changing existing rule flags or making path/token portions case-insensitive, aligning behavior with DNS hostname case-insensitivity and closing #1139.
Changes:
- Extend the generator’s hostname-literal normalization to emit explicit paired-case hostname patterns for the Slack webhook rule (while keeping existing flag semantics).
- Update scrub and generator tests to assert mixed-case hostname acceptance and uppercase/mixed-case path-prefix rejection.
- Document the hostname-scoped rule behavior and include a patch changeset.
File summaries
| File | Description |
|---|---|
| test/scrub.test.ts | Expands Slack webhook tests to cover host casing and adds a negative test for non-lowercase path prefixes. |
| test/scripts/update-gitleaks-patterns.test.ts | Adds normalization/idempotence coverage for raw, escaped, and already paired Slack hostname spellings. |
| src/generated-patterns.ts | Regenerates the Slack webhook rule regex to use explicit paired-case hostname matching. |
| scripts/update-gitleaks-patterns.ts | Introduces paired-case hostname normalization for selected hostname-scoped rules (Slack webhook). |
| docs/privacy.md | Updates redaction documentation to describe hostname case-insensitive matching and preserved path/token semantics. |
| .changeset/1139-slack-hostname-case.md | Adds a patch changeset for the redaction behavior fix. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Summary
Slack webhook redaction now recognizes uppercase and mixed-case
hooks.slack.comhostnames. Only the hostname uses explicit ASCII case pairs; regex flags, lowercase path prefixes, and token syntax retain their existing semantics.Motivation / Why
The generated JavaScript rule treated the hostname as case-sensitive and missed valid webhook URLs. Normalizing raw and already dot-escaped upstream hostname spellings also preserves weekly regeneration and rejects hostname lookalikes.
Type of change
Release-note title
fix: recognize Slack webhook hostname case
Testing done
Focused generation and scrub suites: 80/80 passed. Type checking, touched-file ESLint, diff checks, pinned checksum-verified Gitleaks generation, and original-symptom verification passed. Lowercase, uppercase, and mixed-case hosts are accepted while uppercase/mixed-case paths and hostname lookalikes retain their rejection behavior.
Independent GLM Max and Grok medium reviews, followed by Opus medium synthesis, were clean at signed/DCO candidate
a20c7257e979f6380d0f481a9cd372df9ad2e1ae. Full exact-head CI remains required.Related issues
Closes #1139
Checklist
pnpm run test:ciwith 100% line, branch, function, and statement coverageReview guidance
Start with the rule-scoped generator normalization and its raw, escaped, and idempotence tests, then inspect the generated Slack rule and compiled path/lookalike boundary tests.
Greptile Summary
The PR makes Slack webhook hostname matching ASCII case-insensitive while preserving literal dots, lowercase path prefixes, and existing token syntax.
Confidence Score: 4/5
The behavioral fix appears safe to merge, with only the non-blocking repository-required Codecov taxonomy update outstanding.
The generated regex precisely expands hostname case handling without broadening dots, paths, or token syntax, and focused tests cover the relevant boundaries; the remaining issue is the omitted coverage-classification maintenance required for production TypeScript changes.
Files Needing Attention: src/generated-patterns.ts, codecov.yml, test/codecov-config.test.ts
Important Files Changed
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Merge current main before round 2" | Re-trigger Greptile
Context used: