Skip to content

Commit f653bf5

Browse files
committed
fix(service-print,service-storage): patch round 2 — one render budget, a quiet revoke, re-probe after the refusals, the caller's organization only
Claude-Session: https://claude.ai/code/session_01W2XFCTBfK4LabgqovmvSgK Co-authored-by: Claude <noreply@anthropic.com>
1 parent 19f0174 commit f653bf5

13 files changed

Lines changed: 356 additions & 35 deletions

‎.changeset/22269-print-render-service.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ plugins: [new PrintServicePlugin()]
1818
- **One engine, headless Chromium, in one of two placements.** A local executable (`OS_PRINT_CHROMIUM_PATH`, option `chromiumPath`), or a remote browser's DevTools WebSocket endpoint (`OS_PRINT_BROWSER_ENDPOINT`, option `browserEndpoint`). Either one needs `OS_PRINT_CONSOLE_URL` (option `consoleUrl`): the console's URL as that browser reaches it, `/_console` included. Configuring both placements is refused. `playwright-core` is pinned exact on this package only.
1919
- **Loud at startup.** `start()` probes the engine and logs its placement and Chromium version. An engine that is not configured, is misconfigured or does not answer is reported once, with the setting to fix, and `status()` says why. A remote endpoint is never logged beyond its scheme and host.
2020
- **As the requesting user.** The page is rendered through a short-lived session minted for the caller's own principal by the auth service (`mintPrincipalSession`), and revoked in a `finally`. There is no system principal.
21-
- **Refusals first, each with `code` and `status`, before any session is minted or browser started:** a caller that is not a human user acting for themselves (`PERMISSION_DENIED` / 403); an engine that is not ready (`SERVICE_UNAVAILABLE` / 503); an unknown page (`RESOURCE_NOT_FOUND` / 404); a page that is not the object's default record page or declares no `print` (`VALIDATION_ERROR` / 400); a record the caller cannot read (`RECORD_NOT_FOUND` / 404); an object without `enable: { files: true }` (`FILES_DISABLED` / 403).
22-
- **The archive runs as the requesting user.** `@objectstack/service-storage`'s `file-attachments` service gains a caller-scoped face, `ICallerFileAttachmentService` (exported type), on the same slot and the same writer: `assertMayAttach(parentObject, parentId, context)` throws the insert's own refusal without writing (`PERMISSION_DENIED` / 403 when the caller may not create attachments, `ATTACHMENT_PARENT_ACCESS` / 403 when the attach rule refuses), and `attachFileAs(input, context)` writes one `sys_file` owned by the caller and one `sys_attachment` inserted under the caller's context, so every insert gate judges the caller and the row is stamped `uploaded_by` the caller. The print service asks `assertMayAttach` in its files step, before any session is minted or browser started. `attachFile` (the system verb) is unchanged.
21+
- **Refusals first, each with `code` and `status`, before any session is minted or browser started** (an engine that failed its last probe is re-probed only after them all): a caller that is not a human user acting for themselves (`PERMISSION_DENIED` / 403); an engine that is not ready (`SERVICE_UNAVAILABLE` / 503); an unknown page (`RESOURCE_NOT_FOUND` / 404); a page that is not the object's default record page or declares no `print` (`VALIDATION_ERROR` / 400); a record the caller cannot read (`RECORD_NOT_FOUND` / 404); an object without `enable: { files: true }` (`FILES_DISABLED` / 403).
22+
- **The archive runs as the requesting user.** `@objectstack/service-storage`'s `file-attachments` service gains a caller-scoped face, `ICallerFileAttachmentService` (exported type), on the same slot and the same writer: `assertMayAttach(parentObject, parentId, context)` throws the insert's own refusal without writing (`PERMISSION_DENIED` / 403 when the caller may not create attachments, `ATTACHMENT_PARENT_ACCESS` / 403 when the attach rule refuses), and `attachFileAs(input, context)` writes one `sys_file` owned by the caller and one `sys_attachment` inserted under the caller's context, so every insert gate judges the caller and the row is stamped `uploaded_by` the caller. The print service asks `assertMayAttach` in its files step, before any session is minted or browser started. `attachFileAs` acts only in the caller's own organization: an `organizationId` other than the context's `tenantId` (or `null` when it names none) is refused `PERMISSION_DENIED` / 403. `attachFile` (the system verb) is unchanged.
2323

2424
`@objectstack/spec`'s `llms.txt` counts the new package: the workspace publishes 72 packages.

‎content/docs/deployment/environment-variables.mdx‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,18 @@ read at startup unless noted otherwise. Boolean variables accept `true` / `false
5959

6060
---
6161

62+
## Printing
63+
64+
Read by `@objectstack/service-print` when `PrintServicePlugin` is composed; see [Print Pages](/docs/ui/print-pages). Configure **one** placement plus the console URL; a plugin option of the same purpose wins over its variable.
65+
66+
| Variable | Type | Default | Description |
67+
|:---|:---|:---|:---|
68+
| `OS_PRINT_CHROMIUM_PATH` | path | — | **Local placement**: a Chromium executable on this host, launched per print (plugin option `chromiumPath`). |
69+
| `OS_PRINT_BROWSER_ENDPOINT` | url | — | **Remote placement**: a browser's DevTools WebSocket endpoint, connected per print (option `browserEndpoint`). The minted session cookie travels in its frames, so use `wss://` off this host and `ws://` only on loopback. Never logged beyond its scheme and host. Setting it together with `OS_PRINT_CHROMIUM_PATH` is refused. |
70+
| `OS_PRINT_CONSOLE_URL` | url | — | The console URL that browser loads, `/_console` included (option `consoleUrl`). A URL carrying credentials (`user:password@`) is refused: the browser signs in with the requesting user's own session. |
71+
72+
---
73+
6274
## Secrets & Clustering
6375

6476
| Variable | Type | Default | Description |

‎content/docs/ui/print-pages.mdx‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ one, plus the URL the browser loads the console from:
6464
| Setting | Plugin option | What it is |
6565
|:---|:---|:---|
6666
| `OS_PRINT_CHROMIUM_PATH` | `chromiumPath` | **Local placement**: a Chromium executable on this host, launched per print. |
67-
| `OS_PRINT_BROWSER_ENDPOINT` | `browserEndpoint` | **Remote placement**: a browser's DevTools WebSocket endpoint (`ws://` or `wss://`), connected per print. For hosts that cannot carry a browser of their own. |
68-
| `OS_PRINT_CONSOLE_URL` | `consoleUrl` | The console's URL as that browser reaches it, `/_console` included (`http://127.0.0.1:3000/_console`). |
67+
| `OS_PRINT_BROWSER_ENDPOINT` | `browserEndpoint` | **Remote placement**: a browser's DevTools WebSocket endpoint (`ws://` or `wss://`), connected per print. For hosts that cannot carry a browser of their own. The requesting user's session cookie travels in the endpoint's frames: use `wss://` for a browser off this host, and `ws://` only on loopback. |
68+
| `OS_PRINT_CONSOLE_URL` | `consoleUrl` | The console's URL as that browser reaches it, `/_console` included (`http://127.0.0.1:3000/_console`). A URL carrying credentials (`user:password@`) is refused. |
6969

7070
A plugin option wins over its variable. At startup the plugin probes the engine
7171
— launches the executable or connects to the endpoint — and logs the placement
@@ -97,8 +97,10 @@ const file = await print.printRecordToFile(
9797

9898
The page is rendered as the requesting user — never as the system — through a
9999
short-lived session minted for that user and revoked as soon as the print ends,
100-
whether it succeeded or not. A user can never archive a record they could not
101-
read.
100+
whether it succeeded or not. The PDF is archived as theirs: the attachment is
101+
written under their own permissions, and both the file and the attachment name
102+
them. A user can never archive a record they could not read, or attach a file
103+
where they could not attach one by hand.
102104

103105
## Refusals
104106

@@ -113,7 +115,14 @@ before a session is minted or a browser is started, in this order:
113115
| 3 | The page is not the object's default record page, or declares no `print` | `VALIDATION_ERROR` / 400 |
114116
| 4 | The record does not exist, or the caller cannot read it | `RECORD_NOT_FOUND` / 404 |
115117
| 5 | The object does not declare `enable: { files: true }` | `FILES_DISABLED` / 403 |
118+
| 5 | The caller may not create attachments at all | `PERMISSION_DENIED` / 403 |
119+
| 5 | The caller may not attach a file to this record (attaching needs edit access to it) | `ATTACHMENT_PARENT_ACCESS` / 403 |
120+
121+
An engine that did not answer its last probe is probed once more only after
122+
step 5, so no browser is started before every refusal above has had its say.
116123

117124
After those, a print can still fail as the render or the archive does: the page
118-
not loading in time (`TIMEOUT` / 504), or the archive's own refusals, such as a
119-
file over the upload size limit (`PAYLOAD_TOO_LARGE` / 413).
125+
not rendering within the 30 s budget (`TIMEOUT` / 504), or the archive's own
126+
refusals, such as a file over the upload size limit (`PAYLOAD_TOO_LARGE` / 413).
127+
A failure to end the session afterwards never replaces the print's own outcome;
128+
it is logged, and the session expires on its own within 60 s.

‎packages/services/service-print/src/chromium-driver.ts‎

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -191,25 +191,41 @@ class PlaywrightChromiumDriver implements ChromiumDriver {
191191
const secrets = [this.resolved.target, input.cookie.value];
192192
const started = Date.now();
193193
const remaining = (): number => Math.max(1, input.timeoutMs - (Date.now() - started));
194-
let browser: BrowserHandle | undefined;
195-
let context: ContextHandle | undefined;
196-
try {
197-
browser = await this.open(remaining());
198-
context = await browser.newContext();
199-
await context.addCookies([cookieFor(input.url, input.cookie)]);
200-
const page = await context.newPage();
194+
const handles: { browser?: BrowserHandle; context?: ContextHandle } = {};
195+
// Every step of the render — open, the context, the cookie, the page, the
196+
// load, the ready wait, the fonts and the PDF — runs inside ONE deadline:
197+
// a browser that stalls on any of them answers TIMEOUT within the budget.
198+
const render = (async (): Promise<Buffer> => {
199+
handles.browser = await this.open(remaining());
200+
handles.context = await handles.browser.newContext();
201+
await handles.context.addCookies([cookieFor(input.url, input.cookie)]);
202+
const page = await handles.context.newPage();
201203
await page.goto(input.url, { waitUntil: 'load', timeout: remaining() });
202204
await page.waitForFunction(PRINT_READY_PREDICATE, undefined, { timeout: remaining(), polling: POLL_INTERVAL_MS });
203205
await page.evaluate(FONTS_READY);
204-
return await page.pdf({ preferCSSPageSize: true, printBackground: true });
206+
return page.pdf({ preferCSSPageSize: true, printBackground: true });
207+
})();
208+
// Once the deadline answers, a late rejection of the abandoned render is
209+
// expected (the cleanup below closes what it was waiting on).
210+
render.catch(() => undefined);
211+
let timer: ReturnType<typeof setTimeout> | undefined;
212+
const deadline = new Promise<never>((_, reject) => {
213+
timer = setTimeout(
214+
() => reject(Object.assign(new Error('the render budget ran out'), { name: 'TimeoutError' })),
215+
input.timeoutMs,
216+
);
217+
});
218+
try {
219+
return await Promise.race([render, deadline]);
205220
} catch (err) {
206221
if (isTimeout(err)) {
207222
throw refusalError(
208223
'TIMEOUT',
209224
504,
210-
`The print page did not finish loading within ${Math.round(input.timeoutMs / 1000)} s at ${input.url}: ` +
225+
`The print page did not finish rendering within ${Math.round(input.timeoutMs / 1000)} s at ${input.url}: ` +
211226
'the console never showed a print page for the record (a root carrying data-print-page, with no request ' +
212-
'in flight). Check that the console is served at that URL and reachable from the browser.',
227+
'in flight), or the browser stalled. Check that the console is served at that URL and reachable from the ' +
228+
'browser.',
213229
);
214230
}
215231
throw refusalError(
@@ -218,10 +234,11 @@ class PlaywrightChromiumDriver implements ChromiumDriver {
218234
`The print engine failed at ${this.where()}: ${redact(String((err as Error)?.message ?? err), secrets)}`,
219235
);
220236
} finally {
237+
clearTimeout(timer);
221238
// The context first: on the remote placement it holds the session cookie
222239
// inside a browser this process does not own.
223-
if (context) await context.close().catch(() => undefined);
224-
if (browser) await browser.close().catch(() => undefined);
240+
if (handles.context) await handles.context.close().catch(() => undefined);
241+
if (handles.browser) await handles.browser.close().catch(() => undefined);
225242
}
226243
}
227244
}
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, expect, it, vi } from 'vitest';
4+
import { createChromiumDriver, type ChromiumLauncher } from './chromium-driver.js';
5+
import type { PrintSessionCookie } from './principal-session.js';
6+
7+
/**
8+
* The driver with an injected launcher — no browser, so this file runs on every
9+
* host. What a real Chromium cannot be made to do on demand is staged here: an
10+
* engine error that echoes the session cookie, and a browser that stalls.
11+
*/
12+
13+
const COOKIE: PrintSessionCookie = {
14+
name: 'os.session_token',
15+
value: 'minted-cookie-value-9f2c.signature',
16+
path: '/',
17+
httpOnly: true,
18+
secure: false,
19+
sameSite: 'lax',
20+
};
21+
const URL_ = 'http://127.0.0.1:3000/_console/apps/showcase/invoice/record/inv_1';
22+
const ENDPOINT_TOKEN = 'endpoint-token-7d1a';
23+
24+
/** A step that never settles: a stalled browser. */
25+
const never = <T>(): Promise<T> => new Promise<T>(() => undefined);
26+
27+
interface Stage {
28+
goto?: () => Promise<unknown>;
29+
newContext?: () => Promise<unknown>;
30+
pdf?: () => Promise<Buffer>;
31+
connect?: () => Promise<unknown>;
32+
}
33+
34+
function stagedLauncher(stage: Stage) {
35+
const closed = { context: 0, browser: 0 };
36+
const page = {
37+
goto: vi.fn(stage.goto ?? (async () => undefined)),
38+
waitForFunction: vi.fn(async () => undefined),
39+
evaluate: vi.fn(async () => true),
40+
pdf: vi.fn(stage.pdf ?? (async () => Buffer.from('%PDF-1.4'))),
41+
};
42+
const context = {
43+
addCookies: vi.fn(async () => undefined),
44+
newPage: vi.fn(async () => page),
45+
close: vi.fn(async () => {
46+
closed.context++;
47+
}),
48+
};
49+
const browser = {
50+
version: () => '141.0.0.0',
51+
newContext: vi.fn(stage.newContext ?? (async () => context)),
52+
close: vi.fn(async () => {
53+
closed.browser++;
54+
}),
55+
};
56+
const launcher = {
57+
launch: vi.fn(async () => browser),
58+
connectOverCDP: vi.fn(stage.connect ?? (async () => browser)),
59+
} as unknown as ChromiumLauncher;
60+
return { launcher: async () => launcher, closed, page };
61+
}
62+
63+
async function refusalOf(p: Promise<unknown>): Promise<{ code?: unknown; status?: unknown; message: string }> {
64+
try {
65+
await p;
66+
} catch (err) {
67+
const e = err as { code?: unknown; status?: unknown; message: string };
68+
return { code: e.code, status: e.status, message: e.message };
69+
}
70+
throw new Error('expected a refusal, the call resolved');
71+
}
72+
73+
describe('the driver redacts the session cookie from an engine error', () => {
74+
it('an engine error carrying the cookie value answers INTERNAL_ERROR / 500 with it replaced by [redacted]', async () => {
75+
const { launcher, closed } = stagedLauncher({
76+
goto: async () => {
77+
throw new Error(`net::ERR_ABORTED loading ${URL_} with cookie ${COOKIE.name}=${COOKIE.value}`);
78+
},
79+
});
80+
const driver = createChromiumDriver({ placement: 'local', target: '/usr/bin/chromium', consoleUrl: 'x' }, launcher);
81+
const r = await refusalOf(driver.renderPdf({ url: URL_, cookie: COOKIE, timeoutMs: 5_000 }));
82+
expect([r.code, r.status]).toEqual(['INTERNAL_ERROR', 500]);
83+
expect(r.message).toContain('[redacted]');
84+
expect(r.message).not.toContain(COOKIE.value);
85+
expect(closed).toEqual({ context: 1, browser: 1 });
86+
});
87+
88+
it('a remote endpoint carrying a token is redacted from an engine error, and never named beyond scheme and host', async () => {
89+
const endpoint = `wss://browser.example.com/devtools?token=${ENDPOINT_TOKEN}`;
90+
const { launcher } = stagedLauncher({
91+
connect: async () => {
92+
throw new Error(`WebSocket error: ${endpoint} 403 Forbidden`);
93+
},
94+
});
95+
const driver = createChromiumDriver({ placement: 'remote', target: endpoint, consoleUrl: 'x' }, launcher);
96+
const r = await refusalOf(driver.renderPdf({ url: URL_, cookie: COOKIE, timeoutMs: 5_000 }));
97+
expect([r.code, r.status]).toEqual(['INTERNAL_ERROR', 500]);
98+
expect(r.message).toContain('wss://browser.example.com');
99+
expect(r.message).not.toContain(ENDPOINT_TOKEN);
100+
});
101+
});
102+
103+
describe('one budget bounds the whole render', () => {
104+
for (const [step, stage] of [
105+
['opening a context', { newContext: () => never() }],
106+
['printing the PDF', { pdf: () => never<Buffer>() }],
107+
] as const) {
108+
it(`a browser that stalls ${step} answers TIMEOUT / 504 within the budget, and what was opened is closed`, async () => {
109+
const { launcher, closed } = stagedLauncher(stage);
110+
const driver = createChromiumDriver({ placement: 'remote', target: 'ws://127.0.0.1:9222/x', consoleUrl: 'x' }, launcher);
111+
const started = Date.now();
112+
const r = await refusalOf(driver.renderPdf({ url: URL_, cookie: COOKIE, timeoutMs: 300 }));
113+
const elapsed = Date.now() - started;
114+
expect([r.code, r.status]).toEqual(['TIMEOUT', 504]);
115+
expect(elapsed).toBeGreaterThanOrEqual(290);
116+
expect(elapsed).toBeLessThan(2_000);
117+
expect(closed.browser).toBe(1);
118+
expect(r.message).not.toContain(COOKIE.value);
119+
});
120+
}
121+
});

‎packages/services/service-print/src/config.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,15 @@ describe('resolvePrintPlacement — one engine, two placements', () => {
6767
}
6868
});
6969

70+
it('a console URL carrying credentials is misconfigured, and the credentials are not echoed', () => {
71+
const r = resolvePrintPlacement({ chromiumPath: '/c', consoleUrl: 'http://print:hunter2-secret@127.0.0.1:3000/_console' }, {});
72+
expect(r.state).toBe('misconfigured');
73+
if (r.state === 'misconfigured') {
74+
expect(r.reason).toContain(PRINT_CONSOLE_URL_ENV);
75+
expect(r.reason).not.toContain('hunter2-secret');
76+
}
77+
});
78+
7079
it('a console URL that is not http(s), or carries a query, is misconfigured', () => {
7180
for (const consoleUrl of ['ftp://host/_console', 'not a url', `${CONSOLE}?x=1`]) {
7281
expect(resolvePrintPlacement({ chromiumPath: '/c', consoleUrl }, {}).state).toBe('misconfigured');

‎packages/services/service-print/src/config.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@
2727
* and names the value to fix. The plugin says so once at `start()`, and the
2828
* service refuses every render until it is fixed.
2929
*
30+
* ## Credentials on the wire
31+
*
32+
* The minted session cookie travels to the browser in the DevTools protocol's
33+
* own frames. On a `ws://` endpoint that is cleartext: use `wss://` for a
34+
* remote browser off this host, and `ws://` only on loopback. The console URL
35+
* carries no credentials of its own — one with `user:password@` is refused.
36+
*
3037
* ## The remote endpoint may carry a secret
3138
*
3239
* A hosted browser commonly authenticates the WebSocket by a token in the
@@ -165,6 +172,17 @@ export function resolvePrintPlacement(
165172
} catch {
166173
parsedConsole = null;
167174
}
175+
if (parsedConsole && (parsedConsole.username !== '' || parsedConsole.password !== '')) {
176+
// Credentials in the URL would ride every request the browser makes and
177+
// every line that names the URL; the value is not echoed here either.
178+
return {
179+
state: 'misconfigured',
180+
placement,
181+
reason:
182+
`${PRINT_CONSOLE_URL_ENV} (or consoleUrl) carries credentials (user:password@); the browser signs in with ` +
183+
"the requesting user's own session, so the URL must carry none. Remove them.",
184+
};
185+
}
168186
if (!parsedConsole || (parsedConsole.protocol !== 'http:' && parsedConsole.protocol !== 'https:')) {
169187
return {
170188
state: 'misconfigured',

‎packages/services/service-print/src/print-service-plugin.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ export class PrintServicePlugin implements Plugin {
6969
getEngine: () => optional<PrintEngine>('objectql'),
7070
getAttachments: () => optional<IFileAttachmentService>(FILE_ATTACHMENT_SERVICE),
7171
getMinter: () => principalSessionMinterOf(optional<IAuthService>('auth')),
72+
logger: ctx.logger,
7273
});
7374
ctx.registerService(PRINT_SERVICE, this.service);
7475
}

0 commit comments

Comments
 (0)