feat: extract Slice 8 asset capability base#229
Conversation
![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: f12adaa10d
ℹ️ 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".
| createUrlQuery({ | ||
| environmentId: target.environmentId, | ||
| input: withAssetClientCapabilities(target.input), | ||
| }); |
There was a problem hiding this comment.
Please don't advertise same-origin-relay-v1 from the shared asset atom until every consumer can present the returned surfaceCredential. This shared path is used by the web app too; for bearer/DPoP web connections there is no browser session cookie on plain <img src> requests, and apps/web/src/assets/assetUrls.ts:23-35 drops surfaceCredential and exposes only the URL. The server now rejects relay URLs without a matching cookie/header, so attachments, favicons, and image previews 404 in those web connection modes; plumb a web binding/header flow or gate the capability to clients that implement it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed and fixed in d583bff. The server rejects relay fetches without matching session proof, the web URL helper drops surfaceCredential, and the shared atom advertised the relay unconditionally. Asset negotiation now defaults to no relay capability, mobile explicitly opts in because it presents the credential, and ungated web/bearer/DPoP consumers retain the legacy signed direct-URL path. Unbound claims are accepted only on that direct route; relay claims still require matching proof. Regression coverage verifies the web/bearer gate, capable-client advertisement, and legacy direct fetch.
| ?.trim() | ||
| .toLowerCase(); | ||
| const secure = | ||
| (requestOrigin !== null && new URL(requestOrigin).protocol === "https:") || |
There was a problem hiding this comment.
If the binding POST carries an opaque or malformed Origin such as Origin: null (which WebViews or sandboxed browser contexts can send), normalizeCorsOrigin() returns that raw string and this new URL(requestOrigin) throws before the cookie is set. That turns the asset-surface binding flow into a 500 instead of simply treating the request as non-HTTPS; parse defensively here or only inspect the protocol after a successful URL parse.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d583bff. The binding route now parses a normalized Origin defensively and returns a masked 404 for opaque or malformed values before setting any cookie. The integration test covers both Origin: null and malformed input and asserts no Set-Cookie header. I swept the PR diff for other Origin/Referer-derived new URL calls; this was the only unguarded one (the request.originalUrl fallback and redirect parser were already caught/validated).
| () => ({ | ||
| uri: props.source.uri, | ||
| headers: props.source.headers, | ||
| cache: "force-cache" as const, | ||
| }), |
There was a problem hiding this comment.
When props.source.headers is present, this image is a session-bound relay fetch that the server marks Cache-Control: no-store; keeping cache: "force-cache" tells React Native to satisfy repeat requests from an existing cached entry regardless of age, so a revoked/expired surface can still display the cached private image instead of revalidating the proof. Use the default/reload cache mode for credentialed sources and keep force-caching only for the local cached-file path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d583bff. Credential-bearing WorkspaceFileImagePreview sources now omit force-cache and use React Native default cache behavior; force-cache remains only for the headerless local cached-file source. I swept ProjectFavicon.tsx, ThreadFilesRouteScreen.tsx, WorkspaceFileWebPreview.tsx, ThreadFeed.tsx, and workspaceFileAssetUrl.ts as requested; none contained another force-cache occurrence.
Summary
Extract the smallest coherent Slice 8 prerequisite from the conflicting #208 branch so relay Slices A/B/C and the final #208 rebase can build on a landable main commit.
Binding design: /home/adam/.openclaw/reports/app-origin-asset-relay-design-2026-07-21.md
Parent threat model: /home/adam/.openclaw/reports/data-segregation-design-2026-07-17.md §2
This base adds:
The asset URL contains only a keyed binding identifier, never the auth session id or proof. Asset responses are no-store. The optional mobile WebView cookie is host-only, HttpOnly, SameSite=Lax, and path-scoped to /api/assets/relay.
Extraction rationale
This is a strict subset of #208 posture. It ports only the capability/session-binding primitives and matching same-surface client support needed to avoid breaking current main. It does not merge the Slice 8 draft or its broader asset/filesystem changes.
Verification
Explicitly left for later slices
No project audience classification, pairing-grant ceiling, or listener-split behavior is changed.