Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/app/s/[id]/download/preparing/PreparingDownload.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { useEffect, useState } from 'react';

import styles from './PreparingDownload.module.scss';

import { pollOperation, type OperationProgress } from '@/lib/search/operationPoll';
import { operationPath, pollOperation, type OperationProgress } from '@/lib/search/operationPoll';
import { artifactHref, pinDownloadedVersion } from '@/lib/search/searchClient';
import { renderedFile } from '@/lib/search/shareDownload';
import type { FileRendering } from '@/lib/search/snapshot';
Expand Down Expand Up @@ -36,7 +36,9 @@ const PreparingDownload = ({
const bundle = preparing?.rendering.bundle;

useEffect(() => {
if (!operationId || !format || bundle === undefined) {
const pollPath = operationPath(operationId);

if (!operationId || !pollPath || !format || bundle === undefined) {
return;
}

Expand All @@ -48,7 +50,7 @@ const PreparingDownload = ({
{
operation_id: operationId,
status: 'pending',
poll: `/api/datasets/operations/${operationId}`,
poll: pollPath,
},
{
signal: controller.signal,
Expand Down
22 changes: 17 additions & 5 deletions src/lib/search/operationPoll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@
*
* This module has no DOM/Node dependency, so `node --test` can import it
* directly. It takes no base URL and reads no env: a caller supplies
* `fetchImpl`, a thin wrapper that resolves the envelope's own relative
* `poll` path against wherever it needs to go (see searchClient.ts's
* `mintDataset`, which polls through the site's own same-origin proxy,
* and snapshot.ts's `captureSnapshot`, which polls the Datasets API directly, since
* a done poll needs no principal header, unlike the `pin` call next to it).
* `fetchImpl`, a thin wrapper that resolves the relative `poll` path
* against wherever it needs to go (see searchClient.ts's `mintDataset`,
* which polls through the site's own same-origin proxy). A server-side
* caller that polls the Datasets API directly (snapshot.ts's
* `captureSnapshot`, shareDownload.ts's `awaitOperation`) ignores that path
* and fetches `operationPath` of the checked operation id on every poll,
* because a path a response names is untrusted.
*/

import type {
Expand All @@ -32,6 +34,16 @@ import { datasetsFetch } from './datasetsFetch.ts';

export type { OperationEnvelope, OperationProgress };

/** The shape of an operation id dfo-apis mints. */
export const OPERATION_SHAPE = /^op_[a-f0-9]+$/;

/** The Datasets API path that reads operation `id`, or undefined when `id`
* is not an operation id. */
export const operationPath = (id: unknown): string | undefined =>
typeof id === 'string' && OPERATION_SHAPE.test(id)
? `/api/datasets/operations/${id}`
: undefined;

export type PollResponse = { ok: boolean; status: number; json: () => Promise<unknown> };
export type PollFetch = (url: string, signal?: AbortSignal) => Promise<PollResponse>;

Expand Down
17 changes: 13 additions & 4 deletions src/lib/search/shareDownload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,12 @@ import type { OperationResource, ProblemDetails } from '@developmentfinance/clie

import { datasetsFetch } from './datasetsFetch.ts';
import { DEFAULT_DOWNLOAD_CHOICES } from './downloadOptions.ts';
import { OPERATION_DEADLINE, pollOperation } from './operationPoll.ts';
import {
OPERATION_DEADLINE,
OPERATION_SHAPE,
operationPath,
pollOperation,
} from './operationPoll.ts';
import { artifactHref, DATASET_FORMATS } from './searchClient.ts';
import type { SearchShare } from './searchShare';
import { MINT_DEFAULT_RENDERING, renderingArtifact, type FileRendering } from './snapshot.ts';
Expand All @@ -60,7 +65,6 @@ type Query = { get(name: string): string | null; has(name: string): boolean };

const FORMAT_IDS: readonly string[] = DATASET_FORMATS.map((format) => format.id);
const PART_SHAPE = /^\d+$/;
export const OPERATION_SHAPE = /^op_[a-f0-9]+$/;

const refused = (status: 400 | 404, message: string): ShareDownload => ({
kind: 'refused',
Expand Down Expand Up @@ -421,8 +425,9 @@ export const serveLiveDownload = async ({

if (res.status === 202) {
const operation = (body as Partial<OperationResource> | null)?.operation;
const pollPath = operationPath(operation?.operation_id);

if (operation && OPERATION_SHAPE.test(operation.operation_id)) {
if (operation && pollPath) {
return html
? {
kind: 'redirect',
Expand All @@ -435,6 +440,7 @@ export const serveLiveDownload = async ({
}
: await awaitOperation({
operation,
pollPath,
target,
base,
pin,
Expand Down Expand Up @@ -473,6 +479,7 @@ export const serveLiveDownload = async ({
* file of the rendering asked for. */
const awaitOperation = async ({
operation,
pollPath,
target,
base,
pin,
Expand All @@ -484,6 +491,8 @@ const awaitOperation = async ({
log,
}: {
operation: OperationResource['operation'];
/** `operationPath` of the operation's checked id. */
pollPath: string;
target: Extract<ShareDownload, { kind: 'live' }>;
base: string;
pin: PinServed;
Expand All @@ -496,7 +505,7 @@ const awaitOperation = async ({
}): Promise<DownloadAnswer> => {
// Every poll reads the operation by its checked id. The poll path a
// response names is ignored.
const pollUrl = `${base}/api/datasets/operations/${operation.operation_id}`;
const pollUrl = `${base}${pollPath}`;
let manifest: VersionLinkDocument | null;

try {
Expand Down
42 changes: 28 additions & 14 deletions src/lib/search/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import type {
} from '@developmentfinance/client/wire';

import { datasetsFetch } from './datasetsFetch.ts';
import { pollOperation } from './operationPoll.ts';
import { operationPath, pollOperation } from './operationPoll.ts';
import { SPEC_SHAPE, VERSION_SHAPE } from './versionLink.ts';

/** The fields of a Datasets manifest this module reads. */
Expand Down Expand Up @@ -206,19 +206,33 @@ export const captureSnapshot = async ({
}

// An over-budget mint answers 202 with an operation envelope; a done
// poll needs no principal.
const manifest = (
minted.status === 202
? await pollOperation(minted.body.operation, {
fetchImpl: (url) =>
fetchImpl(`${base}${url}`, {
cache: 'no-store',
signal: AbortSignal.timeout(DOOR_TIMEOUT_MS),
}),
deadlineMs: pollDeadlineMs,
})
: minted.body
) as (Partial<SnapshotManifest> & Partial<Pick<ProblemDetails, 'code'>>) | null;
// poll needs no principal. Every poll reads the operation by its
// checked id, and the poll path a response names is ignored.
let manifest = minted.body as
(Partial<SnapshotManifest> & Partial<Pick<ProblemDetails, 'code'>>) | null;

if (minted.status === 202) {
const operation = minted.body?.operation;
const pollPath = operationPath(operation?.operation_id);

if (!pollPath) {
throw new Error(`mint ${minted.status}`, {
cause: new Error('the mint named no valid operation id'),
});
}

manifest = await pollOperation(
{ ...operation, poll: pollPath },
{
fetchImpl: () =>
fetchImpl(`${base}${pollPath}`, {
cache: 'no-store',
signal: AbortSignal.timeout(DOOR_TIMEOUT_MS),
}),
deadlineMs: pollDeadlineMs,
},
);
}

if (typeof manifest?.content_version !== 'string') {
throw new Error(manifest?.code ?? `mint ${minted.status}`);
Expand Down
86 changes: 86 additions & 0 deletions src/lib/search/test/snapshot.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,92 @@ test('captureSnapshot fails before minting when no principal is configured', asy
assert.equal(calls.length, 0);
});

// A Datasets API double whose mint answers 202 for `operationId` and whose
// every envelope names a hostile poll path: another surface, a `..`
// segment, then an absolute URL. The operation's own path answers pending
// twice, then done. Records every URL requested.
const asyncMint = (operationId: unknown) => {
const hostile = [
'/api/graph/x',
'/api/datasets/operations/../../presentation/shares/x',
'https://evil.example/api/datasets/operations/op_1f',
];
const urls: string[] = [];
let polls = 0;
const fetchImpl = (async (url: string) => {
urls.push(url);
const answer = (status: number, body: unknown) =>
({ ok: status < 300, status, json: async () => body }) as Response;
const envelope = (poll: string) => ({
operation_id: operationId,
status: 'pending',
poll,
retry_after_ms: 0,
});

if (url === 'https://api/api/datasets') {
return answer(202, { operation: envelope(hostile[0]!) });
}

if (url === 'https://api/api/datasets/operations/op_1f') {
polls += 1;

return polls < hostile.length
? answer(200, { operation: envelope(hostile[polls]!) })
: answer(200, {
operation: { operation_id: operationId, status: 'done' },
manifest: {
content_version: 'cv_abc',
definition: { spec_id: 'spec_crs' },
artifacts,
},
});
}

return url.endsWith('/pin') ? answer(200, {}) : answer(404, { code: 'not_found' });
}) as typeof fetch;

return { urls, fetchImpl };
};

// Slow (about 750 ms), because pollOperation floors each retry_after_ms at 250 ms.
test('captureSnapshot polls the operation by its id and never the poll path a response names', async () => {
const { urls, fetchImpl } = asyncMint('op_1f');

assert.equal(
(
await captureSnapshot({
base: 'https://api',
specId: 'spec_crs',
definition: { spec_id: 'spec_crs' },
principal: 'site',
pollDeadlineMs: 5_000,
fetchImpl,
})
).artifactDigest,
'sha256:csv',
);
assert.deepEqual(urls, [
'https://api/api/datasets',
...Array(3).fill('https://api/api/datasets/operations/op_1f'),
'https://api/api/datasets/spec_crs/cv_abc/pin',
]);
});

for (const [name, operationId] of [
['a path in the id', 'op_1f/../../graph'],
['upper-case hex', 'OP_1F'],
['a number', 42],
['no id', undefined],
] as const) {
test(`captureSnapshot refuses an operation with ${name} before any poll`, async () => {
const { urls, fetchImpl } = asyncMint(operationId);

await assert.rejects(capture(fetchImpl, 'site'), { message: 'mint 202' });
assert.deepEqual(urls, ['https://api/api/datasets']);
});
}

const part = (specId: string, status = 'definite') => ({
status,
...(status === 'definite' && { spec: { spec_id: specId } }),
Expand Down
Loading