fix(review): preserve safe enrichment lines when sanitizer finds forbidden terms#3753
Merged
Merged
Conversation
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3753 +/- ##
=======================================
Coverage 93.37% 93.38%
=======================================
Files 316 316
Lines 32296 32303 +7
Branches 11841 11842 +1
=======================================
+ Hits 30158 30165 +7
Misses 1507 1507
Partials 631 631
🚀 New features to boost your workflow:
|
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
sanitizePublicCommentto throw and the engine to drop the entire enrichment prompt, allowing a malicious commit subject to suppress unrelated security findings.Description
sanitizeEnrichmentPromptSectioninsrc/review/enrichment-wire.tsto first attempt sanitizing the whole defanged brief and, if that throws, fall back to per-line sanitization that keeps only lines which individually passsanitizePublicCommentand then truncates toMAX_ENRICHMENT_PROMPT_SECTION_CHARS.neutralizePromptInjectiondefanging step and ensure the function returnsundefinedwhen no safe lines remain.test/unit/enrichment-wire.test.tsthat simulates a brief containing an unsafe commit-lint line plus an independent dependency advisory and asserts the advisory is preserved while the unsafe line is dropped.Testing
npx vitest run test/unit/enrichment-wire.test.ts, and the new test (and the file's suite) passed.npm run typecheck, which completed with no type errors.git diff --check, which reported no whitespace/conflict issues.npm run test:coverage(full coverage gate) in this environment but the global coverage threshold run did not succeed (the repository-wide coverage thresholds were not met when running the local coverage job here).npm audit --audit-level=moderatebut the npm registry audit endpoint returned403 Forbiddenin this environment, so the dependency-review step could not be completed here.Codex Task