refactor: clear the SonarCloud new-code findings on main - #703
Merged
Conversation
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
devops-thiago
added a commit
that referenced
this pull request
Aug 13, 2026
…efeaters Rebased onto the #703 split shapes. DEFENSE_OBJECT_AFTER_VERB gains the parameteriz stem the verb group already had and accepts clause-break punctuation (semicolon, colon, dashes) as a separator, so "Nothing escapes parameterization" and "Nothing escapes; the sanitizer runs on render" read as the mitigations they assert instead of over-firing the floor; the comma is deliberately excluded so comma-coordinated absence verbs and appositive denials stay absence claims, and the denied defense across a clause break is documented as accepted residual under-fire. A new ABSENCE_MODAL_VERB token registers modal absence claims ("Nothing can sanitize the value", one non-negator adverb admitted), MITIGATION_DO_SUPPORTED admits one non-negator gap word so "does always escape" defeats the floor while "does not escape" stays an absence claim, and MITIGATION_ASSERTED_SUBJECT excludes coordinators from its subject slot so "Nothing escapes, sanitizes, or validates the value" does not read its own continuation as a mitigation. Closes #696
8 tasks
devops-thiago
added a commit
that referenced
this pull request
Aug 13, 2026
…efeaters Rebased onto the #703 split shapes. DEFENSE_OBJECT_AFTER_VERB gains the parameteriz stem the verb group already had and accepts clause-break punctuation (semicolon, colon, dashes) as a separator, so "Nothing escapes parameterization" and "Nothing escapes; the sanitizer runs on render" read as the mitigations they assert instead of over-firing the floor; the comma is deliberately excluded so comma-coordinated absence verbs and appositive denials stay absence claims, and the denied defense across a clause break is documented as accepted residual under-fire. A new ABSENCE_MODAL_VERB token registers modal absence claims ("Nothing can sanitize the value", one non-negator adverb admitted), MITIGATION_DO_SUPPORTED admits one non-negator gap word so "does always escape" defeats the floor while "does not escape" stays an absence claim, and MITIGATION_ASSERTED_SUBJECT excludes coordinators from its subject slot so "Nothing escapes, sanitizes, or validates the value" does not read its own continuation as a mitigation. Closes #696
Contributor
🤖 ThrillhouseBot PR SummaryWhat this PR doesClears SonarCloud new-code findings via behavior-neutral refactors: three over-budget regexes are split into smaller patterns whose single-pattern find()/replaceAll semantics are re-implemented with explicit leftmost-match walks, private methods move into their only-caller nested classes with loggers pinned to the outer classes, and several small smell fixes land (assertEquals instead of assertTrue(a==b), the '/' BINARY_PACKAGE_SEPARATOR constant, the isKeepablePattern predicate, extracted mock variables). Description vs. ImplementationNo mismatch found between the PR description and the change. Control-Flow Diagram🔀 Show diagramflowchart TD
A["assertsMitigation(asserted)"] --> B{"hasUnnegatedAssertedMatch: leftmost across BE, GET, SUBJECT"}
B -->|"no match"| C["fall back to MITIGATION_DO_SUPPORTED"]
B -->|"match not preceded by nothing or nobody"| D["defeater fires: mitigation asserted"]
B -->|"match preceded by nothing or nobody"| B
E["claimsNothingNeutralizesIt(text)"] --> F["claimsAbsenceAsSubject: iterate verb tokens"]
F --> G{"prefix is nothing or nobody plus up to 3 gap words, and no defense stem within 2 words after verb"}
G -->|"yes"| H["absence claim fires"]
G -->|"no"| F
I["GitHubApiError.clean"] --> J["redactCredentials: leftmost match across prefix and value patterns"]
J -->|"match"| K["mask match and resume from its end"] --> J
J -->|"no match"| L["emit remaining text"]
Changes Overview
Changed Files
Risk Assessment
Key Findings
|
| Check | Type | Status | Detail |
|---|---|---|---|
| frontend | check-run | ⏳ Pending | - |
| format | check-run | ⏳ Pending | - |
| test | check-run | ⏳ Pending | - |
| trivy | check-run | ⏳ Pending | - |
| dependency-review | check-run | ⏳ Pending | - |
Automated review by ThrillhouseBot. Reply with /review to re-run.
![thrillhousebot[bot]](https://avatars.githubusercontent.com/in/3996985?s=60&v=4){kind=small}
thrillhousebot
Bot
added
java
Contributor
There was a problem hiding this comment.
ThrillhouseBot found no issues in this PR, but some checks are still pending or failed:
- Check trivy is pending
- Check test is pending
- Check format is pending
- Check frontend is pending
- Check dependency-review is pending
Additionally, No new issues in this revision, but 1 previous finding(s) remain unresolved — fix them, or reply on their review thread with why they are deferred. A finding listed only under "Things to double-check" has no thread: clear it by commenting @thrillhousebot resolved path/to/File.java:42 — <the finding's title> on this PR.
devops-thiago
added a commit
that referenced
this pull request
Aug 13, 2026
…efeaters Rebased onto the #703 split shapes. DEFENSE_OBJECT_AFTER_VERB gains the parameteriz stem the verb group already had and accepts clause-break punctuation (semicolon, colon, dashes) as a separator, so "Nothing escapes parameterization" and "Nothing escapes; the sanitizer runs on render" read as the mitigations they assert instead of over-firing the floor; the comma is deliberately excluded so comma-coordinated absence verbs and appositive denials stay absence claims, and the denied defense across a clause break is documented as accepted residual under-fire. A new ABSENCE_MODAL_VERB token registers modal absence claims ("Nothing can sanitize the value", one non-negator adverb admitted), MITIGATION_DO_SUPPORTED admits one non-negator gap word so "does always escape" defeats the floor while "does not escape" stays an absence claim, and MITIGATION_ASSERTED_SUBJECT excludes coordinators from its subject slot so "Nothing escapes, sanitizes, or validates the value" does not read its own continuation as a mitigation. Closes #696
Contributor
🤖 ThrillhouseBot — changes since the last review
|
devops-thiago
added a commit
that referenced
this pull request
Aug 13, 2026
…efeaters Rebased onto the #703 split shapes. DEFENSE_OBJECT_AFTER_VERB gains the parameteriz stem the verb group already had and accepts clause-break punctuation (semicolon, colon, dashes) as a separator, so "Nothing escapes parameterization" and "Nothing escapes; the sanitizer runs on render" read as the mitigations they assert instead of over-firing the floor; the comma is deliberately excluded so comma-coordinated absence verbs and appositive denials stay absence claims, and the denied defense across a clause break is documented as accepted residual under-fire. A new ABSENCE_MODAL_VERB token registers modal absence claims ("Nothing can sanitize the value", one non-negator adverb admitted), MITIGATION_DO_SUPPORTED admits one non-negator gap word so "does always escape" defeats the floor while "does not escape" stays an absence claim, and MITIGATION_ASSERTED_SUBJECT excludes coordinators from its subject slot so "Nothing escapes, sanitizes, or validates the value" does not read its own continuation as a mitigation. Closes #696
…es and region-bound negation checks
Contributor
🤖 ThrillhouseBot — changes since the last review
|
…tead of rescanning the tail
Contributor
There was a problem hiding this comment.
ThrillhouseBot found no issues in this PR, but some checks are still pending or failed:
- Check format is pending
- Check test is pending
- Check frontend is pending
- Check trivy is pending
- Check dependency-review is pending
Additionally, No new issues in this revision, but 1 previous finding(s) remain unresolved — fix them, or reply on their review thread with why they are deferred. A finding listed only under "Things to double-check" has no thread: clear it by commenting @thrillhousebot resolved path/to/File.java:42 — <the finding's title> on this PR.
|
Contributor
🤖 ThrillhouseBot — changes since the last review
|
devops-thiago
added a commit
that referenced
this pull request
Aug 13, 2026
…efeaters Rebased onto the #703 split shapes. DEFENSE_OBJECT_AFTER_VERB gains the parameteriz stem the verb group already had and accepts clause-break punctuation (semicolon, colon, dashes) as a separator, so "Nothing escapes parameterization" and "Nothing escapes; the sanitizer runs on render" read as the mitigations they assert instead of over-firing the floor; the comma is deliberately excluded so comma-coordinated absence verbs and appositive denials stay absence claims, and the denied defense across a clause break is documented as accepted residual under-fire. A new ABSENCE_MODAL_VERB token registers modal absence claims ("Nothing can sanitize the value", one non-negator adverb admitted), MITIGATION_DO_SUPPORTED admits one non-negator gap word so "does always escape" defeats the floor while "does not escape" stays an absence claim, and MITIGATION_ASSERTED_SUBJECT excludes coordinators from its subject slot so "Nothing escapes, sanitizes, or validates the value" does not read its own continuation as a mitigation. Closes #696
devops-thiago
added a commit
that referenced
this pull request
Aug 13, 2026
…efeaters Rebased onto the #703 split shapes. DEFENSE_OBJECT_AFTER_VERB gains the parameteriz stem the verb group already had and accepts clause-break punctuation (semicolon, colon, dashes) as a separator, so "Nothing escapes parameterization" and "Nothing escapes; the sanitizer runs on render" read as the mitigations they assert instead of over-firing the floor; the comma is deliberately excluded so comma-coordinated absence verbs and appositive denials stay absence claims, and the denied defense across a clause break is documented as accepted residual under-fire. A new ABSENCE_MODAL_VERB token registers modal absence claims ("Nothing can sanitize the value", one non-negator adverb admitted), MITIGATION_DO_SUPPORTED admits one non-negator gap word so "does always escape" defeats the floor while "does not escape" stays an absence claim, and MITIGATION_ASSERTED_SUBJECT excludes coordinators from its subject slot so "Nothing escapes, sanitizes, or validates the value" does not read its own continuation as a mitigation. Closes #696
devops-thiago
added a commit
that referenced
this pull request
Aug 14, 2026
…efeaters (#701) ## What type of PR is this? - [x] 🐛 Bug fix ## Description Closes the four residual gaps in the injection-sink floor's defeater patterns in `FindingVerificationService` that survived the #676/#685 hardening: 1. **`parameteriz` added to `DEFENSE_OBJECT_AFTER_VERB`'s defense stems** — "Nothing escapes parameterization before the query runs" is the mitigation it asserts and no longer over-fires the floor. 2. **Modal absence claims register** — a new `ABSENCE_MODAL_VERB` token reads "Nothing can sanitize the value" as the absence claim it is (one non-negator adverb admitted, sharing the finite wording's subject prefix), with the same defense-noun-object rejection so "nothing can escape validation" stays a mitigation. 3. **`MITIGATION_DO_SUPPORTED` admits one non-negator gap word** — "the framework does always escape the value" now defeats the floor, while "does not escape" stays an absence claim via the negator lookahead. 4. **Punctuation separators, decided deliberately: widened for clause breaks only** — `DEFENSE_OBJECT_AFTER_VERB`'s separator accepts clause-break punctuation (`;` `:` and dashes), so "Nothing escapes; the sanitizer runs on render" reads as mitigated. The comma and sentence-ending punctuation are deliberately excluded: comma-coordinated absence verbs ("Nothing escapes, sanitizes, or validates the value") and appositive denials ("Nothing escapes, but the sanitizer is disabled") stay absence claims, and a defense noun in the NEXT sentence cannot defuse this sentence's claim — all tested. The denied defense across a clause break is documented in the javadoc as accepted residual under-fire. As part of the coordination case, `MITIGATION_ASSERTED_SUBJECT` excludes coordinators from its subject slot so "or validates" is not read as a subject-verb mitigation. Over-fire was the dangerous direction; items 1, 3, 4 stop flooring mitigated findings, and item 2's under-fire fix carries its own over-fire guard. ## Related Issues Closes #696 ## How Has This Been Tested? - [x] Unit tests Eleven new tests in `FindingVerificationServiceTest` cover every acceptance criterion plus the over-fire guards (modal defense-noun object, negated do-support, next-sentence defense noun). Full `./mvnw verify` clean locally. ## Checklist - [x] My code follows the project's coding standards - [x] I have performed a self-review of my own code - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] I have updated the documentation accordingly - [x] My changes generate no new warnings or errors ## Screenshots / Logs N/A ## Additional Notes Each pattern's javadoc is updated to stay accurate, and the CHANGELOG gains an `[Unreleased]` Fixed entry. **Depends on #703**: this branch is rebased onto `refactor/sonar-new-code-smells` and expresses the semantic changes in that PR's split pattern shapes (which is also what keeps each pattern under the S5843 complexity budget). Kept as draft until #703 merges; it will then be rebased onto main.
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
1 participant
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.
What type of PR is this?
Description
Clears all 14 SonarCloud new-code findings on
main— every one fixed at the source, no suppressions, no rule exclusions, and behavior-neutral throughout (every fix below was verified against the existing test suite, and the regex splits were additionally fuzz-checked for exact input/output equivalence against the original patterns).java:S5843 / java:S6353 — over-budget regexes (FindingVerificationService ×2 + S6353, GitHubApiError ×1)
MITIGATION_ASSERTED(complexity 50, and the{0,1}S6353) is split intoMITIGATION_ASSERTED_BE,MITIGATION_ASSERTED_GETandMITIGATION_ASSERTED_SUBJECT, following the file's established split-into-a-union pattern.assertsMitigationwalks the three as ONE alternation (leftmost match wins, position tie in declared order, scan resumes after the match) so the defeater reads exactly the matches the single pattern read — a plain per-pattern pass would also surface matches the one-alternation scan stepped over.MITIGATION_ABSENT_SUBJECT(complexity 30) becomes three anchored patterns (ABSENCE_SUBJECT_VERB,NEGATING_SUBJECT_BEFORE_VERB,DEFENSE_OBJECT_AFTER_VERB) joined inclaimsAbsenceAsSubject, which checks every verb token against the verbatim prefix and trailing-lookahead bodies of the one-pattern form — deciding exactly the parses the original decided through backtracking, including the fix(review): floor unmitigated-injection-sink findings at high risk #594/fix(review): close three under-fire gaps in the injection-sink floor's defeaters #676/fix(review): close three under-fire gaps in the injection-sink floor's defeaters #685-pinned phrasings ("Nothing sanitizes…", "Nothing escapes validation", do-support, protasis coordinators). All existing pattern-pinning tests pass unchanged.CREDENTIAL_SHAPED(complexity 24) splits intoCREDENTIAL_SHAPED_PREFIXandCREDENTIAL_SHAPED_VALUE, andredactCredentialsre-implements the exact one-passreplaceAllsemantics as a leftmost-match merge across both patterns (position tie → prefix shapes, i.e. the original alternative order), preserving the overlap behavior the javadoc pins. Two new tests pin the two documented overlap cases.java:S5785 — BlockingStrictnessTest:64/89:
assertTrue(a == b, …)→assertEquals(expected, actual, …).java:S3398 — move private methods into their only-caller inner classes
FindingPipeline#heuristicFailureModesFor→BatchPrompts,ReviewDiffFormatter#compileGlobMatchers→IgnoreGlobs. Both methods emit operator-facing WARNs whose category the build-timeLogfacade would have silently relabeled to the nested class; each class now carries an explicitorg.jboss.logging.Loggerpinned to the outer class, so the category, level and rendered message are unchanged.HeuristicCodeDetector#isTestPath→FileScope(no logging involved).java:S1075 — JacocoCoverageReport:202: the
"/"literal becomes the documented constantBINARY_PACKAGE_SEPARATOR(it is the class-file binary-name separator, deliberately notFile.separator).java:S135 — RepoSettingsParser:268: the sanitize loop's two
continues are extracted into theisKeepablePatternpredicate (which keeps the over-long warn), leaving a singlebreak— warn ordering and outcomes are byte-identical.java:S9016 — AuthResourceTestFixtures:67-69: the three inline
mock(…)calls insidethenReturn(…)are extracted to local variables.Related Issues
N/A
How Has This Been Tested?
./mvnw verifygreen locally (Spotless, SpotBugs, full test suite, JaCoCo). The three regex splits were additionally fuzz-tested (500k randomized inputs per pattern family) against the original patterns with zero divergences, and the redaction overlap cases documented in the javadoc are now pinned by tests.Checklist
Screenshots / Logs
N/A
Additional Notes
No changelog entry: internal smell cleanup with no user-visible change, following #631–#634 precedent.