From a015428cb8d5ef85fa4b9be7e937b5c8f18e3fbf Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Sat, 15 Aug 2026 16:51:57 -0700 Subject: [PATCH 1/2] fix(vault): cap secret body reads Vault put() already rejects secrets larger than 16 KiB. get() called unbounded object.text(), so a large R2 or S3 object could OOM the worker. Cap envelope reads at 32 KiB (enough for a 16 KiB secret) and apply the same cap to AWS consumeBody. Streamed .body stays uncapped for appliance archives. Signed-off-by: Sebastien Tardif --- aws/s3-bucket.ts | 20 ++++++--- tests/aws/storage.test.ts | 15 +++++++ tests/vault.test.ts | 93 +++++++++++++++++++++++++++++++++++++++ worker/vault.ts | 63 ++++++++++++++++++++++++-- 4 files changed, 182 insertions(+), 9 deletions(-) create mode 100644 tests/vault.test.ts diff --git a/aws/s3-bucket.ts b/aws/s3-bucket.ts index 2d9a814..2fd32ca 100644 --- a/aws/s3-bucket.ts +++ b/aws/s3-bucket.ts @@ -45,6 +45,8 @@ export type S3BucketOptions = { kmsKeyId?: string; }; +const maxConsumedObjectBytes = 32 * 1024; + /** Minimal R2Bucket-compatible adapter used by the portable control plane. */ export class AwsS3Bucket { readonly #client: S3Client; @@ -179,11 +181,19 @@ async function consumeBody(body: ReadableStream): Promise maxConsumedObjectBytes) { + await reader.cancel().catch(() => undefined); + throw new Error("object body exceeds the read limit"); + } + chunks.push(value); + } + } finally { + try { reader.releaseLock(); } catch { /* cancel() already released the lock */ } } const bytes = new Uint8Array(size); let offset = 0; diff --git a/tests/aws/storage.test.ts b/tests/aws/storage.test.ts index 64d9269..ed4a4e3 100644 --- a/tests/aws/storage.test.ts +++ b/tests/aws/storage.test.ts @@ -35,6 +35,21 @@ test("S3 adapter preserves bodies and metadata", async () => { assert.equal(client.lastPut?.ServerSideEncryption, "aws:kms"); }); +test("S3 adapter caps object body reads used by vault get", async () => { + const client = new FakeS3Client(); + const bucket = new AwsS3Bucket(client as unknown as S3Client, "test-bucket"); + const huge = "x".repeat(64 * 1024); + await bucket.put("oauth/conn.json", huge, { + httpMetadata: { contentType: "application/json" }, + }); + const object = await bucket.get("oauth/conn.json"); + assert.ok(object); + await assert.rejects(() => object.text(), /read limit/u); + const streamed = await bucket.get("oauth/conn.json"); + assert.ok(streamed); + assert.equal(await new Response(streamed.body).text(), huge); +}); + test("SQS adapter serializes JSON and audit consumer reports partial failures", async () => { const client = new FakeSqsClient(); const queue = new AwsSqsQueue(client as unknown as SQSClient, "https://sqs.example/test"); diff --git a/tests/vault.test.ts b/tests/vault.test.ts new file mode 100644 index 0000000..bfd4425 --- /dev/null +++ b/tests/vault.test.ts @@ -0,0 +1,93 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { OAuthVault } from "../worker/vault.js"; + +const masterKey = Buffer.alloc(32, 7).toString("base64url"); + +test("vault get rejects oversized envelopes before reading the object body", async () => { + let textCalls = 0; + const bucket = { + async get() { + return { + size: 1024 * 1024, + async text() { + textCalls += 1; + return "x".repeat(1024 * 1024); + }, + }; + }, + async put() {}, + async delete() {}, + }; + const vault = new OAuthVault(bucket as never, masterKey); + await assert.rejects( + () => vault.get("oauth/conn.json", "conn", "principal", "github"), + /envelope is too large/u, + ); + assert.equal(textCalls, 0); +}); + +test("vault get rejects a huge body when object size is missing", async () => { + const payload = "x".repeat(64 * 1024); + const bucket = { + async get() { + return { + size: 0, + async text() { + return payload; + }, + body: new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(payload)); + controller.close(); + }, + }), + }; + }, + async put() {}, + async delete() {}, + }; + const vault = new OAuthVault(bucket as never, masterKey); + await assert.rejects( + () => vault.get("oauth/conn.json", "conn", "principal", "github"), + /envelope is too large/u, + ); +}); + +test("vault put and get round-trip a max-size secret", async () => { + const store = new Map(); + const bucket = { + async get(key: string) { + const value = store.get(key); + if (value === undefined) return null; + const bytes = new TextEncoder().encode(value); + return { + size: bytes.byteLength, + async text() { + return value; + }, + body: new ReadableStream({ + start(controller) { + controller.enqueue(bytes); + controller.close(); + }, + }), + }; + }, + async put(key: string, value: string) { + store.set(key, value); + }, + async delete(key: string) { + store.delete(key); + }, + }; + const vault = new OAuthVault(bucket as never, masterKey); + const secret = "s".repeat(16 * 1024); + const key = await vault.put("conn", "principal", "github", secret); + assert.equal(key, "oauth/conn.json"); + assert.equal(await vault.get(key, "conn", "principal", "github"), secret); + await assert.rejects( + () => vault.put("conn", "principal", "github", `${secret}x`), + /OAuth secret is invalid/u, + ); +}); diff --git a/worker/vault.ts b/worker/vault.ts index 7830571..06f1f10 100644 --- a/worker/vault.ts +++ b/worker/vault.ts @@ -1,17 +1,32 @@ type Envelope = { v: 1; iv: string; ciphertext: string; createdAt: string }; +type VaultObject = { + readonly size: number; + text(): Promise; + readonly body?: ReadableStream; +}; +type VaultBucket = { + get(key: string): Promise; + put(key: string, value: string, options?: { + httpMetadata?: { contentType?: string }; + customMetadata?: Record; + }): Promise; + delete(key: string): Promise; +}; const encoder = new TextEncoder(); +const maxSecretBytes = 16 * 1024; +const maxEnvelopeBytes = 32 * 1024; export class OAuthVault { - readonly #bucket: R2Bucket; + readonly #bucket: VaultBucket; readonly #masterKey: string; - constructor(bucket: R2Bucket, masterKey: string) { + constructor(bucket: VaultBucket, masterKey: string) { this.#bucket = bucket; this.#masterKey = masterKey; } async put(connectionId: string, principalId: string, provider: string, secret: string): Promise { - if (!secret || encoder.encode(secret).byteLength > 16 * 1024) throw new Error("OAuth secret is invalid"); + if (!secret || encoder.encode(secret).byteLength > maxSecretBytes) throw new Error("OAuth secret is invalid"); const key = vaultKey(connectionId); const iv = crypto.getRandomValues(new Uint8Array(12)); const ciphertext = await crypto.subtle.encrypt({ name: "AES-GCM", iv: source(iv), additionalData: source(aad(connectionId, principalId, provider)) }, await this.#key(), source(encoder.encode(secret))); @@ -24,7 +39,8 @@ export class OAuthVault { if (vaultObjectKey !== vaultKey(connectionId)) throw new Error("vault key does not match connection"); const object = await this.#bucket.get(vaultObjectKey); if (!object) throw new Error("OAuth credential is unavailable"); - const envelope = JSON.parse(await object.text()) as Envelope; + if (object.size > maxEnvelopeBytes) throw new Error("OAuth credential envelope is too large"); + const envelope = JSON.parse(await readCappedText(object, maxEnvelopeBytes)) as Envelope; if (envelope.v !== 1) throw new Error("OAuth credential envelope is unsupported"); try { const plain = await crypto.subtle.decrypt({ name: "AES-GCM", iv: source(decode(envelope.iv)), additionalData: source(aad(connectionId, principalId, provider)) }, await this.#key(), source(decode(envelope.ciphertext))); @@ -49,3 +65,42 @@ function decode(value: string): Uint8Array { return new Uint8Array(Buffer.from(value, "base64url")); } function source(value: Uint8Array): ArrayBuffer { return Uint8Array.from(value).buffer; } + +async function readCappedText( + object: { text(): Promise; body?: ReadableStream }, + maxBytes: number, +): Promise { + if (object.body) { + return new TextDecoder().decode(await readCappedBytes(object.body, maxBytes)); + } + const text = await object.text(); + if (encoder.encode(text).byteLength > maxBytes) throw new Error("OAuth credential envelope is too large"); + return text; +} + +async function readCappedBytes(body: ReadableStream, maxBytes: number): Promise { + const chunks: Uint8Array[] = []; + let size = 0; + const reader = body.getReader(); + try { + while (true) { + const { done, value } = await reader.read(); + if (done) break; + size += value.byteLength; + if (size > maxBytes) { + await reader.cancel().catch(() => undefined); + throw new Error("OAuth credential envelope is too large"); + } + chunks.push(value); + } + } finally { + try { reader.releaseLock(); } catch { /* cancel() already released the lock */ } + } + const bytes = new Uint8Array(size); + let offset = 0; + for (const chunk of chunks) { + bytes.set(chunk, offset); + offset += chunk.byteLength; + } + return bytes; +} From 7f670727e2c9cbea559cc71f6148fc817cf69c93 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 15 Aug 2026 19:25:38 -0700 Subject: [PATCH 2/2] fix(vault): keep body cap at vault boundary --- aws/s3-bucket.ts | 20 +++++--------------- tests/aws/storage.test.ts | 20 ++++++++++++-------- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/aws/s3-bucket.ts b/aws/s3-bucket.ts index 2fd32ca..2d9a814 100644 --- a/aws/s3-bucket.ts +++ b/aws/s3-bucket.ts @@ -45,8 +45,6 @@ export type S3BucketOptions = { kmsKeyId?: string; }; -const maxConsumedObjectBytes = 32 * 1024; - /** Minimal R2Bucket-compatible adapter used by the portable control plane. */ export class AwsS3Bucket { readonly #client: S3Client; @@ -181,19 +179,11 @@ async function consumeBody(body: ReadableStream): Promise maxConsumedObjectBytes) { - await reader.cancel().catch(() => undefined); - throw new Error("object body exceeds the read limit"); - } - chunks.push(value); - } - } finally { - try { reader.releaseLock(); } catch { /* cancel() already released the lock */ } + while (true) { + const { done, value } = await reader.read(); + if (done) break; + chunks.push(value); + size += value.byteLength; } const bytes = new Uint8Array(size); let offset = 0; diff --git a/tests/aws/storage.test.ts b/tests/aws/storage.test.ts index ed4a4e3..7aafc62 100644 --- a/tests/aws/storage.test.ts +++ b/tests/aws/storage.test.ts @@ -9,6 +9,7 @@ import { AuditQueuePoller } from "../../aws/audit-poller.js"; import { LocalAssetsFetcher } from "../../aws/local-assets.js"; import { AwsS3Bucket } from "../../aws/s3-bucket.js"; import { archiveAuditBatch, AwsSqsQueue } from "../../aws/sqs-queue.js"; +import { OAuthVault } from "../../worker/vault.js"; test("S3 adapter preserves bodies and metadata", async () => { const client = new FakeS3Client(); @@ -35,19 +36,22 @@ test("S3 adapter preserves bodies and metadata", async () => { assert.equal(client.lastPut?.ServerSideEncryption, "aws:kms"); }); -test("S3 adapter caps object body reads used by vault get", async () => { +test("vault caps S3 credential reads without capping the general bucket adapter", async () => { const client = new FakeS3Client(); const bucket = new AwsS3Bucket(client as unknown as S3Client, "test-bucket"); const huge = "x".repeat(64 * 1024); - await bucket.put("oauth/conn.json", huge, { - httpMetadata: { contentType: "application/json" }, - }); + await bucket.put("oauth/conn.json", huge); + const object = await bucket.get("oauth/conn.json"); assert.ok(object); - await assert.rejects(() => object.text(), /read limit/u); - const streamed = await bucket.get("oauth/conn.json"); - assert.ok(streamed); - assert.equal(await new Response(streamed.body).text(), huge); + assert.equal(await object.text(), huge); + + const masterKey = Buffer.alloc(32, 7).toString("base64url"); + const vault = new OAuthVault(bucket, masterKey); + await assert.rejects( + () => vault.get("oauth/conn.json", "conn", "principal", "github"), + /envelope is too large/u, + ); }); test("SQS adapter serializes JSON and audit consumer reports partial failures", async () => {