fix(server): strip framing headers from every surface-destined response
A dev server that sends X-Frame-Options or a frame-ancestors CSP (Shopify's `shopify theme dev` does) told the browser not to render the app inside the canvas frame airship itself created — the shell painted, the frame showed a broken-document icon, and nothing explained why. PR #16 diagnosed this and stripped the headers on the injection path. This goes the rest of the way: responses the proxy cannot inject into (gzip upstream, non-UTF-8 charset) took the passthrough branch, which forwarded every upstream header verbatim, so the frame stayed blank exactly when the overlay was already degraded. resolveMode is now computed before the upstream round trip so the passthrough branch knows a response is frame-destined, and one exported pure filter serves both branches. The CSP is dropped wholesale, not edited: the policy governs the injected editor itself (the inline config script, injected styles, the control socket), so surgically removing frame-ancestors would still break the overlay under a strict script-src. --keep-csp / AIRSHIP_KEEP_CSP opts back in for someone iterating on their own policy; X-Frame-Options is dropped regardless, since inside the editor it can only blank the frame. Diagnosis and the shopify theme dev reproduction: jormon (#16).
This commit is contained in:
@@ -271,6 +271,7 @@ forwarded.
|
||||
| `--mode <name>` | Editor mode: `canvas` or `inline`. Switchable from the editor too. | `canvas` |
|
||||
| `--exec <command>` | Start your dev server with this command and stop it when airship exits. | |
|
||||
| `--open` | Open the editor in your browser once it is listening. | |
|
||||
| `--keep-csp` | Keep your app's `Content-Security-Policy` on editor surfaces instead of stripping it. Framing headers (`X-Frame-Options`) are always stripped. | off |
|
||||
|
||||
### Agent
|
||||
|
||||
@@ -399,7 +400,7 @@ AIRSHIP_EXEC AIRSHIP_MAX_BUDGET AIRSHIP_OPENCODE_AGENT
|
||||
AIRSHIP_OPEN AIRSHIP_COMMIT AIRSHIP_OPENCODE_CONFIG
|
||||
AIRSHIP_SAFE AIRSHIP_JSON AIRSHIP_OPENCODE_MODEL
|
||||
AIRSHIP_DEBUG AIRSHIP_QUIET AIRSHIP_CLAUDE_MODEL
|
||||
AIRSHIP_CODEX_MODEL
|
||||
AIRSHIP_KEEP_CSP AIRSHIP_CODEX_MODEL
|
||||
```
|
||||
|
||||
`AIRSHIP_HELP` and `AIRSHIP_VERSION` are deliberately not read — exporting one would leave the
|
||||
@@ -447,6 +448,13 @@ dev server on it. It won't start one on a port that's already taken.
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
**The canvas frame is blank, or shows a broken-document icon.**
|
||||
Your dev server is probably sending `X-Frame-Options` or a `Content-Security-Policy` with
|
||||
`frame-ancestors` (Shopify's `shopify theme dev` does), which told the browser not to render the
|
||||
app inside airship's canvas frame. Airship strips those headers from the surfaces it serves, so
|
||||
this should not happen — unless you passed `--keep-csp`, which keeps your CSP and with it any
|
||||
`frame-ancestors` restriction. Drop the flag, or loosen the policy while editing.
|
||||
|
||||
**Every `opencode` turn fails with "Thinking mode does not support this tool_choice".**
|
||||
The provider is rejecting the structured-output request opencode sends — it is implemented as a
|
||||
forced tool call, which models with thinking/reasoning enabled refuse (opencode issue #15226,
|
||||
|
||||
+9
-1
@@ -273,6 +273,7 @@ forwarded.
|
||||
| `--mode <name>` | Editor mode: `canvas` or `inline`. Switchable from the editor too. | `canvas` |
|
||||
| `--exec <command>` | Start your dev server with this command and stop it when airship exits. | |
|
||||
| `--open` | Open the editor in your browser once it is listening. | |
|
||||
| `--keep-csp` | Keep your app's `Content-Security-Policy` on editor surfaces instead of stripping it. Framing headers (`X-Frame-Options`) are always stripped. | off |
|
||||
|
||||
### Agent
|
||||
|
||||
@@ -401,7 +402,7 @@ AIRSHIP_EXEC AIRSHIP_MAX_BUDGET AIRSHIP_OPENCODE_AGENT
|
||||
AIRSHIP_OPEN AIRSHIP_COMMIT AIRSHIP_OPENCODE_CONFIG
|
||||
AIRSHIP_SAFE AIRSHIP_JSON AIRSHIP_OPENCODE_MODEL
|
||||
AIRSHIP_DEBUG AIRSHIP_QUIET AIRSHIP_CLAUDE_MODEL
|
||||
AIRSHIP_CODEX_MODEL
|
||||
AIRSHIP_KEEP_CSP AIRSHIP_CODEX_MODEL
|
||||
```
|
||||
|
||||
`AIRSHIP_HELP` and `AIRSHIP_VERSION` are deliberately not read — exporting one would leave the
|
||||
@@ -449,6 +450,13 @@ dev server on it. It won't start one on a port that's already taken.
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
**The canvas frame is blank, or shows a broken-document icon.**
|
||||
Your dev server is probably sending `X-Frame-Options` or a `Content-Security-Policy` with
|
||||
`frame-ancestors` (Shopify's `shopify theme dev` does), which told the browser not to render the
|
||||
app inside airship's canvas frame. Airship strips those headers from the surfaces it serves, so
|
||||
this should not happen — unless you passed `--keep-csp`, which keeps your CSP and with it any
|
||||
`frame-ancestors` restriction. Drop the flag, or loosen the policy while editing.
|
||||
|
||||
**Every `opencode` turn fails with "Thinking mode does not support this tool_choice".**
|
||||
The provider is rejecting the structured-output request opencode sends — it is implemented as a
|
||||
forced tool call, which models with thinking/reasoning enabled refuse (opencode issue #15226,
|
||||
|
||||
@@ -48,6 +48,7 @@ export const SERVE_FLAGS: readonly string[] = [
|
||||
"mode",
|
||||
"exec",
|
||||
"open",
|
||||
"keep-csp",
|
||||
"agent",
|
||||
"model",
|
||||
"effort",
|
||||
@@ -75,6 +76,7 @@ export interface ServeOptions {
|
||||
effort?: Effort;
|
||||
exec?: string;
|
||||
json: boolean;
|
||||
keepCsp: boolean;
|
||||
maxBudgetUsd?: number;
|
||||
maxTurns?: number;
|
||||
model?: string;
|
||||
@@ -118,6 +120,7 @@ export function toServeOptions(settings: Settings, cwd: string): ServeOptions {
|
||||
effort: effort ? (requireEnum(effort, "effort") as Effort) : undefined,
|
||||
exec: asString(settings, "exec"),
|
||||
json: asBoolean(settings, "json"),
|
||||
keepCsp: asBoolean(settings, "keep-csp"),
|
||||
maxBudgetUsd: budget ? requireAmount(budget, "max-budget") : undefined,
|
||||
maxTurns: turns ? requireInteger(turns, "max-turns") : undefined,
|
||||
model,
|
||||
@@ -305,6 +308,7 @@ export const serve = defineCommand({
|
||||
codex: opts.codex,
|
||||
cwd: opts.cwd,
|
||||
effort: opts.effort,
|
||||
keepCsp: opts.keepCsp,
|
||||
maxBudgetUsd: opts.maxBudgetUsd,
|
||||
maxTurns: opts.maxTurns,
|
||||
model: opts.model,
|
||||
|
||||
@@ -20,6 +20,7 @@ const NAMES = [
|
||||
"codex-config",
|
||||
"opencode-url",
|
||||
"safe",
|
||||
"keep-csp",
|
||||
"help",
|
||||
];
|
||||
|
||||
@@ -53,6 +54,7 @@ describe("assertKnownFlags", () => {
|
||||
|
||||
it("accepts --no- negation of a boolean", () => {
|
||||
expect(() => assertKnownFlags(["--no-safe"], NAMES)).not.toThrow();
|
||||
expect(() => assertKnownFlags(["--no-keep-csp"], NAMES)).not.toThrow();
|
||||
});
|
||||
|
||||
it("rejects an unknown flag and suggests the near miss", () => {
|
||||
|
||||
@@ -109,6 +109,13 @@ export const FLAGS: readonly FlagSpec[] = [
|
||||
name: "open",
|
||||
type: "boolean",
|
||||
},
|
||||
{
|
||||
defaultHint: "off",
|
||||
group: "CORE",
|
||||
help: "Keep your app's Content-Security-Policy on editor surfaces instead of stripping it. Framing headers (X-Frame-Options) are always stripped; a kept CSP can block the editor's own scripts.",
|
||||
name: "keep-csp",
|
||||
type: "boolean",
|
||||
},
|
||||
{
|
||||
alias: "a",
|
||||
defaultHint: "claude",
|
||||
|
||||
@@ -136,6 +136,13 @@ describe("loadConfig", () => {
|
||||
expect(() => loadConfig(root)).toThrow(CliError);
|
||||
});
|
||||
|
||||
it("accepts keep-csp in either spelling", () => {
|
||||
const kebab = fixture({ "airship.config.json": '{ "keep-csp": true }' });
|
||||
expect(loadConfig(kebab).values["keep-csp"]).toBe(true);
|
||||
const camel = fixture({ "airship.config.json": '{ "keepCsp": true }' });
|
||||
expect(loadConfig(camel).values["keep-csp"]).toBe(true);
|
||||
});
|
||||
|
||||
it("reports malformed JSON rather than falling through", () => {
|
||||
const root = fixture({ "airship.config.json": "{ nope" });
|
||||
expect(() => loadConfig(root)).toThrow(CliError);
|
||||
@@ -166,6 +173,11 @@ describe("envSettings", () => {
|
||||
expect(() => envSettings({ AIRSHIP_SAFE: "maybe" })).toThrow(CliError);
|
||||
});
|
||||
|
||||
it("reads AIRSHIP_KEEP_CSP as the keep-csp boolean", () => {
|
||||
expect(envSettings({ AIRSHIP_KEEP_CSP: "1" })["keep-csp"]).toBe(true);
|
||||
expect(envSettings({ AIRSHIP_KEEP_CSP: "off" })["keep-csp"]).toBe(false);
|
||||
});
|
||||
|
||||
// A shell profile that exported these would make the CLI unable to run.
|
||||
it("ignores AIRSHIP_HELP and AIRSHIP_VERSION", () => {
|
||||
expect(envSettings({ AIRSHIP_HELP: "1", AIRSHIP_VERSION: "1" })).toEqual(
|
||||
|
||||
@@ -77,6 +77,11 @@ export interface ServerOptions {
|
||||
/** Project root for file edits. */
|
||||
cwd: string;
|
||||
effort?: Effort;
|
||||
/**
|
||||
* Keep upstream `Content-Security-Policy` headers on served surfaces
|
||||
* instead of stripping them. `X-Frame-Options` is dropped regardless.
|
||||
*/
|
||||
keepCsp?: boolean;
|
||||
maxBudgetUsd?: number;
|
||||
/** Claude-only turn cap. */
|
||||
maxTurns?: number;
|
||||
@@ -580,6 +585,7 @@ export async function startServer(opts: ServerOptions): Promise<RunningServer> {
|
||||
|
||||
const server = createProxyServer({
|
||||
defaultMode: surfaceToMode(opts.surface ?? "canvas"),
|
||||
keepCsp: opts.keepCsp,
|
||||
onAirshipUpgrade: (req, socket, head) => {
|
||||
wss.handleUpgrade(req, socket, head, (ws) => {
|
||||
wss.emit("connection", ws, req);
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import type http from "node:http";
|
||||
import { AIRSHIP_SURFACE_COOKIE } from "@airship/protocol";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { resolveMode } from "./proxy";
|
||||
import { filterProxyHeaders, resolveMode } from "./proxy";
|
||||
|
||||
/** Just enough of an IncomingMessage for `resolveMode`. */
|
||||
function req(opts: {
|
||||
@@ -115,3 +115,88 @@ describe("resolveMode", () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("filterProxyHeaders", () => {
|
||||
// As a dev server would send them: original casing, framing headers set.
|
||||
const upstream = {
|
||||
"Content-Security-Policy": "frame-ancestors 'none'; script-src 'self'",
|
||||
"Content-Security-Policy-Report-Only": "frame-ancestors 'none'",
|
||||
"cache-control": "no-cache",
|
||||
"content-encoding": "gzip",
|
||||
"content-length": "1234",
|
||||
"content-type": "text/html",
|
||||
"X-Frame-Options": "DENY",
|
||||
} as http.IncomingHttpHeaders;
|
||||
|
||||
it("strips framing headers from a frame-destined passthrough", () => {
|
||||
const out = filterProxyHeaders(upstream, {
|
||||
forSurface: true,
|
||||
injecting: false,
|
||||
keepCsp: false,
|
||||
});
|
||||
expect(out["X-Frame-Options"]).toBeUndefined();
|
||||
expect(out["Content-Security-Policy"]).toBeUndefined();
|
||||
expect(out["Content-Security-Policy-Report-Only"]).toBeUndefined();
|
||||
// Passthrough bodies are untouched, so length and encoding must survive.
|
||||
expect(out["content-length"]).toBe("1234");
|
||||
expect(out["content-encoding"]).toBe("gzip");
|
||||
expect(out["cache-control"]).toBe("no-cache");
|
||||
});
|
||||
|
||||
it("keeps the CSP pair under keepCsp, but never X-Frame-Options", () => {
|
||||
const out = filterProxyHeaders(upstream, {
|
||||
forSurface: true,
|
||||
injecting: false,
|
||||
keepCsp: true,
|
||||
});
|
||||
expect(out["Content-Security-Policy"]).toBe(
|
||||
"frame-ancestors 'none'; script-src 'self'"
|
||||
);
|
||||
expect(out["Content-Security-Policy-Report-Only"]).toBe(
|
||||
"frame-ancestors 'none'"
|
||||
);
|
||||
expect(out["X-Frame-Options"]).toBeUndefined();
|
||||
});
|
||||
|
||||
it("leaves a subresource passthrough completely untouched", () => {
|
||||
const out = filterProxyHeaders(upstream, {
|
||||
forSurface: false,
|
||||
injecting: false,
|
||||
keepCsp: false,
|
||||
});
|
||||
expect(out).toEqual(upstream);
|
||||
});
|
||||
|
||||
it("strips framing headers and hop-by-hop when injecting", () => {
|
||||
const out = filterProxyHeaders(
|
||||
{ ...upstream, connection: "keep-alive", "transfer-encoding": "chunked" },
|
||||
{ forSurface: true, injecting: true, keepCsp: false }
|
||||
);
|
||||
expect(out["X-Frame-Options"]).toBeUndefined();
|
||||
expect(out["Content-Security-Policy"]).toBeUndefined();
|
||||
// The body is rewritten, so length and encoding are dropped for recompute.
|
||||
expect(out["content-length"]).toBeUndefined();
|
||||
expect(out["content-encoding"]).toBeUndefined();
|
||||
expect(out.connection).toBeUndefined();
|
||||
expect(out["transfer-encoding"]).toBeUndefined();
|
||||
expect(out["content-type"]).toBe("text/html");
|
||||
});
|
||||
|
||||
it("strips framing headers when injecting inline, too", () => {
|
||||
const out = filterProxyHeaders(upstream, {
|
||||
forSurface: false,
|
||||
injecting: true,
|
||||
keepCsp: false,
|
||||
});
|
||||
expect(out["X-Frame-Options"]).toBeUndefined();
|
||||
expect(out["Content-Security-Policy"]).toBeUndefined();
|
||||
});
|
||||
|
||||
it("preserves multi-valued headers as arrays", () => {
|
||||
const out = filterProxyHeaders(
|
||||
{ "set-cookie": ["a=1", "b=2"] },
|
||||
{ forSurface: true, injecting: false, keepCsp: false }
|
||||
);
|
||||
expect(out["set-cookie"]).toEqual(["a=1", "b=2"]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -45,6 +45,61 @@ const STRIP_ON_INJECT = new Set([
|
||||
"content-encoding",
|
||||
]);
|
||||
|
||||
// Policy, not protocol: framing headers exist to stop *other* origins from
|
||||
// embedding the app, but every surface here is same-origin inside the editor's
|
||||
// own canvas — forwarding them just blanks the frame with no page-level error.
|
||||
// The CSP pair also governs the injected editor itself (the inline config
|
||||
// script, injected styles, the control socket), so it is dropped wholesale
|
||||
// rather than edited; `keepCsp` opts back in for someone iterating on their
|
||||
// own policy.
|
||||
const STRIP_FOR_SURFACE = new Set([
|
||||
"x-frame-options",
|
||||
"content-security-policy",
|
||||
"content-security-policy-report-only",
|
||||
]);
|
||||
|
||||
interface HeaderFilter {
|
||||
forSurface: boolean;
|
||||
injecting: boolean;
|
||||
keepCsp: boolean;
|
||||
}
|
||||
|
||||
function keepsHeader(name: string, opts: HeaderFilter): boolean {
|
||||
if (opts.injecting && STRIP_ON_INJECT.has(name)) {
|
||||
return false;
|
||||
}
|
||||
if (!(opts.injecting || opts.forSurface)) {
|
||||
return true;
|
||||
}
|
||||
if (!STRIP_FOR_SURFACE.has(name)) {
|
||||
return true;
|
||||
}
|
||||
// `keepCsp` restores only the CSP pair: X-Frame-Options has no debugging
|
||||
// value inside the editor — keeping it can only blank the frame.
|
||||
return opts.keepCsp && name !== "x-frame-options";
|
||||
}
|
||||
|
||||
/**
|
||||
* Copy upstream response headers onto the response we serve, dropping what
|
||||
* must not be forwarded: hop-by-hop and length/encoding when we rewrite the
|
||||
* body (`injecting`), and framing/CSP policy on any document served as an
|
||||
* editor surface (`forSurface`) — including responses we could not inject
|
||||
* into, which is what blanks the canvas for dev servers that send
|
||||
* `X-Frame-Options` on compressed or non-UTF-8 HTML.
|
||||
*/
|
||||
export function filterProxyHeaders(
|
||||
headers: http.IncomingHttpHeaders,
|
||||
opts: HeaderFilter
|
||||
): http.OutgoingHttpHeaders {
|
||||
const out: http.OutgoingHttpHeaders = {};
|
||||
for (const [key, value] of Object.entries(headers)) {
|
||||
if (value !== undefined && keepsHeader(key.toLowerCase(), opts)) {
|
||||
out[key] = value;
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
const TUNNEL_TIMEOUT_MS = 60_000;
|
||||
|
||||
/** Not a surface: hand the upstream body back exactly as it came. */
|
||||
@@ -173,6 +228,12 @@ export interface ProxyDeps {
|
||||
* browser by the surface cookie; see `resolveMode`.
|
||||
*/
|
||||
defaultMode: AirshipMode;
|
||||
/**
|
||||
* Keep upstream `Content-Security-Policy` headers on served surfaces
|
||||
* instead of stripping them. Framing protections (`X-Frame-Options`) are
|
||||
* dropped regardless — see `STRIP_FOR_SURFACE`.
|
||||
*/
|
||||
keepCsp?: boolean;
|
||||
onAirshipUpgrade: (
|
||||
req: http.IncomingMessage,
|
||||
socket: Duplex,
|
||||
@@ -205,6 +266,10 @@ function handleHttp(
|
||||
return;
|
||||
}
|
||||
|
||||
// Resolved before the upstream round trip: the passthrough branch needs to
|
||||
// know whether the response is frame-destined even when it cannot inject.
|
||||
const resolved = resolveMode(req, deps.defaultMode);
|
||||
|
||||
const headers = {
|
||||
...req.headers,
|
||||
// Ask for identity so we can inject into HTML reliably.
|
||||
@@ -240,11 +305,16 @@ function handleHttp(
|
||||
status !== 204 &&
|
||||
status !== 304;
|
||||
|
||||
const mode = canInject
|
||||
? resolveMode(req, deps.defaultMode)
|
||||
: "passthrough";
|
||||
const mode = canInject ? resolved : "passthrough";
|
||||
if (mode === "passthrough") {
|
||||
res.writeHead(status, proxyRes.headers);
|
||||
res.writeHead(
|
||||
status,
|
||||
filterProxyHeaders(proxyRes.headers, {
|
||||
forSurface: resolved === "frame",
|
||||
injecting: false,
|
||||
keepCsp: deps.keepCsp ?? false,
|
||||
})
|
||||
);
|
||||
proxyRes.pipe(res);
|
||||
return;
|
||||
}
|
||||
@@ -276,12 +346,11 @@ function handleHttp(
|
||||
pathname: appPathname(req),
|
||||
wsPath: deps.wsPath,
|
||||
});
|
||||
const outHeaders: http.OutgoingHttpHeaders = {};
|
||||
for (const [key, value] of Object.entries(proxyRes.headers)) {
|
||||
if (!STRIP_ON_INJECT.has(key.toLowerCase())) {
|
||||
outHeaders[key] = value;
|
||||
}
|
||||
}
|
||||
const outHeaders = filterProxyHeaders(proxyRes.headers, {
|
||||
forSurface: mode === "frame",
|
||||
injecting: true,
|
||||
keepCsp: deps.keepCsp ?? false,
|
||||
});
|
||||
outHeaders["content-length"] = String(Buffer.byteLength(body));
|
||||
res.writeHead(status, outHeaders);
|
||||
res.end(body);
|
||||
|
||||
Reference in New Issue
Block a user