Skip to content

fix(submissions): catch sensitive identity attestations#2110

Open
JSONbored wants to merge 1 commit into
mainfrom
codex/fix-identity-attestation-risk-bypass
Open

fix(submissions): catch sensitive identity attestations#2110
JSONbored wants to merge 1 commit into
mainfrom
codex/fix-identity-attestation-risk-bypass

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Restore reliable detection of identity-related attestations that were missed by a recent narrowing of the attestation signal while avoiding a broad catch-all attestation match.
  • Ensure identity-attestation phrasing such as passport, government ID, biometric, or generic "identity attestation" continues to route submissions to higher-risk review when appropriate.

Description

  • Added curated sensitive attestation term lists and rebuilt the attestation regexes to match attestations? near a broader set of identity terms (including identity, passport, government id, and biometric) without reintroducing a blanket attestation keyword.
  • Kept the original FINANCIAL_OR_IDENTITY_PATTERN for wallet/KYC/on-chain signals and combined it with the new attestation patterns in hasFinancialOrIdentitySensitiveSignal so the financial_or_identity_sensitive flag still triggers when appropriate.
  • Implemented the term-list-based RegExp usage in packages/registry/src/submission-risk.js to make matches explicit and maintainable.
  • Added regression tests in tests/submission-pr-first.test.ts that assert generic identity-attestation descriptions are flagged as financial_or_identity_sensitive.

Testing

  • Ran the targeted test suite with pnpm test:submission-pr-first, which executed vitest and reported all tests in tests/submission-pr-first.test.ts passing (24 tests passed).
  • Updated unit tests include new scenarios for generic identity attestation wording and they passed under the same test run.

Codex Task

@dosubot dosubot Bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Jun 11, 2026
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@JSONbored, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 11 minutes and 11 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 84288693-dd9f-4a97-9c0e-c4a1692b08f4

📥 Commits

Reviewing files that changed from the base of the PR and between 634faec and c9753dd.

📒 Files selected for processing (2)
  • packages/registry/src/submission-risk.js
  • tests/submission-pr-first.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-identity-attestation-risk-bypass

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gittensory

gittensory Bot commented Jun 11, 2026

Copy link
Copy Markdown

Important

Gittensory found maintainer review notes

Public GitHub metadata was checked for review readiness. Gittensor-specific context appears only when confirmed.

Readiness score: 93/100

Signal Result Evidence Action
Linked issue ⚠️ Missing No linked issue or no-issue rationale found. Explain no-issue PR.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found. No action.
Review load ✅ 20/20 Readiness component derived from cached public PR metadata and labels; size label size:S. No action.
Validation evidence ✅ 25/25 PR body includes validation/test evidence. No action.
Open PR queue ✅ 10/10 0 open PR(s), 0 likely reviewable. No action.
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 62 PR(s), 347 issue(s). No action.
Gate result ✅ Passing No configured blocker found. No action.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Review load = cached public PR metadata such as size labels, changed paths, and preflight status.
  • Open PR queue = repo-wide review pressure; it is not a PR quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 62 PR(s), 347 issue(s).
  • PR-specific overlap: none found.
Maintainer notes
  • No linked issue detected: The planned PR does not reference a closing issue or explicit linked issue number.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
  • Re-run Gittensory review

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers. Learn more about Gittensor contribution workflows.

@gittensory gittensory Bot added the gittensory:reviewed Gittensor contributor context label Jun 11, 2026
@superagent-security

Copy link
Copy Markdown

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

@JSONbored JSONbored closed this Jun 11, 2026
@JSONbored JSONbored reopened this Jun 11, 2026
@reviewwed

reviewwed Bot commented Jun 11, 2026

Copy link
Copy Markdown

🔍 Maintainer advisory review

Reviewed 2 changed file(s) — two independent AI reviewers. This is an advisory review — it does not merge or close the PR.

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

Reviewer A · gpt-oss-120b — recommends ✅ merge
The change expands the identity‑attestation detection regexes to cover a broader set of sensitive terms and adds corresponding tests. The implementation correctly constructs new RegExp objects, integrates with existing risk analysis, and does not introduce external calls or injection vectors. Security impact is minimal; the added patterns are static and safe. Maintainability improves by extracting term lists into constants, though the longer term list could be documented. Tests now cover forward and reverse attestation phrasing, increasing confidence in the new logic. The main regression risk is a higher false‑positive rate, which may cause more submissions to be flagged, but this aligns with the intent to catch sensitive identity attestations. Overall the change appears correct, safe, and well‑tested.

Suggestions

  • Add a comment or documentation explaining the rationale for the expanded term list and the 120‑character window.
  • Consider adding a configurable whitelist or threshold if false positives become problematic.
  • Run the full test suite to ensure no existing tests break due to the broader regex.

Worth double-checking

  • Potential increase in false‑positive flags for submissions that mention identity‑related words in benign contexts.
  • If other parts of the code rely on the exact shape of the original regex (e.g., .source comparisons), they may need adjustment.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
The change expands the identity attestation detection patterns to capture additional sensitive terms (passport, government ID, biometric, etc.) and refactors the patterns into reusable strings for better maintainability. It adds corresponding unit tests that verify the patterns trigger the financial_or_identity_sensitive flag. The modifications are limited to regex construction and test data; they do not introduce execution paths, shell commands, or secret handling. No obvious security, correctness, or regression risks are evident.

No blocking issues spotted.

@JSONbored JSONbored self-assigned this Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark codex gittensory:reviewed Gittensor contributor context size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant