Repository navigation
perf!: Add security-audit skill with vulnerability hunting workflow - #897
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47cd23425d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| publishedCondition( | ||
| publicationColumns(parent.target, parentColumns), | ||
| ), |
There was a problem hiding this comment.
Enforce visibility through the full parent chain
When relation-context content types are chained (for example, a reply belongs to a published topic that itself belongs to a hidden forum), this query verifies only the immediate parent's publication columns. Because it never applies that parent's own contentParentRelation, the reply remains publicly readable even though its topic is no longer publicly reachable. Resolve visibility recursively, or otherwise include all ancestor predicates, rather than treating an immediately published row as sufficient.
Useful? React with 👍 / 👎.
| [ | ||
| ...contentRecordPageCacheTags(definition, recordIdOf(payload)), | ||
| ...contentListPageCacheTags(definition), | ||
| ...dependents.flatMap(dependent => contentListPageCacheTags(dependent)), |
There was a problem hiding this comment.
Invalidate cached child detail pages
When a parent is hidden or unpublished after a child detail page has been cached, this invalidates only the dependent type's list tag. The detail route tags a normal child response with its item tag via contentRecordPageCacheTags (public-routes.ts around line 661), so that cached HTML does not match this invalidation; bumping the API generation also does not invalidate the HTML page cache. Consequently, the cached child detail can continue exposing content whose parent now makes it return 404 on a fresh render. Tag child detail reads with their parent or invalidate a type-wide tag shared by dependent item pages.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59dc2eaf38
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| )?.model; | ||
| const parentColumns = model?.columns as Record<string, PgColumn> | undefined; | ||
| const parentId = parentColumns?.id; | ||
| if (!reference || !model || !parentColumns || !parentId) return null; |
There was a problem hiding this comment.
Fail closed when the parent model is unavailable
When a child relation's target definition remains importable but its model is not registered—for example, the target plugin is removed while a dependent plugin remains—this branch returns null, which disables parent filtering entirely. The child detail and list APIs then expose rows whose parent cannot be resolved through the installed public API; the thread route already treats the same missing root model as a 404. Treat an unresolved model/column set as unavailable content or reject this configuration at boot rather than falling back to unrestricted reads.
Useful? React with 👍 / 👎.
- Admin user edits: a non-root admin can no longer edit (reset password, change email, demote) a staff account holding access they lack; root and unrestricted accounts are editable by root only. - Role deletion applies the staff-entry guards (protected, own entry, privilege ceiling) to the staff entries it would cascade away. - Admin email change and admin password set revoke pending password-reset and email-verification tokens. - SSO: `SSOProviderUser.emailVerified` makes provider verification explicit; core refuses an unconfirmed address instead of creating, linking or verifying an account with it. Built-in adapters consistently report it. - Public content reads (list, detail, by ids) drop child records whose parent (thread root or relation search context) is not public, and the parent's writes expire the children's cached reads. - Scheduled-publish effects skip a stale snapshot when the record was deleted, edited or changed visibility since, so retries cannot put withdrawn content back into search. - Rich text: mark/node lookups use own-property checks, so stored types like "constructor" cannot become a React element type. - SSR server fetcher never derives the API origin from the request Host. - AI streams: text already sent before a cancel or failure is charged to the user's points and daily count. - WebSockets remember the authenticating device and are closed (on every instance) when their sessions are revoked. - Give BlogPostAuthors an explicit ReactNode return type and provide core.personalInformationFields in the moderation test harnesses. - Exclude vendored agent skills from oxfmt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 393d542882
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const edges = (await list.hydrate(await keepVisible(data.edges))).map( | ||
| list.project, | ||
| ); |
There was a problem hiding this comment.
Filter parent-hidden records before paginating
When a page contains children whose parents are hidden or unpublished, keepVisible removes those rows only after withPagination has already applied the limit and calculated counts and cursors. Consequently visible rows beyond the page limit are not backfilled, while pageInfo.count, totalCount, totalPages, and navigation cursors still describe the unfiltered records; the response can even report nonzero content while returning no edges. Apply the parent visibility predicate inside the paginated database query so both the page and its metadata use the same row set.
Useful? React with 👍 / 👎.
| ]); | ||
|
|
||
| await deleteSessionCacheKeys(c, { adminSessions, sessions }); | ||
| realtime.closeUserConnections(scope.userId, scope.deviceId ?? null); |
There was a problem hiding this comment.
Close sockets on the normal sign-out path
This close notification runs only through revokeSessions, but DELETE /users/sign_out still calls SessionModel.deleteSession(), which deletes the current session directly and never reaches this hook. If the browser has an open socket when the user signs out, that socket therefore remains indexed as the authenticated user and can continue receiving private sendToUser or room messages until it disconnects independently. Route ordinary sign-out through the same revocation/close mechanism or explicitly close that device's sockets there.
Useful? React with 👍 / 👎.
393d542 to
8a36388
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a36388477
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| SESSION_REVOKED_CLOSE_CODE, | ||
| SESSION_REVOKED_CLOSE_REASON, | ||
| ); | ||
| publish({ data: deviceId, id: "closeUser", type: "closeUser", userId }); |
There was a problem hiding this comment.
Make cross-instance session revocation durable
In a multi-instance deployment, a transient failure of the revoking instance's Redis publisher leaves every remote socket open: this call uses publish(), whose rejection is deliberately swallowed, while only the local registry is closed. A remote instance whose subscriber remains connected will not run the reconnect resync, so that socket can continue receiving private user or room messages after its session has been deleted. Revocation needs an acknowledged/retried delivery path or remote sockets must independently revalidate their sessions.
Useful? React with 👍 / 👎.
Description
What?
Added a comprehensive security audit skill that implements a six-phase vulnerability hunting and validation workflow for codebases. This includes:
validate-coverage-ledger.cjs,validate-findings.cjs): Dependency-free JSON validators with strict input limits and safety checks for coverage ledger and findings dataRECONNAISSANCE.md: Phase 1 - Map source and plan coverageHUNTING.md: Phase 2 - Run coverage-led hunting wavesVALIDATION-AND-REPORTING.md: Phase 3 - Validate candidates and generate reportsSKILL.md: Main skill definition with operating modes and platform terminologyATTACK-CLASSES.md: Attack class taxonomy and selection guidanceWEB-PROTOCOL-AND-AUTH.md: HTTP and authentication boundariesMEMORY-SAFETY-AND-BINARY.md: Memory safety and kernel targetsDESKTOP-MOBILE-AND-LOCAL-IPC.md: Desktop/mobile and local IPCCLOUD-AND-DEPLOYMENT.md: Cloud infrastructure and deploymentDATA-ISOLATION-AND-LIFECYCLE.md: Data isolation and lifecycleAI-AND-LLM.md: AI/LLM-specific security concernsCLIENT-SIDE.md: Browser and client-side securityPROTOCOLS-RPC-AND-MESSAGING.md: Protocol and messaging securityRESOURCE-EXHAUSTION-AND-AVAILABILITY.md: Availability and DoSSUPPLY-CHAIN-AND-RELEASE.md: Supply chain and release securityreport-schema.jsondefining the contract for findings with three verdict branches (confirmed, needs_validation, rejected)Why?
This skill enables systematic, source-grounded vulnerability discovery with clear workflows for hunting, validation, and reporting. It provides:
The validators use conservative resource limits, safe character handling, and comprehensive error reporting to handle untrusted input safely.
Test Plan
Comprehensive test suites included:
validate-coverage-ledger.test.cjs: 740 lines covering structural validation, cardinality limits, field validation, CLI integration, and security edge casesvalidate-findings.test.cjs: 652 lines covering schema validation, verdict branches, unique constraints, and control character injection preventionBoth test suites verify the validators work correctly through the CLI with proper error handling and output safety.
https://claude.ai/code/session_01MVPfoMoYoGyjbGaoFxKfC2