Repository navigation
security: harden filesystem, routing, and request resource boundaries - #719
devin-ai-integration[bot] wants to merge 1 commit into
Conversation
Co-Authored-By: Epinephrine <luvs01@hanmail.net>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
Important Review skippedThis PR was authored by a bot without an assigned CodeRabbit review seat. Ask an organization administrator to assign a seat to the bot. See the seat-assignment guide. After the seat is assigned, request a review again. ⚙️ Run configuration
You can disable this status message by setting the
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. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔄 Devin is addressing Devin Review findings
Devin Review found 1 bug and 1 security issue on c9787aa. Devin is making a first pass at them and will update this comment once it's done.
View findings · Follow Devin's work
| .chain(std::iter::once(temp_dir)) | ||
| .any(|root| path.starts_with(root)) | ||
| || path.starts_with("/Volumes") |
There was a problem hiding this comment.
🟡 Custom temp root blocks installed apps
When TMPDIR names a user's home, stable_bundle rejects the installed app under that home's Applications directory. The environment-derived root matches the stable bundle path, so Desktop cannot register its bundled command.
Learn more
The bundle check runs before validating a normal macOS app under Applications. std::env::temp_dir() follows the process environment, so its value is not necessarily a dedicated temporary directory. A temporary root that contains a stable install path makes the stable install look temporary even after canonicalization.
Example: Alice has TMPDIR=/Users/alice and runs /Users/alice/Applications/OpenCodex.app/Contents/MacOS/OpenCodex. The executable starts with the reported temporary root, so command registration refuses the installed app.
Recommended fix: Restrict the dynamic temporary root to recognized temporary locations, or explicitly preserve installed Applications paths before applying that root. Add a test with TMPDIR set to an ancestor of a stable user Applications bundle.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const copilotRequestedModel = route.modelId; | ||
| /** Project only bounded Copilot refusal metadata; upstream bodies and session tokens never reach the client. */ | ||
| const copilotRefusalResponse = (error: unknown): Response | undefined => { | ||
| if (error instanceof AdmissionModelDeniedError) return admissionModelDeniedResponse(error); |
There was a problem hiding this comment.
🟥 Copilot scope refusal enters provider recovery
When resolveCopilotSelection denies a renegotiated model during dispatch, copilotRefusalResponse returns its 403 as an upstream result. Provider recovery can then process the admission denial as a provider refusal instead of ending the scoped request.
Recommendation: Propagate admission denials as a distinct terminal result through dispatch and retry layers; never return them as provider fetch Responses, and prohibit retries/failovers after the denial.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
Closing this aggregate in favor of the published split PRs. The original production changes have been mapped to the following replacements:
The replacement is by behavior and regression coverage, not a byte-for-byte copy: cancellation tests were folded into the existing disconnect suite, inbound-wire tool tests moved to a sibling file, and #717 uses the real two-server route regression instead of the aggregate's extra fixture commands. ZCode intentionally reuses the integration reader's 4 MiB limit rather than adding a separate 1 MiB policy. This closes only the duplicate aggregate. It does not merge or approve the split PRs, close Security Cloud findings, or claim that native-platform validation or required CI has completed. Outstanding review threads and the documented test/environment limitations remain with their respective split PRs. |
1 participant
Summary
Verification
bun test tests/claude-integration/claude-prompt-too-long.test.ts tests/claude-integration/claude-native-reasoning-provenance.test.ts— 31 passed, 132 expectations, using pinned Bun 1.4.0.bun run typecheck,bun run privacy:scan,bun run structure:check, andbun test tests/test-layout.test.ts tests/test-layout-tooling.test.tspassed during implementation; exact-head confirmation and the complete focused-check ledger are being finalized.git diff --checkpassed.bun run test:changeddid not complete: its 900-second limit expired after launching 1,676 of 2,237 selected files. This is not a passing result. The full local suite is disproportionately expensive; focused regressions plus exact-head CI are the validation scope.cargo fmt --manifest-path scripts/diagnostics/windows-version-control/Cargo.toml --checkpassed. Windows Rust Clippy currently fails to linkkernel32.libafter MSVC installation; SDK environment repair andasync_contractsexecution are pending.Checklist
Exact-head CI and platform validation must finish before merge.
Link to Devin session: https://app.devin.ai/sessions/0f01892c4cf0467eadea3119e10e09be
Open in Devin Desktop: https://app.devin.ai/desktop/session/0f01892c4cf0467eadea3119e10e09be?variant=devin
Requested by: @luvs01