diff --git a/__tests__/resolve-payload-fields.test.ts b/__tests__/resolve-payload-fields.test.ts index c70bcd47..31f0f4b0 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, @@ -165,6 +217,35 @@ 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('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 07558392..c1218375 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,18 +52,31 @@ 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. +/** + * 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( @@ -70,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}` @@ -97,18 +120,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) } }