diff --git a/.superpowers/sdd/container-task-4-report.md b/.superpowers/sdd/container-task-4-report.md index 3f3df636..3ea43714 100644 --- a/.superpowers/sdd/container-task-4-report.md +++ b/.superpowers/sdd/container-task-4-report.md @@ -55,3 +55,28 @@ Implemented and verified runtime-configured frontend packaging. this task. - `.superpowers/sdd/progress.md` was already modified by the orchestrator and was intentionally excluded from this task's commit. + +## P1 review fixes + +Follow-up commit work addressed both review findings: + +- Runtime configuration is now produced with `jq -cn --arg`, so `BACKEND_BASE_URL` is encoded + by a real JSON serializer rather than interpolated into JavaScript by `sed`. +- The image includes `frontend-config-smoke`, which strips only the fixed assignment wrapper, + parses the remaining JSON with `jq`, requires exactly the `backendBaseUrl` key, and compares + the decoded value to the environment input. +- The hostile smoke passed with quotes, backslashes, a literal newline, ampersand, pipe, and + `"; globalThis.PWNED=true; //` in the value. A breakout would leave non-JSON trailing input + and fail parsing. +- Added `joinBackendPath`, shared by API fetch and EventSource creation. It removes duplicate + boundary slashes for relative and absolute bases while keeping empty and `/` bases rooted. + +Follow-up verification: + +- RED: six join cases failed with `joinBackendPath is not a function` before implementation. +- Targeted: runtime config, API client, and EventSource suites — 14 tests passed. +- Full frontend gate — exit 0 (40 test files, 191 tests, TypeScript, Vite build). +- Rebuilt `thothii-frontend:test` successfully. +- Hostile config image smoke — `frontend runtime config smoke: ok`. +- Rebuilt two-container smoke — default `/api` config, proxied `/api/health`, SPA fallback, + and SSE-safe nginx directives all passed. diff --git a/docker/frontend-entrypoint.sh b/docker/frontend-entrypoint.sh index 0c418cdc..fdc6fbab 100644 --- a/docker/frontend-entrypoint.sh +++ b/docker/frontend-entrypoint.sh @@ -2,8 +2,13 @@ set -eu backend_base_url=${BACKEND_BASE_URL:-/api} -escaped_backend_base_url=$(printf '%s' "$backend_base_url" | sed 's/[&|\\]/\\&/g') -sed "s|__BACKEND_BASE_URL__|${escaped_backend_base_url}|g" \ - /usr/share/nginx/html/config.template.js > /usr/share/nginx/html/config.js +runtime_config=$(jq -cn --arg backend_base_url "$backend_base_url" \ + '{backendBaseUrl: $backend_base_url}') +printf 'window.__THOTHII_CONFIG__ = %s;\n' "$runtime_config" \ + > /usr/share/nginx/html/config.js + +if [ "$#" -gt 0 ]; then + exec "$@" +fi exec nginx -g 'daemon off;' diff --git a/docker/frontend.Dockerfile b/docker/frontend.Dockerfile index a1c5beba..c6ae417d 100644 --- a/docker/frontend.Dockerfile +++ b/docker/frontend.Dockerfile @@ -8,12 +8,12 @@ RUN npm run build FROM nginxinc/nginx-unprivileged:1.27-alpine USER root +RUN apk add --no-cache jq COPY --from=build /src/frontend/dist /usr/share/nginx/html COPY docker/nginx.conf.template /etc/nginx/conf.d/default.conf COPY docker/frontend-entrypoint.sh /usr/local/bin/frontend-entrypoint -RUN mv /usr/share/nginx/html/config.js /usr/share/nginx/html/config.template.js \ - && sed -i 's|{}|{ backendBaseUrl: "__BACKEND_BASE_URL__" }|' /usr/share/nginx/html/config.template.js \ - && chmod 0555 /usr/local/bin/frontend-entrypoint \ +COPY docker/smoke/frontend-smoke.sh /usr/local/bin/frontend-config-smoke +RUN chmod 0555 /usr/local/bin/frontend-entrypoint /usr/local/bin/frontend-config-smoke \ && chown -R 101:101 /usr/share/nginx/html EXPOSE 8080 diff --git a/docker/smoke/frontend-smoke.sh b/docker/smoke/frontend-smoke.sh new file mode 100644 index 00000000..30bf47cb --- /dev/null +++ b/docker/smoke/frontend-smoke.sh @@ -0,0 +1,14 @@ +#!/bin/sh +set -eu + +assignment=$(sed \ + -e 's/^window\.__THOTHII_CONFIG__ = //' \ + -e 's/;$//' \ + /usr/share/nginx/html/config.js) + +printf '%s\n' "$assignment" \ + | jq -e --arg expected "${BACKEND_BASE_URL:-/api}" \ + 'type == "object" and keys == ["backendBaseUrl"] and .backendBaseUrl == $expected' \ + >/dev/null + +printf '%s\n' "frontend runtime config smoke: ok" diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 5fb75b8d..d602ba13 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -1,4 +1,4 @@ -import { backendBaseUrl as BASE } from "./runtime-config"; +import { backendBaseUrl as BASE, joinBackendPath } from "./runtime-config"; export async function apiFetch(path: string, init?: RequestInit): Promise { // Only declare a JSON content-type when we actually send a body. Body-less @@ -10,7 +10,7 @@ export async function apiFetch(path: string, init?: RequestInit): Promise if (init?.body != null && !("content-type" in headers) && !("Content-Type" in headers)) { headers["content-type"] = "application/json"; } - const res = await fetch(`${BASE}${path}`, { ...init, headers }); + const res = await fetch(joinBackendPath(BASE, path), { ...init, headers }); if (!res.ok) throw new Error(`${res.status} ${await res.text().catch(() => "")}`); return res.status === 204 ? (undefined as T) : ((await res.json()) as T); } diff --git a/frontend/src/api/runtime-config.test.ts b/frontend/src/api/runtime-config.test.ts index bede385b..7d6a5fa5 100644 --- a/frontend/src/api/runtime-config.test.ts +++ b/frontend/src/api/runtime-config.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; -import { backendBaseUrl, resolveBackendUrl } from "./runtime-config"; +import { backendBaseUrl, joinBackendPath, resolveBackendUrl } from "./runtime-config"; describe("resolveBackendUrl", () => { it("uses the runtime-injected backend URL", () => { @@ -15,3 +15,16 @@ describe("resolveBackendUrl", () => { expect(backendBaseUrl).toBe(import.meta.env.VITE_BACKEND_URL ?? "http://localhost:8787"); }); }); + +describe("joinBackendPath", () => { + it.each([ + ["", "/sessions/s1", "/sessions/s1"], + ["/", "/sessions/s1", "/sessions/s1"], + ["/api", "/sessions/s1", "/api/sessions/s1"], + ["/api/", "/sessions/s1", "/api/sessions/s1"], + ["https://example.test/api", "/sessions/s1", "https://example.test/api/sessions/s1"], + ["https://example.test/api/", "sessions/s1", "https://example.test/api/sessions/s1"], + ])("joins base %j and path %j", (base, path, expected) => { + expect(joinBackendPath(base, path)).toBe(expected); + }); +}); diff --git a/frontend/src/api/runtime-config.ts b/frontend/src/api/runtime-config.ts index 0f045159..0d0e7706 100644 --- a/frontend/src/api/runtime-config.ts +++ b/frontend/src/api/runtime-config.ts @@ -12,6 +12,12 @@ export function resolveBackendUrl(config: RuntimeConfig | undefined): string { return config?.backendBaseUrl ?? import.meta.env.VITE_BACKEND_URL ?? ""; } +export function joinBackendPath(base: string, path: string): string { + const normalizedBase = base === "/" ? "" : base.replace(/\/+$/, ""); + const normalizedPath = path.replace(/^\/+/, ""); + return `${normalizedBase}/${normalizedPath}`; +} + export const backendBaseUrl = resolveBackendUrl(typeof window === "undefined" ? undefined : window.__THOTHII_CONFIG__) || "http://localhost:8787"; diff --git a/frontend/src/stream/useSessionStream.test.tsx b/frontend/src/stream/useSessionStream.test.tsx index c77d8fcd..a5da11f5 100644 --- a/frontend/src/stream/useSessionStream.test.tsx +++ b/frontend/src/stream/useSessionStream.test.tsx @@ -13,7 +13,7 @@ beforeEach(() => { test("opens an EventSource and feeds NAMED events to the store", () => { renderHook(() => useSessionStream("s1")); const es = FakeEventSource.instances[0]; - expect(es.url).toContain("/sessions/s1/events"); + expect(es.url).toBe("http://localhost:8787/sessions/s1/events"); // Backend sends `event: ui_request` (named) — drive the addEventListener path // that production relies on, not the unnamed onmessage fallback. act(() => diff --git a/frontend/src/stream/useSessionStream.ts b/frontend/src/stream/useSessionStream.ts index 982cdf98..5289cdda 100644 --- a/frontend/src/stream/useSessionStream.ts +++ b/frontend/src/stream/useSessionStream.ts @@ -1,5 +1,6 @@ import { useEffect, useState } from "react"; import { BASE } from "../api/client"; +import { joinBackendPath } from "../api/runtime-config"; import { useSessionStore } from "../store/sessionStore"; import type { StreamEvent } from "../api/types"; @@ -10,7 +11,7 @@ export function useSessionStream(sessionId: string | null) { useEffect(() => { if (!sessionId) return; - const es = new EventSource(`${BASE}/sessions/${sessionId}/events`); + const es = new EventSource(joinBackendPath(BASE, `/sessions/${sessionId}/events`)); es.onopen = () => setConnected(true); es.onerror = () => setConnected(false);