Skip to content

Commit 601224d

Browse files
steipetesteipete
authored
fix: prevent blank embedded views and misleading API responses (#148)
* fix: prevent blank embeds from missing browser assets * test: wait for app readiness before channel reordering * test: wait for readiness across artifact viewer scenarios
1 parent f62c170 commit 601224d

5 files changed

Lines changed: 115 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
## 0.3.0 - Unreleased
44

5+
- Fixed blank embedded views caused by stale browser assets and returned proper JSON errors for unknown API routes while preserving app deep links.
56
- Hardened integration callbacks against server-side requests to non-public networks and enforced guest visibility, moderation, and write limits for topics and registered slash commands. Thanks @jason-allen-oneal.
67
- Updated web font packages and the Cloudflare Wrangler toolchain.
78
- Added coherent conversation organization and attention tools: topic selection and filtered timelines, channel-wide pins with a visible 100-message ceiling, per-channel all/mentions/muted notification preferences, resolved mention highlighting whose current-user emphasis follows that preference, and workspace-visible responding-agent identity beside channel and thread composers. Thanks @PollyBot13 and @jjjhenriksen.

‎apps/api/internal/httpapi/authz_test.go‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1182,6 +1182,74 @@ func TestHTTPServesEmbeddedAsset(t *testing.T) {
11821182
}
11831183
}
11841184

1185+
func TestHTTPNotFoundPreservesAPIAndAssetBoundaries(t *testing.T) {
1186+
t.Parallel()
1187+
handler := New(nil, nil, Options{}).Handler()
1188+
1189+
tests := []struct {
1190+
name string
1191+
method string
1192+
path string
1193+
bearer bool
1194+
wantStatus int
1195+
wantType string
1196+
wantAPIJSON bool
1197+
}{
1198+
{name: "api root", method: http.MethodGet, path: "/api", wantStatus: http.StatusNotFound, wantType: "application/json", wantAPIJSON: true},
1199+
{name: "api root slash", method: http.MethodGet, path: "/api/", wantStatus: http.StatusNotFound, wantType: "application/json", wantAPIJSON: true},
1200+
{name: "missing api get", method: http.MethodGet, path: "/api/does-not-exist", wantStatus: http.StatusNotFound, wantType: "application/json", wantAPIJSON: true},
1201+
{name: "missing api post", method: http.MethodPost, path: "/api/does-not-exist", wantStatus: http.StatusNotFound, wantType: "application/json", wantAPIJSON: true},
1202+
{name: "missing api patch", method: http.MethodPatch, path: "/api/does-not-exist", wantStatus: http.StatusNotFound, wantType: "application/json", wantAPIJSON: true},
1203+
{name: "missing api head", method: http.MethodHead, path: "/api/does-not-exist", wantStatus: http.StatusNotFound, wantType: "application/json"},
1204+
{name: "missing api with bearer", method: http.MethodGet, path: "/api/does-not-exist", bearer: true, wantStatus: http.StatusNotFound, wantType: "application/json", wantAPIJSON: true},
1205+
{name: "missing javascript chunk", method: http.MethodGet, path: "/_app/immutable/chunks/missing.js", wantStatus: http.StatusNotFound, wantType: "text/plain"},
1206+
{name: "missing javascript chunk head", method: http.MethodHead, path: "/_app/immutable/chunks/missing.js", wantStatus: http.StatusNotFound, wantType: "text/plain"},
1207+
{name: "missing stylesheet", method: http.MethodGet, path: "/_app/immutable/assets/missing.css", wantStatus: http.StatusNotFound, wantType: "text/plain"},
1208+
{name: "missing extensionless app asset", method: http.MethodGet, path: "/_app/immutable/missing", wantStatus: http.StatusNotFound, wantType: "text/plain"},
1209+
{name: "missing extensionless asset", method: http.MethodGet, path: "/assets/missing", wantStatus: http.StatusNotFound, wantType: "text/plain"},
1210+
{name: "missing javascript worker", method: http.MethodGet, path: "/service-worker.js", wantStatus: http.StatusNotFound, wantType: "text/plain"},
1211+
{name: "missing javascript module", method: http.MethodGet, path: "/workers/missing.mjs", wantStatus: http.StatusNotFound, wantType: "text/plain"},
1212+
{name: "missing source map", method: http.MethodGet, path: "/scripts/missing.js.map", wantStatus: http.StatusNotFound, wantType: "text/plain"},
1213+
{name: "missing font", method: http.MethodGet, path: "/fonts/missing.woff2", wantStatus: http.StatusNotFound, wantType: "text/plain"},
1214+
{name: "missing icon", method: http.MethodGet, path: "/icons/missing.svg", wantStatus: http.StatusNotFound, wantType: "text/plain"},
1215+
{name: "missing favicon", method: http.MethodGet, path: "/favicon.ico", wantStatus: http.StatusNotFound, wantType: "text/plain"},
1216+
{name: "missing image", method: http.MethodGet, path: "/images/missing.webp", wantStatus: http.StatusNotFound, wantType: "text/plain"},
1217+
{name: "missing web manifest", method: http.MethodGet, path: "/manifest.webmanifest", wantStatus: http.StatusNotFound, wantType: "text/plain"},
1218+
{name: "existing embedded favicon", method: http.MethodGet, path: "/favicon.svg", wantStatus: http.StatusOK, wantType: "image/svg+xml"},
1219+
{name: "api lookalike remains spa", method: http.MethodGet, path: "/apiary/deep-link", wantStatus: http.StatusOK, wantType: "text/html"},
1220+
{name: "app deep link remains spa", method: http.MethodGet, path: "/app/TEXAMPLE/CEXAMPLE", wantStatus: http.StatusOK, wantType: "text/html"},
1221+
{name: "embed deep link remains spa", method: http.MethodGet, path: "/embed/channel/TEXAMPLE/CEXAMPLE", wantStatus: http.StatusOK, wantType: "text/html"},
1222+
}
1223+
for _, tc := range tests {
1224+
t.Run(tc.name, func(t *testing.T) {
1225+
req := httptest.NewRequest(tc.method, tc.path, nil)
1226+
req.Header.Set("Accept", "application/json")
1227+
if tc.bearer {
1228+
req.Header.Set("Authorization", "Bearer invalid")
1229+
}
1230+
response := httptest.NewRecorder()
1231+
handler.ServeHTTP(response, req)
1232+
if response.Code != tc.wantStatus {
1233+
t.Fatalf("status = %d, want %d; content type = %q", response.Code, tc.wantStatus, response.Header().Get("Content-Type"))
1234+
}
1235+
if got := response.Header().Get("Content-Type"); !strings.HasPrefix(got, tc.wantType) {
1236+
t.Fatalf("content type = %q, want prefix %q", got, tc.wantType)
1237+
}
1238+
if tc.wantAPIJSON {
1239+
var body struct {
1240+
Error string `json:"error"`
1241+
}
1242+
if err := json.Unmarshal(response.Body.Bytes(), &body); err != nil {
1243+
t.Fatalf("API response is not JSON: %v", err)
1244+
}
1245+
if body.Error != "route not found" {
1246+
t.Fatalf("API error = %q, want %q", body.Error, "route not found")
1247+
}
1248+
}
1249+
})
1250+
}
1251+
}
1252+
11851253
func TestListenAndServeStopsWithContext(t *testing.T) {
11861254
t.Parallel()
11871255
ctx, cancel := context.WithCancel(context.Background())

‎apps/api/internal/httpapi/server.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"log"
1313
"net/http"
1414
"net/url"
15+
"path"
1516
"strconv"
1617
"strings"
1718
"time"
@@ -1597,6 +1598,11 @@ func (a actor) requireWorkspace(workspaceID string) error {
15971598
}
15981599

15991600
func (s *Server) serveSPA(w http.ResponseWriter, r *http.Request) {
1601+
if r.URL.Path == "/api" || strings.HasPrefix(r.URL.Path, "/api/") {
1602+
writeError(w, http.StatusNotFound, errors.New("route not found"))
1603+
return
1604+
}
1605+
16001606
dist, err := fs.Sub(webassets.Dist, "dist")
16011607
if err != nil {
16021608
writeError(w, http.StatusInternalServerError, err)
@@ -1609,6 +1615,10 @@ func (s *Server) serveSPA(w http.ResponseWriter, r *http.Request) {
16091615
return
16101616
}
16111617
}
1618+
if isMissingBrowserAssetPath(r.URL.Path) {
1619+
http.NotFound(w, r)
1620+
return
1621+
}
16121622
fallback := "index.html"
16131623
if r.URL.Path != "/" {
16141624
if _, err := fs.Stat(dist, "200.html"); err == nil {
@@ -1633,6 +1643,20 @@ func (s *Server) serveSPA(w http.ResponseWriter, r *http.Request) {
16331643
_, _ = w.Write(index)
16341644
}
16351645

1646+
func isMissingBrowserAssetPath(urlPath string) bool {
1647+
if strings.HasPrefix(urlPath, "/_app/") || strings.HasPrefix(urlPath, "/assets/") {
1648+
return true
1649+
}
1650+
switch strings.ToLower(path.Ext(urlPath)) {
1651+
case ".avif", ".css", ".gif", ".ico", ".jpeg", ".jpg", ".js", ".json",
1652+
".map", ".mjs", ".otf", ".png", ".svg", ".ttf", ".wasm", ".webmanifest",
1653+
".webp", ".woff", ".woff2":
1654+
return true
1655+
default:
1656+
return false
1657+
}
1658+
}
1659+
16361660
func (s *Server) injectRuntimeConfig(index []byte) []byte {
16371661
config, err := json.Marshal(map[string]string{"apiBaseUrl": s.publicAPIURL})
16381662
if err != nil {

‎tests/e2e/artifact-viewer.spec.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
PDF_CANVAS_PIXEL_LIMIT,
1414
} from "../../apps/web/src/lib/pdf";
1515
import type { Upload } from "../../apps/web/src/lib/types";
16+
import { waitForAppReady } from "./app-ready";
1617

1718
type Fixture = { filename: string; contentType: string; body: Buffer };
1819
type ZipCompression = 0 | 8;
@@ -686,6 +687,7 @@ test("opens spreadsheets and slide decks with navigation", async ({ page }) => {
686687
},
687688
]);
688689
await page.goto("/app");
690+
await waitForAppReady(page);
689691
await page.getByRole("link", { name: `# ${channel.name}` }).click();
690692
const viewer = page.getByRole("complementary", { name: "Artifact viewer" });
691693
const closeViewer = async () => {
@@ -888,6 +890,7 @@ test("opens safe code, Markdown, PDF, and HTML previews with DOCX download-only"
888890
const { channel } = await seedArtifacts(page, fixtures);
889891

890892
await page.goto("/app");
893+
await waitForAppReady(page);
891894
await page.getByRole("link", { name: `# ${channel.name}` }).click();
892895

893896
await page.getByRole("button", { name: "Open viewer-proof.ts" }).click();
@@ -1017,10 +1020,12 @@ test("falls back to source before structured previews can exhaust the DOM", asyn
10171020
},
10181021
]);
10191022
await page.goto("/app");
1023+
await waitForAppReady(page);
10201024
const channelHref = await page
10211025
.getByRole("link", { name: `# ${channel.name}` })
10221026
.getAttribute("href");
10231027
await page.goto(channelHref!);
1028+
await waitForAppReady(page);
10241029

10251030
await page.getByRole("button", { name: "Open complex.html" }).click();
10261031
let viewer = page.getByRole("complementary", { name: "Artifact viewer" });
@@ -1052,10 +1057,12 @@ test("makes a viewer opened at the mobile breakpoint modal immediately", async (
10521057
},
10531058
]);
10541059
await page.goto("/app");
1060+
await waitForAppReady(page);
10551061
const channelHref = await page
10561062
.getByRole("link", { name: `# ${channel.name}` })
10571063
.getAttribute("href");
10581064
await page.goto(channelHref!);
1065+
await waitForAppReady(page);
10591066
await page.getByRole("button", { name: "Open mobile.md" }).click();
10601067

10611068
const viewer = page.getByRole("dialog", { name: "Artifact viewer" });
@@ -1111,6 +1118,7 @@ test("shows local fallbacks for oversized and malformed artifacts", async ({ pag
11111118
];
11121119
const { channel } = await seedArtifacts(page, fixtures);
11131120
await page.goto("/app");
1121+
await waitForAppReady(page);
11141122
await page.getByRole("link", { name: `# ${channel.name}` }).click();
11151123
const viewer = page.getByRole("complementary", { name: "Artifact viewer" });
11161124

@@ -1193,6 +1201,7 @@ test("near-limit code remains interruptible and falls back to escaped source", a
11931201
},
11941202
]);
11951203
await page.goto("/app");
1204+
await waitForAppReady(page);
11961205
await page.getByRole("link", { name: `# ${channel.name}` }).click();
11971206

11981207
await page.getByRole("button", { name: "Open near-limit.ts" }).click();
@@ -1228,6 +1237,7 @@ test("enforces the actual streamed byte limit instead of trusting upload metadat
12281237
});
12291238
});
12301239
await page.goto("/app");
1240+
await waitForAppReady(page);
12311241
await page.getByRole("link", { name: `# ${channel.name}` }).click();
12321242
await page.getByRole("button", { name: "Open metadata-lie.txt" }).click();
12331243

@@ -1243,6 +1253,7 @@ test("adds an attachment from message.updated without reloading", async ({ page
12431253
});
12441254
const { message } = (await messageResponse.json()) as { message: { id: string } };
12451255
await page.goto("/app");
1256+
await waitForAppReady(page);
12461257
await page.getByRole("link", { name: `# ${channel.name}` }).click();
12471258
await expect(page.getByText("Realtime artifact delivery")).toBeVisible();
12481259

@@ -1274,6 +1285,7 @@ test("returns to the routed thread after closing an artifact", async ({ page })
12741285
},
12751286
]);
12761287
await page.goto("/app");
1288+
await waitForAppReady(page);
12771289
await page.getByRole("link", { name: `# ${channel.name}` }).click();
12781290

12791291
const message = page.locator(`[data-message-id="${messages["thread-proof.md"]}"]`);

‎tests/e2e/sidebar-channel-order.spec.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { expect, test, type Page } from "@playwright/test";
22
import { randomUUID } from "node:crypto";
3+
import { waitForAppReady } from "./app-ready";
34

45
type Workspace = { id: string; route_id: string };
56

@@ -34,6 +35,7 @@ test("channel ordering supports drag, keyboard, touch actions, and collapsed sec
3435
}) => {
3536
const { workspace, names } = await createWorkspaceWithChannels(page, "Channel order");
3637
await page.goto(`/app/${workspace.route_id}`);
38+
await waitForAppReady(page);
3739

3840
await expect.poll(() => visibleChannelNames(page)).toEqual(names);
3941

@@ -43,6 +45,7 @@ test("channel ordering supports drag, keyboard, touch actions, and collapsed sec
4345
await expect.poll(() => visibleChannelNames(page)).toEqual([names[2], names[0], names[1]]);
4446

4547
await page.reload();
48+
await waitForAppReady(page);
4649
await expect.poll(() => visibleChannelNames(page)).toEqual([names[2], names[0], names[1]]);
4750

4851
await page.getByRole("button", { name: `Move #${names[2]}` }).focus();
@@ -76,6 +79,7 @@ test("channel ordering supports drag, keyboard, touch actions, and collapsed sec
7679
});
7780
expect(addedResponse.ok()).toBe(true);
7881
await page.reload();
82+
await waitForAppReady(page);
7983
await expect
8084
.poll(() => visibleChannelNames(page))
8185
.toEqual([names[2], names[0], names[1], addedName]);
@@ -89,6 +93,7 @@ test("channel ordering is isolated by workspace", async ({ page }) => {
8993
const { user } = (await meResponse.json()) as { user: { id: string } };
9094

9195
await page.goto(`/app/${first.workspace.route_id}`);
96+
await waitForAppReady(page);
9297
await page.getByRole("button", { name: `Move #${first.names[0]}` }).click();
9398
await page
9499
.getByRole("menu", { name: `Move #${first.names[0]}` })
@@ -99,13 +104,15 @@ test("channel ordering is isolated by workspace", async ({ page }) => {
99104
.toEqual([first.names[1], first.names[0], first.names[2]]);
100105

101106
await page.goto(`/app/${second.workspace.route_id}`);
107+
await waitForAppReady(page);
102108
await expect.poll(() => visibleChannelNames(page)).toEqual(second.names);
103109
const secondStorageKey = `clickclack:sidebar-channel-order:v1:${user.id}:${second.workspace.id}`;
104110
await expect
105111
.poll(() => page.evaluate((key) => localStorage.getItem(key), secondStorageKey))
106112
.toBeNull();
107113

108114
await page.goto(`/app/${first.workspace.route_id}`);
115+
await waitForAppReady(page);
109116
await expect
110117
.poll(() => visibleChannelNames(page))
111118
.toEqual([first.names[1], first.names[0], first.names[2]]);
@@ -122,6 +129,7 @@ test("invalid saved channel ordering falls back to server order", async ({ page
122129
}, storageKey);
123130

124131
await page.goto(`/app/${workspace.route_id}`);
132+
await waitForAppReady(page);
125133
await expect.poll(() => visibleChannelNames(page)).toEqual(names);
126134
});
127135

@@ -142,6 +150,7 @@ test("unavailable channel order storage keeps session reordering functional", as
142150
});
143151

144152
await page.goto(`/app/${workspace.route_id}`);
153+
await waitForAppReady(page);
145154
await expect.poll(() => visibleChannelNames(page)).toEqual(names);
146155
await page.getByRole("button", { name: `Move #${names[0]}` }).click();
147156
await page
@@ -151,5 +160,6 @@ test("unavailable channel order storage keeps session reordering functional", as
151160
await expect.poll(() => visibleChannelNames(page)).toEqual([names[1], names[0], names[2]]);
152161

153162
await page.reload();
163+
await waitForAppReady(page);
154164
await expect.poll(() => visibleChannelNames(page)).toEqual(names);
155165
});

0 commit comments

Comments
 (0)