From fabb5e0ea205ec705e7de4c813d754a8ad232e67 Mon Sep 17 00:00:00 2001 From: Yeela Date: Thu, 13 Aug 2026 14:21:00 +0300 Subject: [PATCH 1/3] fix: read the wrapped compressed-payload envelope The GitHub trigger is wrapping compressed payloads in a double-encoded envelope so that run-name - evaluated before any step exists, and so unreachable from this action - still parses instead of failing the whole workflow at startup. We detected compression by sniffing gzip magic bytes on the raw input, so the wrapped form missed the sniff, fell through to the raw branch and treated the envelope itself as the payload. That failed silently: empty github_token (checkout falls back to github.token), empty url (git remote add upstream '' fails) and has_cm_repo=false, so the cm repo is never checked out and no rules are evaluated. Exit code 0 throughout. Resolve by parsing first and switching on the value of `type`. Both compressed forms are permanent, not a migration step - Bitbucket has no run-name and keeps sending the bare form, so neither branch can be retired. `type` is matched by value rather than presence because a raw payload can carry its own `type`: Bitbucket builds it from the webhook context. Such a payload must fall through to the raw branch. A compressed-payload envelope whose data is not gzip now fails loudly rather than silently resolving to empty fields. Co-Authored-By: Claude Opus 5 (1M context) --- __tests__/resolve-payload-fields.test.ts | 52 ++++++++++++++++++++++++ scripts/resolve-payload-fields.js | 42 ++++++++++++++----- 2 files changed, 84 insertions(+), 10 deletions(-) diff --git a/__tests__/resolve-payload-fields.test.ts b/__tests__/resolve-payload-fields.test.ts index c70bcd47..3b1fec37 100644 --- a/__tests__/resolve-payload-fields.test.ts +++ b/__tests__/resolve-payload-fields.test.ts @@ -111,6 +111,44 @@ describe('run', () => { ) }) + it('inflates a wrapped compressed-payload envelope', async () => { + const envelope = { + type: 'compressed-payload', + data: gzipSync(JSON.stringify(payload)).toString('base64'), + pullRequestNumber: 123 + } + const core = await runWith(JSON.stringify(JSON.stringify(envelope))) + + expect(core.setFailed).not.toHaveBeenCalled() + expect(core.info).toHaveBeenCalledWith( + 'client_payload mode=compressed-envelope' + ) + expect(outputsOf(core).cm_repository).toBe('acme/cm-repo') + }) + + it('fails loudly when a compressed-payload envelope has no gzip data', async () => { + const core = await runWith( + JSON.stringify( + JSON.stringify({ type: 'compressed-payload', data: 'not-gzip' }) + ) + ) + + expect(core.setFailed).toHaveBeenCalledWith( + expect.stringContaining('carries no gzip data') + ) + }) + + it('treats a raw payload carrying its own type as a raw payload', async () => { + // Bitbucket builds the raw payload from the webhook context, which can + // carry an unrelated `type`. Only the two known values are envelopes. + const core = await runWith(JSON.stringify({ ...payload, type: 'push' })) + + expect(core.setFailed).not.toHaveBeenCalled() + expect(core.info).toHaveBeenCalledWith('client_payload mode=plain') + expect(outputsOf(core).github_token).toBe('ghs_token') + expect(outputsOf(core).cm_repository).toBe('acme/cm-repo') + }) + it('fails on a payload that is not valid JSON', async () => { const core = await runWith('not json') @@ -153,6 +191,20 @@ describe('run with an oversized-payload reference', () => { expect(outputsOf(core).cm_repository).toBe('acme/cm-repo') }) + it('fetches from a double-encoded reference envelope', async () => { + const fetchMock = mockFetch({ + ok: true, + text: async () => JSON.stringify(payload) + }) + + const core = await runWith(JSON.stringify(JSON.stringify(reference))) + + expect(core.setFailed).not.toHaveBeenCalled() + expect(core.info).toHaveBeenCalledWith('client_payload mode=reference') + expect(fetchMock).toHaveBeenCalled() + expect(outputsOf(core).cm_repository).toBe('acme/cm-repo') + }) + it('inflates a stashed payload that is gzipped', async () => { mockFetch({ ok: true, diff --git a/scripts/resolve-payload-fields.js b/scripts/resolve-payload-fields.js index 07558392..3d22453c 100644 --- a/scripts/resolve-payload-fields.js +++ b/scripts/resolve-payload-fields.js @@ -14,6 +14,7 @@ const { gunzipSync } = require('zlib') const OVERSIZED_PAYLOAD_REFERENCE = 'oversized-payload-reference' +const COMPRESSED_PAYLOAD = 'compressed-payload' const PAYLOAD_FETCH_TIMEOUT_MS = 10000 // 32MB @@ -51,15 +52,16 @@ function parsePayload(value) { } /** - * @returns {object | null} the stash reference, or null for a regular payload + * @returns {object | null} the parsed value, or null when `raw` is not JSON at + * all - the bare base64(gzip) form, which has no envelope around it */ -function readStashReference(raw) { - // Cheap pre-check so a regular payload is only parsed once, further down. - if (!raw.includes(OVERSIZED_PAYLOAD_REFERENCE)) { +function tryParsePayload(raw) { + try { + const parsed = parsePayload(raw) + return parsed && typeof parsed === 'object' ? parsed : null + } catch { return null } - const parsed = parsePayload(raw) - return parsed && parsed.type === OVERSIZED_PAYLOAD_REFERENCE ? parsed : null } // Builds the stash URL on the resolver's own origin. @@ -97,18 +99,38 @@ async function fetchStashedPayload(reference, resolverUrl, core) { } /** + * Resolves whichever shape the trigger sent. Both compressed forms are + * permanent, not a migration step: GitHub wraps the payload in an envelope so + * that `run-name`, which is evaluated before any step exists and so cannot be + * rescued from here, still parses. Bitbucket has no `run-name` and keeps + * sending the bare form. + * * @returns {Promise<{ mode: string, payload: object }>} */ async function resolvePayload(raw, resolverUrl, core) { - const reference = readStashReference(raw) - if (reference) { - const payload = await fetchStashedPayload(reference, resolverUrl, core) - return { mode: 'reference', payload } + const parsed = tryParsePayload(raw) + if (parsed) { + // Switch on the *value* of `type`, never its presence: a raw payload may + // legitimately carry its own `type` (Bitbucket builds it from the webhook + // context), and must fall through to the raw branch below. + if (parsed.type === OVERSIZED_PAYLOAD_REFERENCE) { + const payload = await fetchStashedPayload(parsed, resolverUrl, core) + return { mode: 'reference', payload } + } + if (parsed.type === COMPRESSED_PAYLOAD) { + const inflated = inflateIfGzipped(parsed.data || '') + if (inflated === null) { + throw new Error(`${COMPRESSED_PAYLOAD} envelope carries no gzip data`) + } + return { mode: 'compressed-envelope', payload: parsePayload(inflated) } + } + return { mode: 'plain', payload: parsed } } const inflated = inflateIfGzipped(raw) if (inflated !== null) { return { mode: 'compressed', payload: parsePayload(inflated) } } + // Not JSON and not gzip - let the JSON error describe what arrived. return { mode: 'plain', payload: parsePayload(raw) } } From 8ecf4e37dbd6ef5fe279a0c70084bb134c2d378d Mon Sep 17 00:00:00 2001 From: Yeela Date: Thu, 13 Aug 2026 14:27:02 +0300 Subject: [PATCH 2/3] test: pin loud failure when the stash returns an unknown form The stash holds the payload rather than the envelope, and its form depends on whether compression won: bare base64(gzip) if it did, raw JSON if not. Both were already covered; this pins the third case, so a stash body that is neither can never start falling through to empty fields. Co-Authored-By: Claude Opus 5 (1M context) --- __tests__/resolve-payload-fields.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/__tests__/resolve-payload-fields.test.ts b/__tests__/resolve-payload-fields.test.ts index 3b1fec37..b7868cfd 100644 --- a/__tests__/resolve-payload-fields.test.ts +++ b/__tests__/resolve-payload-fields.test.ts @@ -217,6 +217,19 @@ describe('run with an oversized-payload reference', () => { expect(outputsOf(core).cm_repo_ref).toBe('main') }) + it('fails loudly when the stash returns neither gzip nor JSON', async () => { + // The stash holds the payload, not the envelope, and its form depends on + // whether compression won: bare base64(gzip) if it did, raw JSON if not. + // Anything else must be an error rather than a fall-through. + mockFetch({ ok: true, text: async () => 'not-json-not-gzip' }) + + const core = await runWith(JSON.stringify(reference)) + + expect(core.setFailed).toHaveBeenCalledWith( + expect.stringContaining('Failed resolving client payload') + ) + }) + it('refuses an origin other than the resolver', async () => { const fetchMock = mockFetch({ ok: true, text: async () => '{}' }) From e8cbf69e74e5675239b576014523fcad3ef06c62 Mon Sep 17 00:00:00 2001 From: Yeela Date: Thu, 13 Aug 2026 14:29:37 +0300 Subject: [PATCH 3/3] fix: name the offending URL when payloadUrl is not absolute A relative payloadUrl failed with a bare "TypeError: Invalid URL", which tells whoever is on call nothing. Fail with the URL and the likely cause instead. The trigger guarantees an absolute URL - payloadUrl and resolver_url are built from the same public API base, and resolver_url ships on every dispatch, so an empty base would take out result reporting for every run long before a stashed payload could expose it. This stays a diagnostic, not relative-URL support: accepting relative URLs would add resolution leniency for a state that cannot be reached quietly. Also document that the host in payloadUrl is discarded always, not only when it disagrees with the resolver, so the immunity to redirection through that field is not later "fixed" away. Co-Authored-By: Claude Opus 5 (1M context) --- __tests__/resolve-payload-fields.test.ts | 16 +++++++++++++++ scripts/resolve-payload-fields.js | 25 ++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/__tests__/resolve-payload-fields.test.ts b/__tests__/resolve-payload-fields.test.ts index b7868cfd..31f0f4b0 100644 --- a/__tests__/resolve-payload-fields.test.ts +++ b/__tests__/resolve-payload-fields.test.ts @@ -230,6 +230,22 @@ describe('run with an oversized-payload reference', () => { ) }) + it('names the offending URL when payloadUrl is not absolute', async () => { + const fetchMock = mockFetch({ ok: true, text: async () => '{}' }) + + const core = await runWith( + JSON.stringify({ + ...reference, + payloadUrl: '/api/v1/gitstream/payload/k' + }) + ) + + expect(fetchMock).not.toHaveBeenCalled() + expect(core.setFailed).toHaveBeenCalledWith( + expect.stringContaining('stashed payload URL is not absolute') + ) + }) + it('refuses an origin other than the resolver', async () => { const fetchMock = mockFetch({ ok: true, text: async () => '{}' }) diff --git a/scripts/resolve-payload-fields.js b/scripts/resolve-payload-fields.js index 3d22453c..c1218375 100644 --- a/scripts/resolve-payload-fields.js +++ b/scripts/resolve-payload-fields.js @@ -64,7 +64,19 @@ function tryParsePayload(raw) { } } -// Builds the stash URL on the resolver's own origin. +/** + * Builds the stash URL on the resolver's own origin. + * + * The host in `payloadUrl` is decorative and is discarded - always, not only + * when it disagrees. Only the path and query are carried over, re-attached to + * resolver_url, which comes from the workflow rather than the payload. That + * makes this structurally immune to being redirected through this field, so + * please do not "fix" it later by honouring the payload's host. + * + * The path is applied via the `pathname` setter rather than by resolving it as + * a relative URL: relative resolution would let a `//host/...` path escape to + * another origin. + */ function stashUrl(payloadUrl, resolverUrl) { if (!resolverUrl) { throw new Error( @@ -72,7 +84,16 @@ function stashUrl(payloadUrl, resolverUrl) { ) } const resolverOrigin = new URL(resolverUrl).origin - const requested = new URL(payloadUrl) + let requested + try { + // The trigger always sends an absolute URL; both it and resolver_url are + // built from the same base, so a relative one means that base was empty. + requested = new URL(payloadUrl) + } catch { + throw new Error( + `stashed payload URL is not absolute: ${payloadUrl} - the resolver's public API base is probably unset` + ) + } if (requested.origin !== resolverOrigin) { throw new Error( `refusing to fetch stashed payload from ${requested.origin}; expected ${resolverOrigin}`