Skip to content

Fix public scenario reward/ranking sanitization - #913

Merged
JSONbored merged 1 commit into
mainfrom
codex/fix-public-scenario-serializer-vulnerabilities
Jun 19, 2026
Merged

Fix public scenario reward/ranking sanitization#913
JSONbored merged 1 commit into
mainfrom
codex/fix-public-scenario-serializer-vulnerabilities

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • The public scenario serializer could emit sensitive reward/ranking phrasing because the sanitizer and defensive forbidden-language regex omitted variants like “estimated rewards”, “rewards”, “ranking”, and “rankings”.

Description

  • Expand sanitizePublicComment patterns to redact generic reward and ranking language so entry label/detail are normalized to "private context" when matched. (changes in src/github/commands.ts)
  • Extend the module-level FORBIDDEN_PUBLIC_LANGUAGE defensive regex to include the newly-covered reward/ranking variants so any surviving terms cause the public-serialization guard to fail closed. (changes in src/scenarios/input-model.ts)
  • Add a regression test that exercises an estimate entry containing reward/ranking wording and extend the public-serialization sample list to include the new variants. (changes in test/unit/scenario-input-model.test.ts)

Testing

  • Ran the unit tests with npx vitest run test/unit/scenario-input-model.test.ts, and all tests passed (16 passed).
  • Ran formatting and checks (prettier --check and project checks) and fixed style issues so checks pass.

Codex Task

@ghost

ghost commented Jun 18, 2026

Copy link
Copy Markdown

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal Result Evidence Action
Gate result ⚠️ Skipped #913 is no longer open. No action.

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

@ghost ghost added the gittensory:reviewed label Jun 18, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@ghost

ghost commented Jun 18, 2026

Copy link
Copy Markdown

gittensory · advisory review

Reviewed 3 changed file(s) — two independent AI reviewers.

Changed files: src/github/commands.ts, src/scenarios/input-model.ts, test/unit/scenario-input-model.test.ts

Suggested action:Safe to merge — both reviewers found no blocking issues.

Reviewer A · gpt-oss-120b — recommends ✅ merge
The PR expands the public sanitization regexes to cover additional private context and private context terminology and updates the corresponding forbidden language regex in the scenario input model, adding tests to verify the new behavior.

Suggestions

  • Consider adding a comment explaining the purpose of the updated private context regex, as it now also sanitizes plain "private context" and "private context".
  • Run the full test suite to confirm no existing tests are affected by the broader regex patterns.

Worth double-checking

  • The broader regex may unintentionally match words in unexpected contexts; verify that word boundaries behave as intended.
  • Ensure that any downstream code relying on the previous exact "private context" pattern still functions correctly.

Reviewer B · mistral-small-3.1-24b-instruct — recommends ✅ merge
This PR updates the sanitization logic to better handle public-facing comments, ensuring that terms related to private context and private context are properly redacted. The changes are focused and appear to be well-implemented.

Suggestions

  • Consider adding a comment in the code to explain the rationale behind the changes, especially the addition of 'private context?' and 'private context?' to the forbidden language patterns.
  • Ensure that the test coverage for the new sanitization logic is comprehensive, including edge cases where the forbidden terms might be obfuscated or partially hidden.

Worth double-checking

  • Verify that the updated sanitization logic does not introduce any false positives or negatives, especially in cases where the forbidden terms are used in a non-sensitive context.
  • Check that the new test cases cover all possible variations of the forbidden terms to ensure robustness.

@codecov

codecov Bot commented Jun 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.71%. Comparing base (5d9d73a) to head (aaf5d43).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #913   +/-   ##
=======================================
  Coverage   96.71%   96.71%           
=======================================
  Files         108      108           
  Lines       14641    14641           
  Branches     5298     5298           
=======================================
  Hits        14160    14160           
  Misses        102      102           
  Partials      379      379           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored
JSONbored merged commit 84fcb09 into main Jun 19, 2026
20 checks passed
@JSONbored
JSONbored deleted the codex/fix-public-scenario-serializer-vulnerabilities branch June 19, 2026 05:51
loopover-orb Bot pushed a commit that referenced this pull request Jul 26, 2026
…idden-word gate (#8913)

FORBIDDEN_PATTERN matched only the bare singular `ranking` — the trailing `\b` can never sit between
"g" and "s", so "rankings" slipped past this secondary public-safety gate. Change `ranking` to
`rankings?`, matching the sibling regex fix in input-model.ts (#913), and export the pattern so its
plural coverage can be asserted directly (the term is unreachable through the public builder because
sanitizePublicComment rewrites it before this gate is consulted).

Closes #8886

Co-authored-by: RealDiligent <brave.challenge007@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
loopover-orb Bot pushed a commit that referenced this pull request Jul 26, 2026
…uards (#8920)

scenario-summary.ts kept a stale copy of FORBIDDEN_PUBLIC_LANGUAGE that
missed rewards/estimated rewards/rankings after #913. Export one shared
constant from input-model and cover assertPublicSummaryClean for those
terms.

Closes #8885
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant