Skip to content

feat(codex): add security findings workflow#321

Open
umgbhalla wants to merge 5 commits into
steipete:mainfrom
umgbhalla:feat/codex-findings
Open

feat(codex): add security findings workflow#321
umgbhalla wants to merge 5 commits into
steipete:mainfrom
umgbhalla:feat/codex-findings

Conversation

@umgbhalla

@umgbhalla umgbhalla commented Jul 12, 2026

Copy link
Copy Markdown

Motivation

Oracle previously had no Codex Cloud security-findings workflow. This PR adds the complete path from discovery to action: it can enumerate findings, open a selected finding, extract the report and evidence needed to understand it, and then invoke the finding’s supported detail-panel actions from the CLI.

The first part is deliberately read-only. It turns the authenticated findings surface into structured CLI output instead of forcing an operator to navigate the web UI, while preserving the report sections and evidence links that explain why a finding exists. The second part adds the operational step that was missing before: an explicit finding action command that can create a PR, start a finding chat, close or adjust a finding, and copy the available patch or report material.

Design

Discovery and report extraction

  • oracle codex findings enumerates the paginated findings surface, reports severity and repository metadata, supports severity/limit filtering, and can emit JSON.
  • oracle codex findings --finding <selection-id> opens one finding and extracts its title, repository/commit reference, Summary, Validation, Evidence, and Attack-path analysis sections.
  • Detail extraction also returns deduplicated GitHub evidence links and the signed validation-artifact URL without fetching or modifying the artifact.
  • Finding IDs are validated before they are used to build detail URLs, and the browser runner uses the authenticated browser surface rather than an unauthenticated API shortcut.

Explicit finding actions

  • oracle codex finding --finding <selection-id> --action <action> provides the action layer for create-pr, chat, close, adjust, copy-content, copy-link, copy-patch, and copy-git-apply.
  • The action runner targets the accessible detail-panel controls with trusted CDP mouse events. This is required because the Radix pointer-event controls can ignore synthetic element.click() calls.
  • Chat requires --text, while mutating actions require --confirm; read-only copy actions do not require confirmation.
  • Create PR opens the git action menu and checks for View PR before clicking Create PR, so rerunning the command returns an existing PR instead of creating a duplicate.
  • Actions can be constrained to an expected GitHub repository with --repo, and the finding detail is re-read immediately before the action is dispatched.

Compatibility and safety

The original oracle codex findings command remains read-only, and adding the action command does not make list or detail inspection mutating. The runner refuses mutating actions without explicit confirmation and refuses to proceed when trusted browser input is unavailable, so a failed automation primitive cannot be reported as a successful action.

Validation

  • pnpm exec tsc --noEmit
  • pnpm exec vitest run tests/codex/findings.test.ts — 16 passed
  • pnpm run format:check
  • pnpm run lint
  • pnpm run build
  • pnpm test — 1,572 passed, 44 skipped

Exact-head live proof

Current head: 1b7d6c7cfe42dc2668ccfde8c7f6c57b51b1cbc7

  • Authenticated findings launch returned 33 findings.
  • A selected finding’s report was read successfully, including its evidence links and validation artifact reference.
  • A trusted copy action completed successfully.
  • Repeating Create PR returned that existing PR instead of creating a duplicate.

umgbhalla and others added 2 commits July 12, 2026 21:26
List ChatGPT Codex Cloud security findings and inspect a single finding's
detail sections, over the existing signed-in browser automation.

- list: scrape the SSR'd `li > button` rows in the main world and page
  through with a deny-list-guarded Next-page click; aggregate + dedupe.
- detail: direct-URL nav to `/findings/{id}?sev=` (32-hex validated), then
  read #summary/#validation/#evidence/#attack-path + github /blob/ evidence
  links + the signed validation artifact.

Read-only by construction: the CDP Input domain is never wired, so the
mutating Radix actions (Create PR / Patch / Chat / Close / Adjust / feedback)
are unreachable; the only click path is exact-aria-label Prev/Next paging.
No repo/account/id is ever hardcoded.

Mirrors the project-sources runner for the Chrome lifecycle (manual-login or
cookie-synced temp profile, lease/lock, teardown). Pure parsers live in
src/codex/* with unit tests; browser/action code follows the existing
DOM-eval idiom.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@umgbhalla umgbhalla changed the title feat(codex): add trusted finding actions feat(codex): add security findings workflow Jul 12, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels Jul 12, 2026
@clawsweeper

clawsweeper Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 12, 2026, 2:21 PM ET / 18:21 UTC.

Summary
The PR adds CLI commands and authenticated browser automation to list, inspect, and perform actions on Codex Cloud security findings.

Reproducibility: yes. for the review findings: current source directly exposes the unchanged pull-request-link and menu-closure predicates. An authenticated browser run is additionally required to demonstrate the repaired live behavior.

Review metrics: 3 noteworthy metrics.

  • Change surface: 10 files, 1,933 additions. The PR introduces an extensive authenticated browser workflow rather than a small CLI wrapper.
  • Action surface: 8 actions, 4 mutating. Create PR, chat, close, and adjust require stronger completion guarantees than read-only inspection.
  • Prior blockers: 2 still present. Both actionable findings from the previous completed review remain unchanged on the current head.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🦐 gold shrimp
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Fix both action-verification findings and add focused regression coverage.
  • Post redacted exact-head authenticated output for Create PR and patch-copy success and refusal paths.
  • Obtain explicit repository-owner approval for the core CLI mutation surface.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR body contains useful authenticated results for 1b7d6c7, but current head 840b727 includes later hardening and copied-profile changes without redacted exact-head proof; update the PR body to trigger a fresh review, or ask a maintainer to comment @clawsweeper re-review, and redact credentials, private endpoints, repository details, and other sensitive data.

Risk before merge

  • [P1] Create PR can report success using an unrelated or pre-existing pull-request URL even when the requested creation did not complete.
  • [P1] Patch-copy actions can report success after dismissal or clipboard failure because menu closure is not evidence that content was copied.
  • [P1] The current head changes copied-profile authentication behavior, but the PR body provides no redacted live result from that exact head.
  • [P1] Even after mechanical repair, maintainers must explicitly accept adding authenticated commands that can create PRs, chat, close, or adjust security findings.

Maintainer options:

  1. Repair and prove the full workflow (recommended)
    Require finding-local PR outcomes and direct clipboard confirmation, add focused tests, then obtain redacted exact-head authenticated proof.
  2. Reduce to read-only discovery
    Remove or defer the action subcommand and retain listing and detail extraction if reliable mutation completion cannot be established.
  3. Pause pending product sponsorship
    Do not land the new operational surface until a repository owner explicitly accepts authenticated finding mutations in Oracle core.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Harden Codex finding action postconditions so Create PR requires a newly observed finding-local PR outcome and patch-copy actions require copied or clipboard confirmation; add focused tests while preserving explicit confirmation, repository scope, and evidence-path safeguards.

Next step before merge

  • [P1] The code defects are mechanically repairable, but automation should pause until the core mutation surface has explicit product sponsorship; contributor-supplied authenticated proof is also required after repair.

Maintainer decision needed

  • Question: Should Oracle expose authenticated mutating Codex security-finding actions in the core CLI after the correctness and proof blockers are resolved?
  • Rationale: The PR adds a substantial new operational surface that can create PRs, start chats, close findings, and adjust findings; code review can establish safety mechanics but cannot authorize that product boundary.
  • Likely owner: Peter Steinberger — Current-main history connects Peter Steinberger to the browser and CLI foundations, making him the best available owner for the core CLI product boundary.
  • Options:
    • Accept the full workflow (recommended): Sponsor the list, detail, copy, and mutating action commands after the predicates are repaired and exact-head proof is supplied.
    • Land read-only discovery: Accept listing and detail extraction while deferring all action commands until the browser contract is more stable.
    • Decline the feature: Keep security-finding operations in the Codex web interface and close the PR after preserving any reusable research externally.

Security
Needs attention: Trusted actions are panel-scoped, but false-success predicates still weaken the safety guarantees of authenticated security-finding operations.

Review findings

  • [P1] Require a newly observed finding-local PR link — src/browser/actions/codexFindings.ts:474
  • [P2] Verify clipboard success before returning — src/browser/actions/codexFindings.ts:467
Review details

Best possible solution:

Preserve read-only discovery, make each action succeed only on a newly observed selected-finding outcome or directly verified clipboard result, and demonstrate successful and refused paths on the exact head before accepting the mutation commands.

Do we have a high-confidence way to reproduce the issue?

Yes for the review findings: current source directly exposes the unchanged pull-request-link and menu-closure predicates. An authenticated browser run is additionally required to demonstrate the repaired live behavior.

Is this the best way to solve the issue?

No in its current form; panel targeting and explicit confirmation are appropriate, but successful completion must be tied to a newly observed action-specific result rather than generic page state.

Full review comments:

  • [P1] Require a newly observed finding-local PR link — src/browser/actions/codexFindings.ts:474
    Create PR succeeds on any truthy document-wide state.pr without comparing it with before.pr. An unrelated or already-rendered PR link can therefore make a failed creation return success; scope the link to the finding panel and require a new or changed result.
    Confidence: 0.99
  • [P2] Verify clipboard success before returning — src/browser/actions/codexFindings.ts:467
    Patch-copy completion only checks that the git action menu closed. Dismissal or clipboard failure can also close the menu, so require a new copied status or direct clipboard confirmation before returning success.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against ec2e99bf57b4.

Label changes

Label justifications:

  • P2: This optional CLI feature has bounded user impact but two concrete correctness blockers, missing current-head proof, and an unresolved product boundary.
  • merge-risk: 🚨 security-boundary: False-success results on authenticated security-finding actions can mislead operators about PR creation and copied patch material.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body contains useful authenticated results for 1b7d6c7, but current head 840b727 includes later hardening and copied-profile changes without redacted exact-head proof; update the PR body to trigger a fresh review, or ask a maintainer to comment @clawsweeper re-review, and redact credentials, private endpoints, repository details, and other sensitive data.
Evidence reviewed

Security concerns:

  • [medium] Authenticated actions can report false success — src/browser/actions/codexFindings.ts:474
    PR creation and patch copying are inferred from weak page state instead of newly observed action-specific outcomes, which can mislead operators handling security findings.
    Confidence: 0.99

What I checked:

Likely related people:

  • Peter Steinberger: Current-main history attributes the shared browser configuration, lifecycle, and CLI foundations used by the new workflow to Peter Steinberger. (role: browser and CLI feature owner; confidence: high; commits: 54acb79dc716; files: src/cli/browserConfig.ts, src/browser/chromeLifecycle.ts, bin/oracle-cli.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (2 earlier review cycles)
  • reviewed 2026-07-12T17:25:49.534Z sha 1b7d6c7 :: needs real behavior proof before merge. :: [P1] Scope actions to the finding panel and verify completion | [P2] Remove the Harp-specific --modal-only filter | [P2] Enforce --repo consistently or limit it to actions
  • reviewed 2026-07-12T18:00:44.810Z sha 498d1b0 :: needs real behavior proof before merge. :: [P1] Require a newly observed finding-local PR link | [P2] Verify clipboard success before returning

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels Jul 12, 2026
@clawsweeper

clawsweeper Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper status: review started.

I am starting a fresh review of this pull request: feat(codex): add security findings workflow This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

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

Labels

merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant