From 1ae77a98bc555428b50751518f8d310d187e792d Mon Sep 17 00:00:00 2001 From: wei-hai Date: Wed, 22 Jul 2026 23:46:06 -0700 Subject: [PATCH 1/3] feat: output.getDownloadUrl() + SDK User-Agent identification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a first-class Output.getDownloadUrl() returning { url, expiresAt } — a directly-fetchable URL for an output (signed URL on object-storage backends, content URL on self-hosted; never throws). Also set a User-Agent identifying the SDK on every request, with an optional clientInfo app token. Co-Authored-By: Claude Opus 4.8 --- README.md | 5 ++ src/low/index.ts | 1 + src/low/transport.test.ts | 74 ++++++++++++++++++++++++ src/low/transport.ts | 111 +++++++++++++++++++++++++++++++++++- src/sdk/client.ts | 4 ++ src/sdk/outputs.test.ts | 38 +++++++++++- src/sdk/outputs.ts | 14 +++++ test/support/stub-server.ts | 19 ++++++ 8 files changed, 263 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 8aa595f..7777744 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,11 @@ different host (for example a job's `events`/`cancel` link, or a redirect on an asset download), the key is not sent there — see "Typed errors" below for the exception classes this surface can raise. +The SDK identifies itself via `User-Agent` (for support + usage analytics); +no other data is collected. Pass `clientInfo` to `new Comfy(baseUrl, { ... })` +to append your own app's name to it, for example when attributing traffic +from a Worker built on top of this SDK. + ## Quickstart ```ts diff --git a/src/low/index.ts b/src/low/index.ts index d059da5..d7170fb 100644 --- a/src/low/index.ts +++ b/src/low/index.ts @@ -25,4 +25,5 @@ export { OPERATION_METHODS, type ComfyLowOptions, type RequestOptions, + type AssetContentUrl, } from "./transport.js"; diff --git a/src/low/transport.test.ts b/src/low/transport.test.ts index a5a56e4..9a2374a 100644 --- a/src/low/transport.test.ts +++ b/src/low/transport.test.ts @@ -92,6 +92,56 @@ describe("ComfyLow transport", () => { expect(await response.text()).toBe("234"); }); + // -- getAssetContentUrl ----------------------------------------------- + + it("getAssetContentUrl returns the signed URL + parsed expiresAt when the server redirects (Cloud/object storage)", async () => { + const signedUrl = + "https://storage.googleapis.com/bucket/asset_1?X-Goog-Date=20260722T120000Z&X-Goog-Expires=3600&X-Goog-Signature=deadbeef"; + server.state.contentRedirectLocation = signedUrl; + const result = await low.getAssetContentUrl("asset_1"); + expect(result.url).toBe(signedUrl); + expect(result.expiresAt).toEqual(new Date("2026-07-22T13:00:00.000Z")); + }); + + it("getAssetContentUrl returns this endpoint's own absolute URL with a null expiresAt when served inline (self-hosted)", async () => { + server.state.contentBytes = Buffer.from("inline-bytes"); + const result = await low.getAssetContentUrl("asset_1"); + expect(result.url).toBe(`${server.baseUrl}/api/v2/assets/asset_1/content`); + expect(result.expiresAt).toBeNull(); + }); + + it("getAssetContentUrl returns null expiresAt for a redirect URL with no recognizable signed-URL params", async () => { + server.state.contentRedirectLocation = "https://cdn.example.invalid/asset_1?token=opaque"; + const result = await low.getAssetContentUrl("asset_1"); + expect(result.url).toBe("https://cdn.example.invalid/asset_1?token=opaque"); + expect(result.expiresAt).toBeNull(); + }); + + it("getAssetContentUrl never throws for a real 404 — no, it maps to the usual typed error", async () => { + // Sanity check that a genuine failure still surfaces via the existing + // error mapping rather than being swallowed by the "never throws" cases. + await expect(low.getAssetContentUrl("")).rejects.toBeTruthy(); + }); + + it("getAssetContentUrl does not attach the bearer token to the redirect target (redirect is never followed)", async () => { + const authed = new ComfyLow(server.baseUrl, "top-secret-key"); + const attacker = new StubServer(); + await attacker.start(); + try { + server.state.contentRedirectOrigin = attacker.baseUrl; + const result = await authed.getAssetContentUrl("asset_1"); + // The redirect is handed back, not followed. + expect(result.url.startsWith(attacker.baseUrl)).toBe(true); + // The initial (same-origin) request carried the token... + expect(server.state.lastAuthorizationHeader).toBe("Bearer top-secret-key"); + // ...but since the redirect was never followed, the attacker's server + // was never even contacted, let alone with the bearer token. + expect(attacker.state.lastAuthorizationHeader).toBeNull(); + } finally { + await attacker.stop(); + } + }); + it("postJobs rejects a reused Idempotency-Key (single-use, no replay)", async () => { const key = "idem-key-1"; const first = await low.postJobs({ "1": {} }, { idempotencyKey: key }); @@ -144,6 +194,30 @@ describe("ComfyLow transport", () => { expect(job.status).toBe("canceling"); }); + // -- User-Agent identification --------------------------------------------- + + it("sends a default User-Agent identifying the SDK + node runtime", async () => { + await low.getJob("job_01"); + expect(server.state.lastUserAgentHeader).toMatch( + /^comfy-sdk-typescript\/\d+\.\d+\.\d+ \(node v\d+\.\d+\.\d+\)$/, + ); + }); + + it("appends an app/{clientInfo} token when clientInfo is set, without dropping the base token", async () => { + const withClientInfo = new ComfyLow(server.baseUrl, undefined, { clientInfo: "my-worker" }); + await withClientInfo.getJob("job_01"); + expect(server.state.lastUserAgentHeader).toMatch( + /^comfy-sdk-typescript\/\d+\.\d+\.\d+ \(node v\d+\.\d+\.\d+\) app\/my-worker$/, + ); + }); + + it("does not override a caller-supplied User-Agent header", async () => { + // No public method currently lets a caller pass headers through, so this + // exercises the escape hatch directly. + await low.request("GET", "/jobs/job_01", { headers: { "User-Agent": "custom-agent/1.0" } }); + expect(server.state.lastUserAgentHeader).toBe("custom-agent/1.0"); + }); + // -- per-surface auth (FIX 3) --------------------------------------------- it("a request with no apiKey against a no-auth server sends no Authorization header", async () => { diff --git a/src/low/transport.ts b/src/low/transport.ts index 6253168..57778c7 100644 --- a/src/low/transport.ts +++ b/src/low/transport.ts @@ -33,6 +33,12 @@ import { iterateSse, type RawEvent } from "./sse.js"; const API_PREFIX = "/api/v2"; const DEFAULT_TIMEOUT_MS = 30_000; +// Mirrors `version` in package.json. Hand-kept in sync: this package builds +// with plain `tsc` (no bundler/codegen step to inline it), and importing +// `../../package.json` directly would step outside `tsc`'s configured +// `rootDir`. +const SDK_VERSION = "0.1.0"; + export interface RequestOptions { headers?: Record; json?: unknown; @@ -44,17 +50,71 @@ export interface RequestOptions { * must not time out while idle mid-job). */ timeoutMs?: number | null; + /** Defaults to `"follow"`; `getAssetContentUrl` passes `"manual"` so it can + * read a redirect's `Location` instead of following it. */ + redirect?: "follow" | "manual" | "error"; } export interface ComfyLowOptions { fetch?: typeof fetch; timeoutMs?: number; + /** + * Appended to the default `User-Agent` as `app/{clientInfo}` — lets an + * app built on this SDK attribute its own traffic in request logs. + * Opt-in; omitted by default. + */ + clientInfo?: string; +} + +/** Return shape of {@link ComfyLow.getAssetContentUrl}. */ +export interface AssetContentUrl { + url: string; + expiresAt: Date | null; } function looksLikePath(value: string): boolean { return value.startsWith("http") || value.startsWith("/"); } +function buildUserAgent(clientInfo?: string): string { + const base = `comfy-sdk-typescript/${SDK_VERSION} (node ${process.version})`; + return clientInfo ? `${base} app/${clientInfo}` : base; +} + +const GOOG_DATE_RE = /^(\d{4})(\d{2})(\d{2})T(\d{2})(\d{2})(\d{2})Z$/; + +/** + * Expiry off a GCS V4 signed URL's `X-Goog-Date` (`YYYYMMDDTHHMMSSZ`, UTC) + + * `X-Goog-Expires` (seconds) query params. `null` if either is missing or + * doesn't match — an unrecognized signed-URL flavor degrades to "unknown + * expiry" rather than throwing. + */ +function parseExpiry(url: string): Date | null { + let parsed: URL; + try { + parsed = new URL(url); + } catch { + return null; + } + const dateParam = parsed.searchParams.get("X-Goog-Date"); + const expiresParam = parsed.searchParams.get("X-Goog-Expires"); + if (!dateParam || !expiresParam) return null; + const match = GOOG_DATE_RE.exec(dateParam); + if (!match) return null; + const expiresSeconds = Number.parseInt(expiresParam, 10); + if (Number.isNaN(expiresSeconds)) return null; + const [, year, month, day, hour, minute, second] = match; + const epochMs = Date.UTC( + Number(year), + Number(month) - 1, + Number(day), + Number(hour), + Number(minute), + Number(second), + ); + return new Date(epochMs + expiresSeconds * 1000); +} + function parseRetryAfter(response: Response): number | null { const raw = response.headers.get("Retry-After"); if (raw === null) return null; @@ -68,12 +128,14 @@ export class ComfyLow { private readonly apiKey?: string; private readonly fetchImpl: typeof fetch; private readonly defaultTimeoutMs: number; + private readonly userAgent: string; constructor(baseUrl: string, apiKey?: string, options: ComfyLowOptions = {}) { this.baseUrl = baseUrl.replace(/\/$/, ""); this.apiKey = apiKey; this.fetchImpl = options.fetch ?? fetch; this.defaultTimeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; + this.userAgent = buildUserAgent(options.clientInfo); } private urlFor(path: string): string { @@ -114,6 +176,12 @@ export class ComfyLow { headers.set(key, value); } } + // Identifies this SDK's traffic in request logs (support + adoption + // metrics, no other data collected) — never overrides a caller-supplied + // header. + if (!headers.has("User-Agent")) { + headers.set("User-Agent", this.userAgent); + } return headers; } @@ -141,7 +209,13 @@ export class ComfyLow { body = JSON.stringify(options.json); } const signal = this.resolveSignal(options.signal, options.timeoutMs); - return this.fetchImpl(url, { method, headers, body, signal, redirect: "follow" }); + return this.fetchImpl(url, { + method, + headers, + body, + signal, + redirect: options.redirect ?? "follow", + }); } private async parseOrRaise(response: Response, ok: readonly number[]): Promise { @@ -261,6 +335,41 @@ export class ComfyLow { return response; } + /** + * `GET /api/v2/assets/{id}/content` with the redirect **not** followed — + * hands back a directly-fetchable URL instead of the bytes, so a caller + * (e.g. a serverless/Cloudflare Worker) can pass the URL to a downstream + * consumer without streaming the body through itself. On an object-storage + * backend (Cloud/serverless) the server redirects to a short-lived signed + * URL; that redirect is deliberately not followed (unlike + * {@link getAssetContent}), both so its `Location` can be read and so this + * client's bearer token is never attached to the object-storage host. On a + * self-hosted proxy (which serves the bytes inline, no redirect) this + * returns the endpoint's own absolute URL instead. Works on every backend + * and never throws for either shape — only a genuine failure maps to the + * usual typed error. + */ + async getAssetContentUrl( + assetId: string, + options: { signal?: AbortSignal } = {}, + ): Promise { + const path = `/assets/${encodeURIComponent(assetId)}/content`; + const response = await this.request("GET", path, { + signal: options.signal, + redirect: "manual", + }); + const location = response.headers.get("Location"); + // Node/undici hands back the real 3xx status with a readable `Location` + // for a manual redirect; status 0 covers a browser's opaque redirect. + if (location && (response.status === 0 || (response.status >= 300 && response.status < 400))) { + return { url: location, expiresAt: parseExpiry(location) }; + } + if (response.status === 200 || response.status === 206) { + return { url: this.urlFor(path), expiresAt: null }; + } + return this.parseOrRaise(response, [200, 206]); // always throws here + } + // -- jobs ----------------------------------------------------------------- /** diff --git a/src/sdk/client.ts b/src/sdk/client.ts index b04b958..f475e85 100644 --- a/src/sdk/client.ts +++ b/src/sdk/client.ts @@ -54,6 +54,9 @@ export interface ComfyOptions { apiKey?: string; timeoutMs?: number; fetch?: ComfyLowOptions["fetch"]; + /** Appended to the SDK's default `User-Agent` as `app/{clientInfo}` — lets + * an app built on this SDK attribute its own traffic in request logs. */ + clientInfo?: string; } function guardUiFormat(workflow: Workflow): void { @@ -75,6 +78,7 @@ export class Comfy { this.low = new ComfyLow(baseUrl, options.apiKey, { timeoutMs: options.timeoutMs, fetch: options.fetch, + clientInfo: options.clientInfo, }); this.assets = new AssetFactory(this.low); this.workflows = new WorkflowFactory(); diff --git a/src/sdk/outputs.test.ts b/src/sdk/outputs.test.ts index 407b033..33c5668 100644 --- a/src/sdk/outputs.test.ts +++ b/src/sdk/outputs.test.ts @@ -22,7 +22,7 @@ describe("Output", () => { await server.stop(); }); - function output(): Output { + function outputWithId(id: string): Output { return new Output( { node_id: "13", @@ -30,7 +30,7 @@ describe("Output", () => { type: "image", content_type: "image/png", size_bytes: 10, - id: "asset_out_01", + id, hash: null, url: "http://example.invalid/out", url_expires_at: "2026-07-10T19:20:00Z", @@ -39,6 +39,10 @@ describe("Output", () => { ); } + function output(): Output { + return outputWithId("asset_out_01"); + } + it("exposes the low-level output fields", () => { const out = output(); expect(out.nodeId).toBe("13"); @@ -74,4 +78,34 @@ describe("Output", () => { await rm(dir, { recursive: true, force: true }); } }); + + // -- getDownloadUrl ----------------------------------------------------- + + it("getDownloadUrl returns this output's content URL with a null expiresAt on a self-hosted backend", async () => { + server.state.contentBytes = Buffer.from("bytes"); + const result = await output().getDownloadUrl(); + expect(result.url).toBe(`${server.baseUrl}/api/v2/assets/asset_out_01/content`); + expect(result.expiresAt).toBeNull(); + }); + + it("getDownloadUrl surfaces the signed URL + expiresAt when the backend redirects (Cloud/serverless)", async () => { + const signedUrl = + "https://storage.googleapis.com/bucket/o?X-Goog-Date=20260722T000000Z&X-Goog-Expires=60"; + server.state.contentRedirectLocation = signedUrl; + const result = await output().getDownloadUrl(); + expect(result.url).toBe(signedUrl); + expect(result.expiresAt).toEqual(new Date("2026-07-22T00:01:00.000Z")); + }); + + it("getDownloadUrl resolves each output of a multi-output job to its own distinct URL", async () => { + server.state.contentBytes = Buffer.from("bytes"); + const outA = outputWithId("asset_out_a"); + const outB = outputWithId("asset_out_b"); + const [a, b] = await Promise.all([outA.getDownloadUrl(), outB.getDownloadUrl()]); + expect(a.url).not.toBe(b.url); + expect(a.url).toBe(`${server.baseUrl}/api/v2/assets/asset_out_a/content`); + expect(b.url).toBe(`${server.baseUrl}/api/v2/assets/asset_out_b/content`); + expect(a.expiresAt).toBeNull(); + expect(b.expiresAt).toBeNull(); + }); }); diff --git a/src/sdk/outputs.ts b/src/sdk/outputs.ts index e5e0aed..d3dd3e8 100644 --- a/src/sdk/outputs.ts +++ b/src/sdk/outputs.ts @@ -57,4 +57,18 @@ export class Output { const response = await this.low.getAssetContent(this.model.id, { range: options.range }); return new Uint8Array(await response.arrayBuffer()); } + + /** + * A directly-fetchable URL for this output's bytes — a short-lived, + * self-authorizing bearer credential (readable until `expiresAt`) — so a + * caller (e.g. a serverless/Cloudflare Worker) can hand the URL to a + * downstream consumer instead of streaming the bytes through itself. On an + * object-storage backend (Cloud/serverless) this is a signed URL and + * `expiresAt` is set; on a self-hosted proxy (which serves the bytes + * inline) it's this asset's own content URL and `expiresAt` is `null`. + * Works on every backend and never throws. + */ + async getDownloadUrl(): Promise<{ url: string; expiresAt: Date | null }> { + return this.low.getAssetContentUrl(this.model.id); + } } diff --git a/test/support/stub-server.ts b/test/support/stub-server.ts index fc64639..be04757 100644 --- a/test/support/stub-server.ts +++ b/test/support/stub-server.ts @@ -56,6 +56,14 @@ export interface ServerState { * URL) and check what does/doesn't follow the client there. */ contentRedirectOrigin: string | null; + /** + * When set, `GET /assets/{id}/content` redirects to this exact URL + * (verbatim, including any query string) instead of appending `path` to + * `contentRedirectOrigin` — lets a test simulate a GCS-style signed URL + * complete with `X-Goog-Date`/`X-Goog-Expires` query params. Takes + * precedence over `contentRedirectOrigin` when both are set. + */ + contentRedirectLocation: string | null; /** * Overrides the `hash` field returned by `GET /assets/{id}`. `undefined` * (the default) uses `serverHash` as before; an explicit `null` simulates @@ -88,6 +96,9 @@ export interface ServerState { * `null` if that request carried none — lets a test prove a bearer token * was (or was not) attached/received. */ lastAuthorizationHeader: string | null; + /** The raw `User-Agent` header value of the most recent request, or + * `null` if that request carried none. */ + lastUserAgentHeader: string | null; } function defaultState(): ServerState { @@ -107,6 +118,7 @@ function defaultState(): ServerState { firstReconnectProgress: 0.4, progressValue: 0.5, contentRedirectOrigin: null, + contentRedirectLocation: null, getAssetHashOverride: undefined, hangJobPoll: false, uploadCount: 0, @@ -121,6 +133,7 @@ function defaultState(): ServerState { lastUploadContentLength: null, idempotency: new Map(), lastAuthorizationHeader: null, + lastUserAgentHeader: null, }; } @@ -244,6 +257,7 @@ export class StubServer { const path = url.pathname; const state = this.state; state.lastAuthorizationHeader = (req.headers.authorization as string | undefined) ?? null; + state.lastUserAgentHeader = (req.headers["user-agent"] as string | undefined) ?? null; if (!this.authOk(req)) { await readBody(req); @@ -318,6 +332,11 @@ export class StubServer { } private serveContent(req: IncomingMessage, res: ServerResponse, path: string): void { + if (this.state.contentRedirectLocation) { + res.writeHead(302, { Location: this.state.contentRedirectLocation }); + res.end(); + return; + } if (this.state.contentRedirectOrigin) { res.writeHead(302, { Location: `${this.state.contentRedirectOrigin}${path}` }); res.end(); From 005b499edb2f040e6ad18323b1f110f6cec662d6 Mon Sep 17 00:00:00 2001 From: wei-hai Date: Thu, 23 Jul 2026 08:29:38 -0700 Subject: [PATCH 2/3] harden: reject CR/LF in clientInfo to prevent header injection Co-Authored-By: Claude Opus 4.8 --- src/low/transport.test.ts | 6 ++++++ src/low/transport.ts | 9 ++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/low/transport.test.ts b/src/low/transport.test.ts index 9a2374a..9355832 100644 --- a/src/low/transport.test.ts +++ b/src/low/transport.test.ts @@ -218,6 +218,12 @@ describe("ComfyLow transport", () => { expect(server.state.lastUserAgentHeader).toBe("custom-agent/1.0"); }); + it("rejects a clientInfo containing CR/LF (no header injection)", () => { + for (const bad of ["evil\r\nX-Injected: 1", "line\nbreak", "carriage\rreturn"]) { + expect(() => new ComfyLow(server.baseUrl, undefined, { clientInfo: bad })).toThrow(); + } + }); + // -- per-surface auth (FIX 3) --------------------------------------------- it("a request with no apiKey against a no-auth server sends no Authorization header", async () => { diff --git a/src/low/transport.ts b/src/low/transport.ts index 57778c7..06292b4 100644 --- a/src/low/transport.ts +++ b/src/low/transport.ts @@ -78,7 +78,14 @@ function looksLikePath(value: string): boolean { function buildUserAgent(clientInfo?: string): string { const base = `comfy-sdk-typescript/${SDK_VERSION} (node ${process.version})`; - return clientInfo ? `${base} app/${clientInfo}` : base; + if (!clientInfo) return base; + // A caller-set token goes verbatim into a header value; reject CR/LF so it + // can never split/inject headers (undici would reject it anyway, but fail + // fast with a clear message at construction). + if (/[\r\n]/.test(clientInfo)) { + throw new Error("clientInfo must not contain CR or LF characters"); + } + return `${base} app/${clientInfo}`; } const GOOG_DATE_RE = /^(\d{4})(\d{2})(\d{2})T(\d{2})(\d{2})(\d{2})Z$/; From d4a547c2573ec0c02e755d506476905bcbbdb84a Mon Sep 17 00:00:00 2001 From: wei-hai Date: Thu, 23 Jul 2026 08:41:39 -0700 Subject: [PATCH 3/3] fix: release response body on getAssetContentUrl no-download paths; guard SDK_VERSION drift Co-Authored-By: Claude Opus 4.8 --- src/low/transport.test.ts | 9 +++++++++ src/low/transport.ts | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/src/low/transport.test.ts b/src/low/transport.test.ts index 9355832..4b35335 100644 --- a/src/low/transport.test.ts +++ b/src/low/transport.test.ts @@ -1,3 +1,4 @@ +import { readFileSync } from "node:fs"; import { writeFile, mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -224,6 +225,14 @@ describe("ComfyLow transport", () => { } }); + it("User-Agent version matches package.json (guards SDK_VERSION drift)", async () => { + const pkg = JSON.parse( + readFileSync(new URL("../../package.json", import.meta.url), "utf8"), + ) as { version: string }; + await low.getJob("job_01"); + expect(server.state.lastUserAgentHeader).toContain(`comfy-sdk-typescript/${pkg.version}`); + }); + // -- per-surface auth (FIX 3) --------------------------------------------- it("a request with no apiKey against a no-auth server sends no Authorization header", async () => { diff --git a/src/low/transport.ts b/src/low/transport.ts index 06292b4..f7e3b75 100644 --- a/src/low/transport.ts +++ b/src/low/transport.ts @@ -368,10 +368,14 @@ export class ComfyLow { const location = response.headers.get("Location"); // Node/undici hands back the real 3xx status with a readable `Location` // for a manual redirect; status 0 covers a browser's opaque redirect. + // We only want the URL, not the bytes — release the response body so + // undici can reuse the connection instead of pinning it until GC. if (location && (response.status === 0 || (response.status >= 300 && response.status < 400))) { + await response.body?.cancel(); return { url: location, expiresAt: parseExpiry(location) }; } if (response.status === 200 || response.status === 206) { + await response.body?.cancel(); return { url: this.urlFor(path), expiresAt: null }; } return this.parseOrRaise(response, [200, 206]); // always throws here