FEATURE: OSEP-0011 for secure access endpoint#754
FEATURE: OSEP-0011 for secure access endpoint#754Pangjiping wants to merge 2 commits intoalibaba:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06873df29a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
06873df to
8cf5d25
Compare
jwx0925
left a comment
There was a problem hiding this comment.
Two inline comments for the P1 issues on the signed preview contract.
| - Existing response remains. | ||
| - If enabled, add header: | ||
| - `OPENSANDBOX-SECURE-ACCESS: <token>` | ||
| 3. **GetSignedEndpoint** | ||
| - New method: |
There was a problem hiding this comment.
GetSignedEndpoint is specified as returning only a URL, but current gateway header mode also requires the routing header OpenSandbox-Ingress-To in Endpoint.headers. As written, the signed-URL flow is not usable for header-route ingress. Please either scope signed URLs to wildcard/URI route modes only, or define the companion headers that must be returned alongside signed_endpoint.
| - New method: | ||
| - `GetSignedEndpoint(sandboxId, port, uri, timeout)` | ||
| - Return: | ||
| - `signed_endpoint = {endpoint}{uri}?osb_signature=<sig>&osb_expires=<epoch>&osb_signed_key=<cipher>` | ||
|
|
||
| ### 2. Signing Contract | ||
|
|
||
| Algorithm: | ||
|
|
||
| - HMAC-SHA256 | ||
| - base64url (no padding) | ||
| - constant-time compare | ||
|
|
||
| Canonical payload: | ||
|
|
||
| - Static token: | ||
| - `v1\nstatic\n{sandbox_id}\n{port}` | ||
| - Signed URL: | ||
| - `v1\nsigned\n{sandbox_id}\n{port}\n{normalized_uri}\n{osb_expires}` | ||
|
|
||
| Rules: |
There was a problem hiding this comment.
The wire contract for signed_endpoint and normalized_uri is still under-specified. {endpoint}{uri}?osb_signature=... breaks when uri already contains a query string, and the text does not define whether osb_expires / osb_signed_key are part of normalized_uri, how existing query parameters are ordered and encoded, or when to append with ? versus &. Please nail down one exact canonical form here; otherwise server and ingress can implement different normalization rules and fail verification for valid links.
jwx0925
left a comment
There was a problem hiding this comment.
Please change the signed access model from a URI-scoped signed URL to a host-scoped signed endpoint.
Right now GetSignedEndpoint signs sandbox_id + port + normalized_uri + expires, which means every distinct path or query string requires a new signature. That makes the feature cumbersome for real browser traffic because a single app session will naturally issue many requests under the same preview host: page navigations, static assets, XHR/fetch calls, redirects, websocket handshakes, and so on.
A better fit here is to make GetSignedEndpoint return a host-scoped signed endpoint instead of a URI-scoped signed URL. In that model, the signed token should authorize access to {sandbox_id, port, expires} at the host level, and the request path/query should pass through without participating in the signature. This keeps verification stateless while avoiding the need to re-sign every URL.
That direction also has practical advantages:
- It matches browser and iframe usage much better.
- It avoids query canonicalization ambiguity entirely.
- It produces a cleaner contract that is closer to Daytona-style signed preview hosts.
- It lets all requests under the same host reuse the same credential until expiry.
If request-scoped signing is still desired for some special cases, it should be a separate mode rather than the default contract of GetSignedEndpoint.
Please revise the proposal accordingly before moving forward.
No description provided.