refactor: per-workspace script cleanup and CI path filtering#316
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…providers Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughSplit the monolithic CI into three targeted workflows (root, data-sanitization, data-sanitization-log-providers), removed the old ChangesCI Workflow Refactoring and Script Consolidation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Coverage Report for packages/data-sanitization
File CoverageNo changed files found. |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/workflows/ci-data-sanitization-log-providers.yml:
- Around line 7-23: The workflow's push and pull_request path filters under the
pull_request job omit shared root lint config files (e.g., .markdownlint.json),
so update the paths arrays in the workflow (the pull_request.paths and the
push.paths entries shown) to include the workspace root lint config filenames
(for example add '.markdownlint.json' and any other shared root config files) so
config-only changes trigger the CI; ensure both the push and pull_request paths
sections are updated consistently.
In @.github/workflows/ci-data-sanitization.yml:
- Around line 6-21: The workflow's push and pull_request path filters (the two
paths arrays in .github/workflows/ci-data-sanitization.yml) omit shared root
lint/config files so config-only changes can skip CI; update both 'paths' lists
to include the workspace's shared lint/config files (for example add entries for
.markdownlint.json and any other root lint configs used by the workspace) so
changes to those files trigger the data-sanitization workflow.
In @.github/workflows/ci-root.yml:
- Around line 38-40: Remove the over-privileged pull-requests: write entry from
the permissions block so the workflow only requests read-only token scope;
locate the permissions YAML block (the permissions: contents: read section) and
delete the pull-requests: write line (or replace it with no permission) to
ensure the job does not request PR write rights.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: bd24c0f2-07a4-4b52-85b4-40c09f4ed597
📒 Files selected for processing (8)
.github/workflows/ci-data-sanitization-log-providers.yml.github/workflows/ci-data-sanitization.yml.github/workflows/ci-root.yml.github/workflows/ci.ymldocs/superpowers/plans/2026-05-26-ci-and-script-cleanup.mdpackage.jsonpackages/data-sanitization-log-providers/package.jsonpackages/data-sanitization/package.json
💤 Files with no reviewable changes (1)
- .github/workflows/ci.yml
Coverage Report for packages/data-sanitization-log-providers
File CoverageNo changed files found. |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a comprehensive adversarial test suite to harden coverage of the regex-based string sanitization path and expose documented limitations. replacers.test.ts — new 'adversarial string inputs' describe block: - boolean, null, array, object values: not masked on regex path (known limitation), masked correctly with parseJsonStrings: true - empty-string value: regex misbehaves (over-consumption); parseJsonStrings path handles it correctly - escaped-quote values: regex path may garble output; parseJsonStrings path handles correctly; both paths never throw - truncated / brace-less JSON fragments: no exception - very long values (10 KB): masked successfully - performance: two timing guards against catastrophic backtracking - double-encoded JSON: escapedJsonMatcher catches inner fields on regex path; parseJsonStrings + regex fallback also suppresses the inner secret - idempotency: sanitizing twice produces the same result - form-encoded edge cases: empty value, base64 padding, multiple occurrences, URL-encoded ampersand, semicolon-delimited strings - multiline strings: sensitive field on one line; surrounding lines preserved - unicode escape values: masked correctly - value-contains-key-name strings: non-sensitive keys not masked - mixed-type arrays: string masked, number not masked on regex path; both masked with parseJsonStrings: true matchers.test.ts — adversarial additions per matcher: - formEncodedMatcher: empty value, URL-encoded ampersand, base64 padding, semicolon-separated strings, very long values, tab in value - jsonMatcher: empty-string over-consumption quirk, numeric/boolean/null/array values not matched, unicode escapes, escaped-quote partial-match, whitespace- before-colon limitation, very long values, digit-suffix key names, value-only pattern not matched - escapedJsonMatcher: empty-value delimiter-bleed quirk, double-escaped backslash, unicode sequences, very long values, multiple fields, value-only pattern not matched Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
# Overview Adds a comprehensive adversarial test suite to harden coverage of the regex-based string sanitization path. The tests document known limitations precisely — tests that expose a limitation assert the *current* behavior so any future regression (in either direction) is caught immediately. ## Details **`replacers.test.ts` — new `adversarial string inputs` describe block inside `stringReplacer`:** - Boolean, null, array, and object field values: not masked on the regex path (known limitation); correctly masked with `parseJsonStrings: true` - Empty-string field values: regex over-consumes neighboring delimiter characters ( bleeds), producing an incorrect replacement; path handles correctly - Escaped-quote values (`sec"ret`): regex stops at the first unescaped `"`, may corrupt surrounding JSON; never throws; `parseJsonStrings` path handles correctly - Truncated and brace-less JSON fragments: no exception on either path - Very long values (10 KB): masked successfully - Two performance guards against catastrophic backtracking (2-second limit) - Double-encoded JSON: `escapedJsonMatcher` catches inner fields on the regex path; `parseJsonStrings` path also suppresses the inner secret - Idempotency: sanitizing twice produces the same result - Form-encoded edge cases: empty value, base64 padding (`==`), multiple occurrences, URL-encoded ampersand (`%26`), semicolon-delimited strings, single key with no delimiter - Multiline strings: sensitive field on one line, surrounding lines preserved - Mixed-type arrays: string value masked, number value not masked on regex path; both masked with `parseJsonStrings: true` **`matchers.test.ts` — adversarial additions per matcher:** - `formEncodedMatcher`: empty value, URL-encoded ampersand, base64 padding, semicolon-separated strings (not treated as delimiters), very long values, tab in value - `jsonMatcher`: empty-string over-consumption quirk, numeric/boolean/null/array values not matched (known limitation), unicode escapes matched correctly, escaped-quote partial-match limitation, whitespace-before-colon limitation, very long values, digit-suffix key names, value-only pattern not matched - `escapedJsonMatcher`: empty-value delimiter-bleed quirk, double-escaped backslash, unicode sequences, very long values, multiple fields in one string, value-only pattern not matched ## Related Tickets and/or Pull Requests Relates to #316 ## Checklist - [x] Tests added or updated - [ ] README and TSDoc updated if the public API changed - [ ] Breaking changes called out (if any) - [ ] Roadmap item checked off if this PR completes one 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Broader, more robust sanitization matching (better handling of hyphenated field names and escape-aware quoted JSON) for more reliable masking. * **Tests** * Expanded and reorganized test coverage across many edge cases: empty/very long values, escaped/unicode sequences, delimiter/format tolerance, tabs/whitespace, malformed/truncated inputs, nested/double-encoded JSON, collections, idempotency, performance bounds, and multi-occurrence masking. * Clarified numerous test descriptions for logging and error-wrapping behaviors. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/ioncache/data-sanitization/pull/319?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a comprehensive adversarial test suite to harden coverage of the regex-based string sanitization path. The tests document known limitations precisely — tests that expose a limitation assert the *current* behavior so any future regression (in either direction) is caught immediately. **`replacers.test.ts` — new `adversarial string inputs` describe block inside `stringReplacer`:** - Boolean, null, array, and object field values: not masked on the regex path (known limitation); correctly masked with `parseJsonStrings: true` - Empty-string field values: regex over-consumes neighboring delimiter characters ( bleeds), producing an incorrect replacement; path handles correctly - Escaped-quote values (`sec"ret`): regex stops at the first unescaped `"`, may corrupt surrounding JSON; never throws; `parseJsonStrings` path handles correctly - Truncated and brace-less JSON fragments: no exception on either path - Very long values (10 KB): masked successfully - Two performance guards against catastrophic backtracking (2-second limit) - Double-encoded JSON: `escapedJsonMatcher` catches inner fields on the regex path; `parseJsonStrings` path also suppresses the inner secret - Idempotency: sanitizing twice produces the same result - Form-encoded edge cases: empty value, base64 padding (`==`), multiple occurrences, URL-encoded ampersand (`%26`), semicolon-delimited strings, single key with no delimiter - Multiline strings: sensitive field on one line, surrounding lines preserved - Mixed-type arrays: string value masked, number value not masked on regex path; both masked with `parseJsonStrings: true` **`matchers.test.ts` — adversarial additions per matcher:** - `formEncodedMatcher`: empty value, URL-encoded ampersand, base64 padding, semicolon-separated strings (not treated as delimiters), very long values, tab in value - `jsonMatcher`: empty-string over-consumption quirk, numeric/boolean/null/array values not matched (known limitation), unicode escapes matched correctly, escaped-quote partial-match limitation, whitespace-before-colon limitation, very long values, digit-suffix key names, value-only pattern not matched - `escapedJsonMatcher`: empty-value delimiter-bleed quirk, double-escaped backslash, unicode sequences, very long values, multiple fields in one string, value-only pattern not matched Relates to #316 - [x] Tests added or updated - [ ] README and TSDoc updated if the public API changed - [ ] Breaking changes called out (if any) - [ ] Roadmap item checked off if this PR completes one 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> * **Improvements** * Broader, more robust sanitization matching (better handling of hyphenated field names and escape-aware quoted JSON) for more reliable masking. * **Tests** * Expanded and reorganized test coverage across many edge cases: empty/very long values, escaped/unicode sequences, delimiter/format tolerance, tabs/whitespace, malformed/truncated inputs, nested/double-encoded JSON, collections, idempotency, performance bounds, and multi-occurrence masking. * Clarified numerous test descriptions for logging and error-wrapping behaviors. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/ioncache/data-sanitization/pull/319?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
:allsuffix (lint:all,format:all,build:all, etc.) so root-scoped work scripts (lint,format,format:check) can use the plain names without recursion:code/:srcscript aliases from both packages — each workspace now has one canonical script per actionci.ymlinto three per-workspace workflows with path filters:ci-root.yml,ci-data-sanitization.yml,ci-data-sanitization-log-providers.ymldata-sanitization-log-providersalso triggers onpackages/data-sanitization/**since it has that as a workspace peer depTest plan
ci-root.ymltriggers on this PR (root files changed:package.json,.github/workflows/**)ci-data-sanitization.ymltriggers (rootpackage.jsonin its path filter)ci-data-sanitization-log-providers.ymltriggers (rootpackage.jsonin its path filter)yarn lint:all,yarn format:check:all,yarn build:all,yarn test:allall pass locally (verified)🤖 Generated with Claude Code
Summary by CodeRabbit
Chores
Documentation