fix: harden browser media and remote CLI attachments#347
Conversation
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: fix: harden browser media and remote CLI attachments 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a62dffca95
ℹ️ 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".
| if (args.sourceKind !== "asset-url" || !isPathBasedAttachment(args)) return attempts; | ||
| return attempts.filter((attempt) => isRemoteCliAttachmentProviderSafe(attempt, args)); |
There was a problem hiding this comment.
Preserve CLI attempts for inline remote files
When a remote URL resolves to a text-like file (or a document that preprocessing turns into inline text), prepareAssetPrompt sends only prompt text and buildAssetCliContext returns null because there are no attachments, so no staged path or broad tool access is involved. This check still treats every asset-url with attachment.kind === "file" as unsafe and filters out providers like cli/claude/cli/gemini; with an explicit unsafe-filtered CLI model, executableAttempts becomes empty and the new error is thrown for otherwise safe remote text summaries. Gate the filter on whether a path-based attachment is actually used rather than the original file kind.
Useful? React with 👍 / 👎.
Summary
Security Review
Deep scan results:
Verification
./node_modules/.bin/vitest run tests/chrome.browser-media.test.ts tests/asset.summary-attempts.test.ts tests/llm.cli.test.ts tests/llm.cli.copilot.test.ts./node_modules/.bin/vitest run tests/asset.summary-branches.test.ts./node_modules/.bin/oxfmt --check CHANGELOG.md docs/cli.md apps/chrome-extension/src/entrypoints/background/browser-media.ts src/run/flows/asset/summary-attempts.ts src/run/flows/asset/summary.ts tests/asset.summary-attempts.test.ts tests/chrome.browser-media.test.ts./node_modules/.bin/oxlint --type-aware --tsconfig tsconfig.build.json --config .oxlintrc.json ../node_modules/.bin/tsgo -p tsconfig.build.json --noEmit./node_modules/.bin/tsgo -p tsconfig.build.jsonnode scripts/build-cli.mjs./node_modules/.bin/wxt buildfromapps/chrome-extension