fix: bound the activation marker in bytes and give a click one observer
The marker read added a whole chunk to a running total and compared the total afterwards, so a corrupt body could hand activation a 1 MiB chunk against a 257-byte ceiling. It now reads at most the remaining allowance — through a BYOB reader where the source offers one, and by refusing an oversized chunk before copying it otherwise. A declared oversize cancels the body it refuses instead of leaving the stream open, and the reader lock is released on every path. The build generator and the runtime decoder shared only the extension table, not the path grammar. The generator happily emitted `/assets/bad@name-abcdefgh.js`, which the decoder then refused — a correct build failing at install time. Both now use one exported canonical path predicate and the generator decodes its own output before returning it. The notification click handler emitted its terminal record from inside `process` and again from the `waitUntil` wrapper, so an ordinary click was counted twice. Worse, a late rejection downgraded `MAYBE_APPLIED` to `NOT_APPLIED` — telling operators the click had definitely not been applied when nobody knew that — and the late observation ran outside `waitUntil`, so a worker shutdown lost the evidence. There is one observation authority per click now, certainty is monotone, only an explicit null window confirms `NOT_APPLIED`, and the bounded tail is owned by `waitUntil` without extending the public deadline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
DongHyeonka
co-authored by
Claude Opus 5
Claude Opus 5
parent
39a4a973a8
commit
d7b35cfca3
@@ -916,3 +916,141 @@ describe("service worker static asset install", () => {
|
||||
await controller.stop();
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* SW-01. The marker read must be bounded in bytes, not only in logic. Adding a
|
||||
* whole chunk and *then* comparing the running total meant a corrupt body could
|
||||
* hand activation a 1 MiB chunk against a 257-byte ceiling, and the reader lock
|
||||
* was never released.
|
||||
*/
|
||||
describe("SW-01 the activation marker read is bounded in bytes", () => {
|
||||
function scopeWithMarkerBody(
|
||||
body: ReadableStream<Uint8Array> | null,
|
||||
headers: Readonly<Record<string, string>> = {},
|
||||
) {
|
||||
const markerUrl = "__service-worker-activation-v1__";
|
||||
const response = body
|
||||
? new Response(body, { status: 200, headers })
|
||||
: new Response("", { status: 200, headers });
|
||||
const cache = {
|
||||
match: vi.fn(async (request: RequestInfo | URL) =>
|
||||
String(request).includes(markerUrl) ? response : undefined,
|
||||
),
|
||||
put: vi.fn(async () => {}),
|
||||
delete: vi.fn(async () => true),
|
||||
} as unknown as Cache;
|
||||
return {
|
||||
response,
|
||||
scope: {
|
||||
caches: {
|
||||
open: vi.fn(async () => cache),
|
||||
// The current static cache must exist for its marker to be read.
|
||||
keys: vi.fn(async () => [
|
||||
staticCacheName(identity.staticAssetSetDigest),
|
||||
]),
|
||||
delete: vi.fn(async () => true),
|
||||
match: vi.fn(),
|
||||
},
|
||||
clients: { matchAll: vi.fn(async () => []) },
|
||||
skipWaiting: vi.fn(async () => {}),
|
||||
fetcher: vi.fn(),
|
||||
digest: vi.fn(),
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
const runtimeFor = (scope: unknown) =>
|
||||
createServiceWorkerRuntime(scope as never, {
|
||||
identity,
|
||||
handlers: ["PWA_STATIC_ASSETS"],
|
||||
manifest: {
|
||||
schemaVersion: 1,
|
||||
buildId: identity.buildId,
|
||||
releaseId: identity.releaseId,
|
||||
setDigest: identity.staticAssetSetDigest as `sha256:${string}`,
|
||||
assets: [],
|
||||
},
|
||||
runtimeConfigUrl: "/runtime-config.json",
|
||||
releaseManifestUrl: "/release-manifest.json",
|
||||
});
|
||||
|
||||
it("never retains a single chunk larger than the marker ceiling", async () => {
|
||||
let delivered = 0;
|
||||
let cancels = 0;
|
||||
const oversized = new ReadableStream<Uint8Array>({
|
||||
pull(controller) {
|
||||
delivered += 1;
|
||||
controller.enqueue(new Uint8Array(1_048_576));
|
||||
},
|
||||
cancel() {
|
||||
cancels += 1;
|
||||
},
|
||||
});
|
||||
const fixture = scopeWithMarkerBody(oversized);
|
||||
|
||||
// Activation still completes; the marker is simply not admitted.
|
||||
await expect(runtimeFor(fixture.scope).onActivate()).resolves.toBeTypeOf(
|
||||
"number",
|
||||
);
|
||||
// Every oversized chunk that arrived was refused before being retained,
|
||||
// and the reader that saw it was cancelled. The marker is probed once per
|
||||
// candidate cache, so the counts track each other rather than a constant.
|
||||
expect(cancels).toBeGreaterThanOrEqual(1);
|
||||
expect(delivered).toBeLessThanOrEqual(2);
|
||||
expect(fixture.response.body?.locked).toBe(false);
|
||||
});
|
||||
|
||||
it("bounds a stream that never produces a chunk", async () => {
|
||||
let cancels = 0;
|
||||
const stalled = new ReadableStream<Uint8Array>({
|
||||
pull() {
|
||||
return new Promise<void>(() => {});
|
||||
},
|
||||
cancel() {
|
||||
cancels += 1;
|
||||
},
|
||||
});
|
||||
const fixture = scopeWithMarkerBody(stalled);
|
||||
|
||||
await expect(
|
||||
runtimeFor(fixture.scope).onActivate(),
|
||||
).resolves.toBeTypeOf("number");
|
||||
expect(cancels).toBeGreaterThanOrEqual(1);
|
||||
}, 10_000);
|
||||
|
||||
it("cancels the body it refuses for a declared oversize", async () => {
|
||||
let cancels = 0;
|
||||
const declared = new ReadableStream<Uint8Array>({
|
||||
pull(controller) {
|
||||
controller.enqueue(new Uint8Array(8));
|
||||
},
|
||||
cancel() {
|
||||
cancels += 1;
|
||||
},
|
||||
});
|
||||
const fixture = scopeWithMarkerBody(declared, {
|
||||
"content-length": "1048576",
|
||||
});
|
||||
|
||||
await expect(
|
||||
runtimeFor(fixture.scope).onActivate(),
|
||||
).resolves.toBeTypeOf("number");
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
expect(cancels).toBeGreaterThanOrEqual(1);
|
||||
});
|
||||
|
||||
it("refuses a marker body that is not valid UTF-8", async () => {
|
||||
const invalid = new ReadableStream<Uint8Array>({
|
||||
start(controller) {
|
||||
controller.enqueue(new Uint8Array([0xff, 0xfe, 0xfd]));
|
||||
controller.close();
|
||||
},
|
||||
});
|
||||
const fixture = scopeWithMarkerBody(invalid);
|
||||
|
||||
await expect(
|
||||
runtimeFor(fixture.scope).onActivate(),
|
||||
).resolves.toBeTypeOf("number");
|
||||
expect(fixture.response.body?.locked).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,8 +1,13 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
|
||||
import { mkdir, mkdtemp, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
import {
|
||||
CACHEABLE_ASSET_CONTENT_TYPES,
|
||||
decodeStaticAssetManifest,
|
||||
isCanonicalStaticAssetUrl,
|
||||
} from "../../src/contracts/service-worker-static-manifest.ts";
|
||||
|
||||
/**
|
||||
@@ -67,3 +72,105 @@ describe("SW-RR-03 one authoritative cacheable asset table", () => {
|
||||
expect(decoded.ok).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* SW-02. Sharing only the extension table left the generator and the decoder
|
||||
* with different path grammars: the generator emitted a URL for a directory
|
||||
* containing an `@` or a space, and the decoder then refused the manifest it
|
||||
* had just produced, failing the release build at install time.
|
||||
*/
|
||||
describe("SW-02 the generator and the decoder share one path grammar", () => {
|
||||
async function distWith(
|
||||
files: Readonly<Record<string, string>>,
|
||||
): Promise<string> {
|
||||
const root = await mkdtemp(path.join(tmpdir(), "sw-assets-"));
|
||||
for (const [relative, content] of Object.entries(files)) {
|
||||
const absolute = path.join(root, relative);
|
||||
await mkdir(path.dirname(absolute), { recursive: true });
|
||||
await writeFile(absolute, content);
|
||||
}
|
||||
return root;
|
||||
}
|
||||
|
||||
it("emits a manifest the runtime decoder accepts for every table entry", async () => {
|
||||
const { collectStaticAssets } = await import(
|
||||
"../../scripts/generate-service-worker-assets.ts"
|
||||
);
|
||||
const files: Record<string, string> = {};
|
||||
for (const [index, extension] of Object.keys(
|
||||
CACHEABLE_ASSET_CONTENT_TYPES,
|
||||
).entries()) {
|
||||
files[`assets/name-abcdefg${index}${extension}`] = `content-${index}`;
|
||||
}
|
||||
files["assets/nested/deep/name-abcdefgz.js"] = "nested";
|
||||
const root = await distWith(files);
|
||||
|
||||
const manifest = await collectStaticAssets(root, "build-1", "release-1");
|
||||
|
||||
expect(manifest.assets.length).toBe(
|
||||
Object.keys(CACHEABLE_ASSET_CONTENT_TYPES).length + 1,
|
||||
);
|
||||
expect(decodeStaticAssetManifest(manifest)).toMatchObject({ ok: true });
|
||||
for (const asset of manifest.assets) {
|
||||
expect(isCanonicalStaticAssetUrl(asset.url)).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ label: "an at sign", name: "bad@name-abcdefgh.js" },
|
||||
{ label: "a space", name: "bad name-abcdefgh.js" },
|
||||
{ label: "a percent escape", name: "bad%20name-abcdefgh.js" },
|
||||
{ label: "a hash", name: "bad#name-abcdefgh.js" },
|
||||
])(
|
||||
"stops the build rather than emitting a path the decoder refuses ($label)",
|
||||
async ({ name }) => {
|
||||
const { collectStaticAssets } = await import(
|
||||
"../../scripts/generate-service-worker-assets.ts"
|
||||
);
|
||||
const root = await distWith({
|
||||
"assets/good-abcdefgh.js": "ok",
|
||||
[`assets/${name}`]: "bad",
|
||||
});
|
||||
|
||||
await expect(
|
||||
collectStaticAssets(root, "build-1", "release-1"),
|
||||
).rejects.toThrow(/not canonical/u);
|
||||
},
|
||||
);
|
||||
|
||||
it("agrees with the decoder on the whole path policy table", () => {
|
||||
const cases: readonly (readonly [string, boolean])[] = [
|
||||
["/assets/name-abcdefgh.js", true],
|
||||
["/assets/nested/name-abcdefgh.js", true],
|
||||
["/assets/bad@name-abcdefgh.js", false],
|
||||
["/assets/bad name-abcdefgh.js", false],
|
||||
["/assets/bad%20name-abcdefgh.js", false],
|
||||
["/assets/bad#name-abcdefgh.js", false],
|
||||
["/assets/../name-abcdefgh.js", false],
|
||||
["/assets/./name-abcdefgh.js", false],
|
||||
["assets/name-abcdefgh.js", false],
|
||||
["/assets/\\name-abcdefgh.js", false],
|
||||
["/assets/naïve-abcdefgh.js", false],
|
||||
];
|
||||
for (const [url, canonical] of cases) {
|
||||
expect(isCanonicalStaticAssetUrl(url), url).toBe(canonical);
|
||||
const decoded = decodeStaticAssetManifest({
|
||||
schemaVersion: 1,
|
||||
buildId: "build-1",
|
||||
releaseId: "release-1",
|
||||
setDigest: `sha256:${"a".repeat(64)}`,
|
||||
assets: [
|
||||
{
|
||||
url,
|
||||
sha256: `sha256:${"a".repeat(64)}`,
|
||||
bytes: 4,
|
||||
contentType: "text/javascript",
|
||||
},
|
||||
],
|
||||
});
|
||||
// The decoder still owns the digest check, so a canonical path may fail
|
||||
// for other reasons; a non-canonical one must always fail.
|
||||
if (!canonical) expect(decoded.ok, url).toBe(false);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -621,3 +621,210 @@ describe("Web Push worker runtime", () => {
|
||||
expect(listeners.size).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* WP-01. One click is one terminal record. The adapter emitted the terminal
|
||||
* event from inside `process` *and* again from the `waitUntil` wrapper, so an
|
||||
* ordinary click was counted twice. A late rejection also downgraded the
|
||||
* certainty from `MAYBE_APPLIED` to `NOT_APPLIED`, telling operators the click
|
||||
* had definitely not been applied when nobody knew that, and the late
|
||||
* observation ran outside `waitUntil`, so a worker shutdown lost the evidence.
|
||||
*/
|
||||
describe("WP-01 the click handler has one observation authority", () => {
|
||||
type Observation = Readonly<{
|
||||
event: string;
|
||||
outcome: string;
|
||||
reason?: string;
|
||||
nativeEffect?: string;
|
||||
}>;
|
||||
|
||||
async function clickAdapterWith(
|
||||
clients: Readonly<{
|
||||
matchControlledWindowClients(): Promise<readonly unknown[]>;
|
||||
openWindow(target: string): Promise<unknown>;
|
||||
}>,
|
||||
scheduler?: TimeoutScheduler,
|
||||
) {
|
||||
const store = await activeFence();
|
||||
const observations: Observation[] = [];
|
||||
const adapter = createNotificationClickAdapter({
|
||||
fenceStore: store,
|
||||
registry,
|
||||
origin: "https://app.example.test",
|
||||
now: () => now,
|
||||
clients: clients as never,
|
||||
observer: {
|
||||
record(observation) {
|
||||
observations.push(observation as Observation);
|
||||
},
|
||||
},
|
||||
...(scheduler ? { scheduler } : {}),
|
||||
});
|
||||
return { adapter, observations };
|
||||
}
|
||||
|
||||
const dispatched = (observations: readonly Observation[]) =>
|
||||
observations.filter(
|
||||
(observation) => observation.event === "web_push_click_dispatched",
|
||||
);
|
||||
|
||||
it("records exactly one terminal event for an ordinary focus", async () => {
|
||||
const focus = vi.fn(async () => {});
|
||||
const { adapter, observations } = await clickAdapterWith({
|
||||
async matchControlledWindowClients() {
|
||||
return [
|
||||
{
|
||||
url: "https://app.example.test/current",
|
||||
focus,
|
||||
postMessage() {},
|
||||
},
|
||||
];
|
||||
},
|
||||
openWindow: vi.fn(async () => null),
|
||||
});
|
||||
let waited: Promise<void> | null = null;
|
||||
const result = await adapter.handle({
|
||||
notification: { data: clickData(), close() {} },
|
||||
waitUntil(task) {
|
||||
waited = task;
|
||||
},
|
||||
});
|
||||
await waited;
|
||||
|
||||
expect(result).toEqual({ ok: true, value: undefined });
|
||||
expect(dispatched(observations)).toEqual([
|
||||
{
|
||||
event: "web_push_click_dispatched",
|
||||
outcome: "SUCCEEDED",
|
||||
nativeEffect: "CONFIRMED",
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("confirms NOT_APPLIED only for an explicit null window", async () => {
|
||||
const { adapter, observations } = await clickAdapterWith({
|
||||
async matchControlledWindowClients() {
|
||||
return [];
|
||||
},
|
||||
openWindow: vi.fn(async () => null),
|
||||
});
|
||||
let waited: Promise<void> | null = null;
|
||||
const result = await adapter.handle({
|
||||
notification: { data: clickData(), close() {} },
|
||||
waitUntil(task) {
|
||||
waited = task;
|
||||
},
|
||||
});
|
||||
await waited;
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
expect(dispatched(observations)).toEqual([
|
||||
expect.objectContaining({
|
||||
outcome: "FAILED",
|
||||
nativeEffect: "NOT_APPLIED",
|
||||
}),
|
||||
]);
|
||||
});
|
||||
|
||||
it("keeps MAYBE_APPLIED when the native effect rejects after the deadline", async () => {
|
||||
const clock = manualScheduler();
|
||||
let rejectFocus: ((reason: unknown) => void) | undefined;
|
||||
const { adapter, observations } = await clickAdapterWith(
|
||||
{
|
||||
async matchControlledWindowClients() {
|
||||
return [
|
||||
{
|
||||
url: "https://app.example.test/current",
|
||||
focus: () =>
|
||||
new Promise<void>((_resolve, reject) => {
|
||||
rejectFocus = reject;
|
||||
}),
|
||||
postMessage() {},
|
||||
},
|
||||
];
|
||||
},
|
||||
openWindow: vi.fn(async () => null),
|
||||
},
|
||||
clock.scheduler,
|
||||
);
|
||||
let waited: Promise<void> | null = null;
|
||||
const handling = adapter.handle({
|
||||
notification: { data: clickData(), close() {} },
|
||||
waitUntil(task) {
|
||||
waited = task;
|
||||
},
|
||||
});
|
||||
await vi.waitFor(() => expect(rejectFocus).toBeDefined());
|
||||
clock.expireAll();
|
||||
const result = await handling;
|
||||
expect(result.ok).toBe(false);
|
||||
|
||||
// The effect lands only now, after the terminal result.
|
||||
rejectFocus?.(new Error("focus failed late"));
|
||||
await waited;
|
||||
|
||||
const records = dispatched(observations);
|
||||
expect(records).toHaveLength(2);
|
||||
// The evidence record never claims the click was definitely not applied.
|
||||
expect(records.at(-1)).toEqual({
|
||||
event: "web_push_click_dispatched",
|
||||
outcome: "DEGRADED",
|
||||
reason: "ABORTED",
|
||||
nativeEffect: "MAYBE_APPLIED",
|
||||
});
|
||||
});
|
||||
|
||||
it("waits for the late effect evidence inside waitUntil", async () => {
|
||||
const clock = manualScheduler();
|
||||
let resolveFocus: (() => void) | undefined;
|
||||
const { adapter, observations } = await clickAdapterWith(
|
||||
{
|
||||
async matchControlledWindowClients() {
|
||||
return [
|
||||
{
|
||||
url: "https://app.example.test/current",
|
||||
focus: () =>
|
||||
new Promise<void>((resolve) => {
|
||||
resolveFocus = resolve;
|
||||
}),
|
||||
postMessage() {},
|
||||
},
|
||||
];
|
||||
},
|
||||
openWindow: vi.fn(async () => null),
|
||||
},
|
||||
clock.scheduler,
|
||||
);
|
||||
let waited: Promise<void> | null = null;
|
||||
const handling = adapter.handle({
|
||||
notification: { data: clickData(), close() {} },
|
||||
waitUntil(task) {
|
||||
waited = task;
|
||||
},
|
||||
});
|
||||
await vi.waitFor(() => expect(resolveFocus).toBeDefined());
|
||||
clock.expireAll();
|
||||
await handling;
|
||||
|
||||
let waitUntilSettled = false;
|
||||
const pendingWait = waited as Promise<void> | null;
|
||||
void pendingWait?.then(() => {
|
||||
waitUntilSettled = true;
|
||||
});
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
// The handler's lifetime is still open because the effect has not landed.
|
||||
expect(waitUntilSettled).toBe(false);
|
||||
|
||||
resolveFocus?.();
|
||||
await waited;
|
||||
expect(
|
||||
dispatched(observations).at(-1),
|
||||
).toEqual({
|
||||
event: "web_push_click_dispatched",
|
||||
outcome: "DEGRADED",
|
||||
reason: "ABORTED",
|
||||
nativeEffect: "CONFIRMED",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user