fix(server): fail closed on public agent routes in ungranted shared runtimes - #4325
Conversation
… routes (issue-inbox#854)
…untimes (issue-inbox#854) GET /api/agents and GET /api/agents/:id called ensureProjectDiscovery without a locality guard, so in a shared multi-project runtime without a host execution grant the INITIALIZATION_ERROR thrown by the discovery guard escaped the handler as a raw 500. Guard both handlers with requiresIsolatedProjectRuntime and answer with the structured project-execution-unavailable 503 problem response, matching the SSR, snippet, and app-router surfaces. Granted shared runtimes still reach discovery and are served.
|
Warning Review limit reachedNext included review available in 4 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
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 |
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
kwakayama has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18bc7f453d
ℹ️ 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".
…e the runtime gate (issue-inbox#854) Three review follow-ups on the public agent discovery routes. - Add public-agent-metadata.handler.ts and public-agents-list.handler.ts to CAPABILITY_GATED_SURFACES. Both call requiresIsolatedProjectRuntime() and both already have paired fail-closed and granted-path tests, so the execution-surface inventory test was the only thing missing. - Validate the agent id before the runtime gate in the metadata handler. A malformed id such as GET /api/agents/% needs neither discovery nor project-code execution, so it keeps the 400 "Invalid agent id" contract on an ungranted shared runtime instead of becoming a retryable 503. Covered by a new test in the shared-runtime describe block. - Extract buildProjectExecutionUnavailableResponse() into server/handlers/utils and use it from both public agent handlers and from snippet.handler.ts, whose block was behaviourally identical (CORS + security + no-store, body on every method). The ssr, markdown preview, app-router, api-handler-wrapper and module copies differ (no CORS, nonce, HEAD null body, executionTopology metadata) and are left as they are. The helper does not call the predicate itself so the inventory test keeps seeing each surface's own gate.
There was a problem hiding this comment.
kwakayama has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
@codex review |
|
Note Automatic reviews are paused because your trial's included automatic processing has been used for this period. Upgrade now, or comment "Gitar review" to run a review anytime. Code Review ✅ Approved 1 resolved / 1 findingsAdds fail-closed validation to public agent routes in ungranted shared runtimes, preventing a raw ✅ 1 resolved✅ Quality: Duplicated shared-runtime denial block across handlers
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
2 participants
In a shared multi-project runtime without a host execution grant,
GET /api/agentsandGET /api/agents/:idcalledensureProjectDiscovery(ctx)with no locality guard, so theVeryfrontErrorthrown bysrc/server/handlers/request/api/project-discovery.ts:151("Remote executable discovery requires an isolated project runtime and cannot run in the shared host", INITIALIZATION_ERROR, 500) escaped the handler and surfaced in Sentry as an unhandled 500. BothPublicAgentsListHandlerandPublicAgentMetadataHandlernow checkrequiresIsolatedProjectRuntime(ctx)before discovery and return a structured RFC 9457 problem response (503,application/problem+json, typehttps://veryfront.com/docs/code/guides/errors#project-execution-unavailable) built through the response builder with CORS/security headers andno-storecaching, exactly mirroring the sibling surfaces (ssr.handler, snippet.handler, app-router-handler). Shared runtimes the host granted execution (allowHostProjectCodeExecution: true) and explicit local projects still reach discovery and are served as before.Fixes veryfront/veryfront-issue-inbox#854
Red
Before the fix:
Green
After the fix, same command:
Affected suite:
deno task test:file src/server/handlers/request/-> ok | 69 passed (612 steps) | 0 failed (10s).deno check,deno fmt --check, anddeno lintare clean on both changed handlers.Revert check
Recorded HEAD 18bc7f4.
git revert --no-commit 18bc7f453(only the two handler .ts files reverted; tests kept), then the red test command:Both "fails closed with project-execution-unavailable instead of leaking the discovery error" steps fail with
VeryfrontError: Remote executable discovery requires an isolated project runtime and cannot run in the shared hostatsrc/server/handlers/request/api/project-discovery.ts:151, escaping viawithProxyContext(base-handler.ts:211), which is the exact Sentry leak.git reset --hard 18bc7f453-> ok | 2 passed (14 steps) | 0 failed. Broader sweep at restored HEAD:deno task test:file src/server/handlers/request/-> ok | 69 passed (612 steps) | 0 failed.Acceptance criteria
GET /api/agentsandGET /api/agents/:idin a shared multi-project runtime without a host execution grant return a structured RFC 9457 problem response instead of letting the raw VeryfrontError escape the handlerapplication/problem+jsonand problem typehttps://veryfront.com/docs/code/guides/errors#project-execution-unavailable, matching the convention of sibling surfaces (ssr.handler, snippet.handler, app-router-handler, api-handler-wrapper)allowHostProjectCodeExecution: true, or explicit local project) is still served: the guard isrequiresIsolatedProjectRuntime(ctx), not a blanket denial, and the granted-counterpart tests assert 200 plus that discovery ran