Uh oh!
There was an error while loading. Please reload this page.
fix(sandbox): report a missing Daytona snapshot permission as 403, not an invalid key - #477
Conversation
…t an invalid key Daytona scopes snapshot writes separately from sandbox writes, so an API key can be entirely valid — able to create sandboxes — and still be refused when TrueForge registers the release snapshot it clones every sandbox from. That refusal came back as 422 "Daytona rejected the API key — check the credentials", which points the user at the one thing that is not wrong. The documented fix is to grant Snapshots (write); the error told them to replace the key. The cause is that isDaytonaAuthError treated 401 and 403 as one condition. 401 means the credentials were not accepted and retrying the same key cannot succeed; 403 means Daytona authenticated the key and declined the operation. Collapsing them left the PUT handler with no way to tell "wrong key" from "key missing a permission", so both took the credentials branch. isDaytonaAuthError now matches 401 only, and a new isDaytonaPermissionError matches 403. The settings write maps the permission case to 403 and names write:snapshots in the message, so the response says which grant is missing rather than implying the key is bad. putSandboxProviderRoute declares the 403 so the OpenAPI document describes it. Behaviour change: the forbidden case now answers 403 where it previously answered 422. That is the correction asked for on the issue. It required updating an existing assertion that expected 422 for a DaytonaError(403); that test's real subject is that config is not persisted when the build call fails, and it still asserts the follow-up GET returns 404, so the coverage is preserved and only the status it waits on moved. Not addressed here: the report also suggests falling back to running the image directly when write:snapshots is absent. That decides whether snapshot registration stays mandatory, which is a product question about the provider's setup contract rather than an error-mapping bug, and it would change behaviour for keys that work today. Verified: tests/unit/apis/sandboxProviders.test.ts 13 passed. Full package unit suite 270 passed, 6 failed; those six are LocalSandboxProvider and Code Mode UDS on a Windows host (unix sockets and 0700 chmod) and fail identically on a clean checkout, which runs 269 passed and the same 6 failed. tsc and eslint clean. The five changed files are Prettier clean; the repo-wide format:check failure is a CRLF artifact of a Windows checkout and hits 779 files that this change does not touch. Claude-Session: https://claude.ai/code/session_01XCubdWKdbhQ2vVuDdYu68E
🦋 Changeset detectedLatest commit: 70a8be6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| return error instanceof DaytonaError && error.statusCode === 401; | ||
| } | ||
| /** | ||
| * Daytona authenticated the key but refused the operation (403 forbidden). Snapshot registration is | ||
| * gated on `write:snapshots`, which Daytona scopes separately from `write:sandboxes`, so a key that | ||
| * creates sandboxes can still fail here — the key is valid and needs a permission, not replacing. | ||
| */ | ||
| export function isDaytonaPermissionError(error: unknown): boolean { | ||
| return error instanceof DaytonaError && error.statusCode === 403; |
There was a problem hiding this comment.
Can you see what error message Daytona returns with 401 and 403? Maybe based on that we can show the error message without doing any changes?
Some thing like Invalid Daytona API key: {error.msg}
This is just a thought
There was a problem hiding this comment.
Checked. The messages are:
- 403 —
"Access denied" - 401 —
"Invalid credentials"
And the message does reach us intact, so that part of your idea works. The 403 in this issue never goes through the SDK's axios interceptor at all — registerSnapshot uses raw fetch and builds the error itself, taking the body's message verbatim:
// packages/trueforge-core/src/core/sandbox/provider/DaytonaProvider.ts:322-326constmessage=isRecord(body)&&typeofbody['message']==='string'
? body['message']
: `Daytona snapshot registration failed (${String(response.status)})`;thrownewDaytonaError(message,response.status);The catch is what that string can ever contain. On Daytona's side the 403 is a zero-argument exception:
// daytonaio/daytona, apps/api/src/common/exceptions/access-denied.exception.tsexportclassAccessDeniedExceptionextendsForbiddenException{constructor(){super('Access denied')// no parameter}}thrown by the org permission guard for @RequiredOrganizationResourcePermissions([OrganizationResourcePermission.WRITE_SNAPSHOTS]) on POST /snapshots (snapshot.controller.ts:93). So Daytona will never name the missing permission — "Access denied" is a constant. The only place write:snapshots is knowable is our side, statically, because we know we are POSTing to /snapshots.
That means Invalid Daytona API key: {error.msg} renders, for the exact case in #461:
Invalid Daytona API key: Access denied— with HTTP 422
which still tells the operator their key is invalid when it is not. Daytona authenticated it: the same key creates and deletes sandboxes, and the reporter's GET /snapshots/{missing} returned an authenticated 404. Only snapshot registration is forbidden. So that wording keeps the part of the bug the issue is actually about, and the status stays 422 rather than the 403 you suggested in the issue.
Your underlying point stands though — showing Daytona's own text is better than only showing ours. Both, if you want it:
401 → 422 Daytona rejected the API key — check the credentials (Invalid credentials)
403 → 403 Daytona accepted the API key but denied the request — the key needs the
write:snapshots permission to register the sandbox image (Access denied)
That is this PR plus one interpolation per branch. Say the word and I'll push it.
One thing to flag if you were thinking of the SDK's typed subclasses: DaytonaForbiddenError would not work here. STATUS_CODE_TO_ERROR is only applied on the axios path, and the 403 above is a bare DaytonaError from the fetch path, so instanceof would miss it. The statusCode === 403 check is the one that matches.
Caveat on sourcing: the 403 string is confirmed both in Daytona's source and by the reporter's live run. The 401 string is from source only — daytonaio/daytona's default branch is now just a README, so the newest tag carrying source is v0.190.0 while the bundled SDK is 0.204.1. I have not observed a live 401.
There was a problem hiding this comment.
So better would be:Daytona denied access: the API key is missing required permissions. Grant write:sandboxes, write:snapshots, and delete:snapshots on the key in the Daytona dashboard, then try again.
There was a problem hiding this comment.
Also after the changes can you incluse the SS of the error message, I want to ensure we are not breaking the UI
There was a problem hiding this comment.
Done, verbatim. The 403 body is now:
Daytona denied access: the API key is missing required permissions. Grant write:sandboxes, write:snapshots, and delete:snapshots on the key in the Daytona dashboard, then try again.
Changed in packages/trueforge/src/apis/sandboxProviders.ts, with the assertion in tests/unit/apis/sandboxProviders.test.ts and the changeset text updated to match. Nothing else moved — the 401/403 split, the status mapping, and the declared OpenAPI 403 are unchanged.
The 403 body now reads exactly as asked on review: Daytona denied access: the API key is missing required permissions. Grant write:sandboxes, write:snapshots, and delete:snapshots on the key in the Daytona dashboard, then try again. It names every grant the setup needs and where to add them, instead of only write:snapshots. Nothing else moved: the 401/403 split, the status mapping, and the OpenAPI 403 are unchanged. The assertion in tests/unit/apis/sandboxProviders.test.ts and the changeset text follow the new string. Verified against a standalone server with the Daytona API pointed at a local stub that answers POST /snapshots with 403 "Access denied": PUT /api/v1/settings/sandbox-providers returns 403 with this body, and the Configure Daytona modal renders it as four wrapped lines above Save, in both light and dark themes, with no truncation or layout shift. Claude-Session: https://claude.ai/code/session_01Pdf58MEtVwuM2JRD1gxqPt
Fixes#461.
What was wrong
Daytona scopes snapshot writes separately from sandbox writes. A key that can create sandboxes but has not been granted Snapshots (write) is still a valid key, but TrueForge builds a release snapshot in the Daytona account and clones every sandbox from it, so saving the provider fails at that step. The settings write answered
422 "Daytona rejected the API key — check the credentials", which points the user at the one thing that is not wrong — the documented fix is to add the missing grant, not to replace the key.Root cause
isDaytonaAuthErrortreated 401 and 403 as a single condition:The two are different answers. 401 means the credentials were not accepted, and retrying the same key cannot succeed. 403 means Daytona authenticated the key and declined the operation. Collapsing them left the PUT handler unable to distinguish "wrong key" from "key missing a permission", so both took the credentials branch.
What changed
providerUtils.ts—isDaytonaAuthErrornow matches 401 only; newisDaytonaPermissionErrormatches 403.sandboxProviders.ts— the permission case returns 403 and nameswrite:snapshotsin the message, so the response says which grant is missing instead of implying the key is bad.sandboxProviderRoutes.ts—putSandboxProviderRoutedeclares the 403 so the generated OpenAPI document describes it.Behaviour change
The forbidden case now answers 403 where it previously answered 422. This is the correction asked for on the issue ("The error should have been a 403 instead of a 422").
It required updating one existing assertion that expected 422 for a
DaytonaError(403). That test's actual subject is that config is not persisted when the build call fails; it still asserts the follow-upGETreturns 404, so the coverage is unchanged and only the status it waits on moved. A new test covers the 403 body and message.Not addressed
The report's third bullet suggests falling back to running the image directly when
write:snapshotsis absent. That decides whether snapshot registration stays mandatory, which is a product question about the provider's setup contract rather than an error-mapping bug, and it would change behaviour for keys that work today. Happy to take it in a follow-up if you want it.Verification
tests/unit/apis/sandboxProviders.test.ts: 13 passed.@truefoundry/trueforgeunit suite: 270 passed, 6 failed. The six areLocalSandboxProviderandCode Mode UDSon a Windows host (unix sockets, and a0700chmod that reports0666). They fail identically on a clean checkout ofmain, which runs 269 passed with the same 6 failed — this branch adds exactly one passing test and changes nothing else.pnpm typecheckandpnpm lint:ci: clean.pnpm format:checkdoes fail locally, but on ~779 files includingREADME.md,SECURITY.mdandtsconfig.base.jsonthat this branch never touches; it is a CRLF artifact of a Windows checkout, and the committed blobs are LF. I deliberately did not runprettier --writeacross the repo.https://claude.ai/code/session_01XCubdWKdbhQ2vVuDdYu68E
Note
Low Risk
Localized change to error classification and HTTP status on the settings sandbox-provider PUT path; no auth or persistence logic beyond clearer client-facing errors.
Overview
Daytona 403 responses during sandbox provider setup are no longer treated as invalid API keys.
isDaytonaAuthErrornow matches 401 only; a newisDaytonaPermissionErrorhandles 403 when the key is valid but lacks snapshot-related grants.Saving the sandbox provider via PUT returns 403 (was 422) with a message that tells users to grant
write:sandboxes,write:snapshots, anddelete:snapshotsin the Daytona dashboard. 422 remains for rejected credentials. OpenAPI forputSandboxProviderRoutedocuments the new 403 response.Unit tests cover the 403 body and update the “build fails, config not persisted” case to expect 403 for a forbidden build error.
Reviewed by Cursor Bugbot for commit 70a8be6. Bugbot is set up for automated code reviews on this repo. Configure here.