From fe8bd7184d119f7d8c2d6312d34d070f8fe7da70 Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Wed, 11 Nov 2026 20:21:44 -0500 Subject: [PATCH 1/2] fix(github): hide every request of a redacted target, and read an empty repository as unproven A redacted target's debug trace printed any request whose path named no repository: the owner probe GET /orgs/ the teams and custom_properties sections send, and the app lookup GET /apps/ the branches section sends. The trace collapsed a path only when it found a masked /repos// slug in it. The run now says which requests serve a redacted target. A redacted target's channel hands the engine the client's redacted view (redactedClient), which marks every request redactTrace, and GitHubApi seals the whole trace while a marked request is in flight, so the request-log plugin's lines collapse with the client's own. The four places a flow hands a target's client to the engine (single, multi, snapshot-file, snapshot-dir) go through the channel. The slug-keyed hold and its refusal of a slug-free path are gone. An empty repository answers its ref read 409 "Git Repository is empty.", which getRepoFile read as a failure and told the operator to re-run. It is now the inconclusive proof the file already had for a denied grant, which names the uninitialized branch. The throttle callback printed "retry N/2 after Ns" before deciding, so the line announced a retry for a wait over the 60 s cap and for the attempt after the last retry. It now writes the line where it decides: a retry, or giving up and why. --- src/flows/multi.ts | 3 +- src/flows/redact.ts | 6 + src/flows/single.ts | 2 +- src/flows/snapshot.ts | 4 +- src/github/api.ts | 116 +++++++---- src/github/repo-file.ts | 9 +- src/github/trace-redaction.ts | 62 +++--- test/github/api.test.ts | 225 ++++++++++++++++++---- test/github/redacted-target-trace.test.ts | 160 +++++++++++++++ test/github/repo-file.test.ts | 17 +- test/github/stub.ts | 7 + 11 files changed, 504 insertions(+), 107 deletions(-) create mode 100644 test/github/redacted-target-trace.test.ts diff --git a/src/flows/multi.ts b/src/flows/multi.ts index 42db6dfb7..209dacab8 100644 --- a/src/flows/multi.ts +++ b/src/flows/multi.ts @@ -98,7 +98,8 @@ async function processTarget(ctx: { injectMarker: boolean; channel: TargetChannel; }): Promise { - const { api, target, defaults, cfg, injectMarker, channel } = ctx; + const { target, defaults, cfg, injectMarker, channel } = ctx; + const api = channel.client(ctx.api); const fail = (richMessage: string): TargetResult => targetFailure(channel.io, richMessage); // Injection needs the typed labels, so it follows validation; the injected document is validated again inside. diff --git a/src/flows/redact.ts b/src/flows/redact.ts index d2c50c886..b2bd9aa46 100644 --- a/src/flows/redact.ts +++ b/src/flows/redact.ts @@ -8,6 +8,7 @@ import type { Target } from "../discovery/targets.js"; import type { RunOutcome } from "../engine/outcome.js"; +import { type GitHubClient, redactedClient } from "../github/api.js"; import type { RepoVisibility } from "../github/repo-visibility.js"; import { type SlugKey, slugKey } from "../github/slug.js"; import { type CollectedLine, type Io, prefixedIo } from "../io.js"; @@ -243,6 +244,9 @@ export interface TargetChannel { /** Sink for lines that already name their source (validation warnings): unprefixed, or the same capture. */ unprefixed: Io; + /** The client the target's requests go through: `api` itself in the clear, its redactedClient() view when redacted. */ + client(api: GitHubClient): GitHubClient; + /** Seals or opens the target's end state: everything in its detail but the slug, which the channel holds. */ close(end: Omit): TargetOutcome["detail"]; } @@ -252,6 +256,7 @@ export function publicChannel(io: Io, slug: string, attributed: boolean): Target display: slug, io: prefixedIo(io, attributed ? `${slug}: ` : ""), unprefixed: io, + client: (api) => api, close: ({ outcomes, note, file }) => ({ slug, outcomes, note, file }), }; } @@ -262,6 +267,7 @@ export function redactedChannel(io: Io, slug: string, display: string): TargetCh display, io: capture.io, unprefixed: capture.io, + client: redactedClient, close: ({ outcomes, note, file }) => markPrivate({ slug, outcomes, note, file, transcript: capture.drain() }), }; diff --git a/src/flows/single.ts b/src/flows/single.ts index 815403e2f..c25b38b82 100644 --- a/src/flows/single.ts +++ b/src/flows/single.ts @@ -101,7 +101,7 @@ async function runTarget( async () => engineOutcome( await runForRepo( - api, + channel.client(api), { repo: cfg.repo, settings: injected.settings, diff --git a/src/flows/snapshot.ts b/src/flows/snapshot.ts index 82e0cce13..13f355367 100644 --- a/src/flows/snapshot.ts +++ b/src/flows/snapshot.ts @@ -280,7 +280,7 @@ async function snapshotFile( channel, () => snapshotTarget({ - api, + api: channel.client(api), repo: cfg.repo, cfg, path: cfg.snapshotFile, @@ -333,7 +333,7 @@ async function snapshotDir( channel, () => snapshotTarget({ - api, + api: channel.client(api), repo: target.repo, cfg, path: located.path, diff --git a/src/github/api.ts b/src/github/api.ts index dc8f97506..68b38885c 100644 --- a/src/github/api.ts +++ b/src/github/api.ts @@ -1,8 +1,8 @@ /** - * The GitHubClient port and its one implementation, GitHubApi, on @octokit/core with the request-log, retry, - * throttling, and paginate-rest plugins; what a failed request becomes is api-error.ts's, and what a trace line may - * show is trace-redaction.ts's. A payload's JSON body is its own serialization (redactSecretPayloadSafe), never an - * endpoint typing that could drop an unknown field. + * The GitHubClient port, its view for a redacted target (redactedClient), and its one implementation, GitHubApi, on + * @octokit/core with the request-log, retry, throttling, and paginate-rest plugins; what a failed request becomes is + * api-error.ts's, and what a trace line may show is trace-redaction.ts's. A payload's JSON body is its own + * serialization (redactSecretPayloadSafe), never an endpoint typing that could drop an unknown field. */ import { Octokit } from "@octokit/core"; @@ -35,12 +35,7 @@ import { throttleGroups, } from "./scheduler.js"; import { redactSecretPayloadSafe } from "./secret-scan.js"; -import { - redactingOctokitLog, - repoSlugOf, - type TraceIo, - TraceRedaction, -} from "./trace-redaction.js"; +import { redactingOctokitLog, type TraceIo, TraceRedaction } from "./trace-redaction.js"; /** The single source for the header default here, the action.yml `api-version` default, and the inputs fallback; the action-yml contract test asserts the three stay equal. */ export const DEFAULT_API_VERSION = "2022-11-28"; @@ -56,6 +51,8 @@ export interface GraphqlOp { } /** + * What a caller knows about a request that the client cannot read off it. + * * `carriesSecret` marks a request whose payload or variables hold a resolved secret. The engine sets it from the act of * resolving (engine/execute.ts) and withholds the request's error on its own side of this port whatever the client * answers (sections/contract/requests.ts), so a caller-supplied client cannot leak an echoed value into an outcome or @@ -63,6 +60,9 @@ export interface GraphqlOp { */ export interface RequestMark { carriesSecret?: boolean; + + /** The request serves a redacted target, so its route and payload stay out of the trace; redactedClient() sets it. */ + redactTrace?: boolean; } /** @@ -76,7 +76,7 @@ export type ClientAnswer = { data: D } | { error: ApiError } | { failed: stri /** The page size every list walk asks GitHub for, GitHub's documented maximum, unless the endpoint caps lower. */ export const PAGE_SIZE = 100; -export interface ListOptions { +export interface ListOptions extends Pick { /** Items per page; PAGE_SIZE when omitted. An endpoint with a documented cap below it passes the cap. */ perPage?: number | undefined; @@ -85,15 +85,11 @@ export interface ListOptions { } export interface GitHubClient { - /** - * `redactTrace` holds the request's `/repos//` slug redacted for the request's duration, for the - * visibility probe, which must not leak the slug before it knows whether the repository is private. - */ tryRequest( method: string, path: string, payload?: unknown, - options?: RequestMark & { accept?: string; raw?: boolean; redactTrace?: boolean }, + options?: RequestMark & { accept?: string; raw?: boolean }, ): Promise>; /** @@ -116,7 +112,34 @@ export interface GitHubClient { ): Promise>>; } -/** The primary and secondary limits are handled identically but for the log `label`, so one factory keeps them from drifting. */ +/** One view per client, so what a section memoizes per client (the owner gate's probe) spans every redacted target of a run. */ +const redactedViews = new WeakMap(); + +/** + * The view of `api` a redacted target's requests go through: each carries `redactTrace`, so none prints its route or + * payload, whatever section sends it and whether or not the route names the repository. + */ +export function redactedClient(api: GitHubClient): GitHubClient { + const known = redactedViews.get(api); + if (known !== undefined) { + return known; + } + + const view: GitHubClient = { + tryRequest: (method, path, payload, options) => + api.tryRequest(method, path, payload, { ...options, redactTrace: true }), + tryList: (path, options) => api.tryList(path, { ...options, redactTrace: true }), + tryGraphql: (op, variables, slug, options) => + api.tryGraphql(op, variables, slug, { ...options, redactTrace: true }), + }; + redactedViews.set(api, view); + return view; +} + +/** + * The primary and secondary limits are handled identically but for the log `label`, so one factory keeps them from + * drifting. The line is written where the answer is decided, so it never announces a retry the answer refuses. + */ function throttleCallback( label: string, trace: TraceRedaction, @@ -127,10 +150,21 @@ function throttleCallback( retryCount: number, ) => boolean { return (retryAfter, options, _octokit, retryCount) => { - trace.debug( - `${label} on ${options.method} ${trace.path(options.url).path}; retry ${retryCount + 1}/${MAX_RETRIES} after ${retryAfter}s`, - ); - return retryAfter <= MAX_RETRY_WAIT_S && retryCount < MAX_RETRIES; + const limited = `${label} on ${options.method} ${trace.path(options.url).path}`; + if (retryCount >= MAX_RETRIES) { + trace.debug(`${limited}; giving up: the ${MAX_RETRIES} retries are spent`); + return false; + } + + if (retryAfter > MAX_RETRY_WAIT_S) { + trace.debug( + `${limited}; giving up: the wait of ${retryAfter}s is over the ${MAX_RETRY_WAIT_S}s cap`, + ); + return false; + } + + trace.debug(`${limited}; retry ${retryCount + 1}/${MAX_RETRIES} after ${retryAfter}s`); + return true; }; } @@ -276,25 +310,31 @@ export class GitHubApi implements GitHubClient { }); } - async tryRequest( + /** The port's three methods share one rule: a request marked `redactTrace` seals the trace until it settles. */ + tryRequest( method: string, path: string, payload?: unknown, - options?: RequestMark & { accept?: string; raw?: boolean; redactTrace?: boolean }, + options?: RequestMark & { accept?: string; raw?: boolean }, ): Promise> { - if (!options?.redactTrace) { - return this.request(method, path, payload, options); - } - const slug = repoSlugOf(path); - if (slug === undefined) { - throw new Error(`BUG: redactTrace needs a /repos// path, got ${path}`); - } - const release = this.trace.hold(slug); - try { - return await this.request(method, path, payload, options); - } finally { - release(); - } + return this.trace.during(options?.redactTrace === true, () => + this.request(method, path, payload, options), + ); + } + + tryList(path: string, options?: ListOptions): Promise> { + return this.trace.during(options?.redactTrace === true, () => this.list(path, options)); + } + + tryGraphql( + op: GraphqlOp, + variables: Readonly>, + slug: string, + options?: RequestMark, + ): Promise>> { + return this.trace.during(options?.redactTrace === true, () => + this.graphql(op, variables, slug, options), + ); } /** @@ -305,7 +345,7 @@ export class GitHubApi implements GitHubClient { * is noted before the plugin reads it as an empty list (GitHub's answer for an empty repository) and surfaces as the * error it is. */ - async tryList(path: string, options?: ListOptions): Promise> { + private async list(path: string, options?: ListOptions): Promise> { const pages: unknown[] = []; let conflict: OctokitHttpError | undefined; let current = path; @@ -495,7 +535,7 @@ export class GitHubApi implements GitHubClient { * any errors[] entry, even beside partial data -> { error }, so a section never acts on a half-answered query * `extensions.warnings` (legacy node-ID notices) -> the debug trace only */ - async tryGraphql( + private async graphql( op: GraphqlOp, variables: Readonly>, slug: string, diff --git a/src/github/repo-file.ts b/src/github/repo-file.ts index 71ddd4e02..53db562d4 100644 --- a/src/github/repo-file.ts +++ b/src/github/repo-file.ts @@ -6,12 +6,15 @@ * -> that branch's git ref is read, which needs Contents: read and succeeds whether or not the file exists * -> the token could have read the file, so it is missing * - * A failed ref read leaves the proof inconclusive: a denied grant, or an empty repository whose branch has no commit. + * A refused ref read leaves the proof inconclusive: a denied grant (403, 404), or an empty repository (409, no commit yet). */ import type { GitHubClient } from "./api.js"; import { type ApiError, classifyApiError } from "./api-error.js"; +/** The status of a ref read on a repository with no commit. */ +const EMPTY_REPOSITORY = 409; + export async function getRepoFile( api: GitHubClient, slug: string, @@ -64,7 +67,9 @@ export async function getRepoFile( if (!("error" in refProbe)) { return { missing: true }; } - if (classifyApiError(refProbe.error) !== "permission") { + const inconclusive = + refProbe.error.status === EMPTY_REPOSITORY || classifyApiError(refProbe.error) === "permission"; + if (!inconclusive) { return { error: refProbe.error }; } return { diff --git a/src/github/trace-redaction.ts b/src/github/trace-redaction.ts index 2fa05d5c9..7f109c516 100644 --- a/src/github/trace-redaction.ts +++ b/src/github/trace-redaction.ts @@ -1,28 +1,27 @@ /** - * Slug redaction of the client's trace lines: a /repos// slug collapses its line while the Io port masks - * it or an in-flight request holds it (GitHubApi's redactTrace, for the visibility probe), and octokit's plugin log - * runs through the same scan (redactingOctokitLog). TraceIo is the slice of Io the client (api.ts) takes for this. + * What the client's trace lines may show of a redacted target; TraceIo is the slice of Io the client (api.ts) takes + * for it. Two rules, either one enough: + * + * a request marked as serving a redacted target -> the trace is sealed until it settles (during) + * a slug masked through the Io port -> every line naming it collapses, whoever sent the request + * + * Octokit's plugin log runs through the same rules (redactingOctokitLog). */ import type { Io } from "../io.js"; -import { SLUG_SEGMENT, type SlugKey, slugKey } from "./slug.js"; +import { SLUG_SEGMENT, slugKey } from "./slug.js"; export type TraceIo = Pick; // The segment charset stops at the segment boundary so an octokit line's trailing " - 204 with id ..." is never folded into the name, and the `i` flag keeps a mixed-case path from slipping the redaction. const REPO_SLUG = new RegExp(`/repos/(${SLUG_SEGMENT}/${SLUG_SEGMENT})`, "i"); -export function repoSlugOf(path: string): string | undefined { - return path.match(REPO_SLUG)?.[1]; -} +const REDACTED = ""; -/** - * A slug is redacted while it is masked through the Io port or held by an in-flight request (the visibility probe). A - * traced payload is private content no mask covers, so it is dropped. - */ +/** A traced payload is private content no mask covers, so a redacted path drops it. */ export class TraceRedaction { - // One token per hold, so concurrent holds on the same slug release independently and a double release is inert. - private readonly holds = new Set<{ readonly slug: SlugKey }>(); + /** How many requests in flight serve a redacted target. */ + private sealed = 0; constructor(private readonly io: TraceIo) {} @@ -30,12 +29,22 @@ export class TraceRedaction { this.io.debug(line); } - hold(slug: string): () => void { - const token = { slug: slugKey(slug) }; - this.holds.add(token); - return () => { - this.holds.delete(token); - }; + /** + * Runs `request`; when it serves a redacted target, no line written until it settles shows a path or a payload. + * The seal is the whole trace's, not the request's: a plugin logs through the client, so its line for this + * request cannot be told from its line for another one in flight beside it. + */ + async during(servesRedactedTarget: boolean, request: () => Promise): Promise { + if (!servesRedactedTarget) { + return request(); + } + + this.sealed += 1; + try { + return await request(); + } finally { + this.sealed -= 1; + } } isRedacted(slug: string): boolean { @@ -53,9 +62,9 @@ export class TraceRedaction { * anything but a constant leaks what redaction hides. */ path(path: string): { path: string; redacted: boolean } { - const slug = repoSlugOf(path); - if (slug && this.isRedacted(slug)) { - return { path: "", redacted: true }; + const slug = path.match(REPO_SLUG)?.[1]; + if (this.sealed > 0 || (slug !== undefined && this.isRedacted(slug))) { + return { path: REDACTED, redacted: true }; } return { path, redacted: false }; } @@ -65,19 +74,20 @@ export class TraceRedaction { * needle as a case-insensitive substring collapses the whole line. */ message(message: string): string { + if (this.sealed > 0) { + return REDACTED; + } + const lower = message.toLowerCase(); for (const needle of this.needles()) { if (lower.includes(needle)) { - return ""; + return REDACTED; } } return message; } private *needles(): Iterable { - for (const token of this.holds) { - yield token.slug; - } for (const value of this.io.masked()) { // An empty mask would match every line. if (value !== "") { diff --git a/test/github/api.test.ts b/test/github/api.test.ts index 6bd3055f7..36a9b40d5 100644 --- a/test/github/api.test.ts +++ b/test/github/api.test.ts @@ -3,8 +3,11 @@ import { parse as parseYaml } from "yaml"; import { DEFAULT_API_VERSION, GitHubApi, + type GitHubClient, + type GraphqlOp, MAX_RETRIES, MAX_RETRY_WAIT_S, + redactedClient, } from "../../src/github/api.js"; import { type ApiError, @@ -18,7 +21,7 @@ import { TIMERS_SCHEDULER, } from "../../src/github/scheduler.js"; import { redactingOctokitLog, TraceRedaction } from "../../src/github/trace-redaction.js"; -import { api, restoreFetch, stubFetch, traceIo } from "./stub.js"; +import { api, json, restoreFetch, stubFetch, timeless, traceIo } from "./stub.js"; afterEach(restoreFetch); @@ -31,6 +34,12 @@ const rateLimited = () => headers: { "retry-after": "0", "x-ratelimit-remaining": "0" }, }); +const READ_OP: GraphqlOp = { + name: "RepoId", + kind: "read", + query: 'query RepoId($owner: String!) { repository(owner: $owner, name: "r") { id } }', +}; + describe("retry and throttling", () => { test("429 rate limits are retried until they succeed", async () => { const state = stubFetch([rateLimited, rateLimited, okJson]); @@ -104,6 +113,56 @@ describe("retry and throttling", () => { expect("error" in result && result.error.message).toBe('{"message":"rate limited"}'); expect("error" in result && result.error.documentationUrl).toBeUndefined(); }); + + // The throttle line is the trace's only record of what the client did about a limit: it once announced a retry the + // callback then refused, so a run that failed at once read as one that had waited. + const primaryLimit = (resetInSeconds: number) => () => + new Response('{"message":"API rate limit exceeded for user ID 1."}', { + status: 403, + headers: { + "content-type": "application/json", + "x-ratelimit-remaining": "0", + "x-ratelimit-reset": String(Math.floor(Date.now() / 1000) + resetInSeconds), + }, + }); + test.each< + [ + name: string, + send: (client: GitHubApi) => Promise, + response: () => Response, + requests: number, + lines: string[], + ] + >([ + [ + "a primary limit whose reset is beyond the cap gives up with one request sent", + (client) => client.tryRequest("GET", "/repos/o/r"), + primaryLimit(3000), + 1, + [ + `rate limit on GET /repos/o/r; giving up: the wait of Ns is over the ${MAX_RETRY_WAIT_S}s cap`, + ], + ], + [ + "a GraphQL RATE_LIMITED is retried, then given up once the retries are spent", + (client) => client.tryGraphql(READ_OP, { owner: "o" }, "o/r"), + json({ data: null, errors: [{ type: "RATE_LIMITED", message: "slow down" }] }), + 1 + MAX_RETRIES, + [ + `rate limit on POST /graphql; retry 1/${MAX_RETRIES} after 0s`, + `rate limit on POST /graphql; retry 2/${MAX_RETRIES} after 0s`, + `rate limit on POST /graphql; giving up: the ${MAX_RETRIES} retries are spent`, + ], + ], + ])("the throttle trace says what follows: %s", async (_name, send, response, requests, lines) => { + const state = stubFetch([response]); + const trace = traceIo(); + await send(api(trace.io)); + expect(state.calls).toBe(requests); + // The wait is read off the clock (3000 or 3001 here), so its number is folded. + const limited = trace.lines.filter((line) => line.startsWith("rate limit on")); + expect(limited.map((line) => line.replace(/wait of \d+s/, "wait of Ns"))).toEqual(lines); + }); }); describe("GSAC_RETRY_BASE_MS: millisecond units and the immediate scheduler, same plugin topology", () => { @@ -482,27 +541,78 @@ describe("debug-trace hardening for redacted slugs", () => { expect(dbg.lines).toEqual(["", ""]); }); - test("redactTrace holds the slug for the request only; the same client traces it legibly afterwards", async () => { - // The visibility probe must not leak its target before the answer is known; once the flow decided it is public, the same slug is legible again. - stubFetch([() => new Response(null, { status: 204 })]); - const dbg = traceIo(); - const client = api(dbg.io); - await client.tryRequest("GET", "/repos/owner/probed", undefined, { redactTrace: true }); - // Octokit's own request lines ride the same channel, so the whole window must stay free of the slug. - const held = dbg.lines.join("\n"); - expect(held).toContain("GET -> 204"); - expect(held).not.toContain("owner/probed"); - dbg.lines.length = 0; - await client.tryRequest("GET", "/repos/owner/probed"); - expect(dbg.lines.join("\n")).toContain("GET /repos/owner/probed -> 204"); - // A hold needs a slug to hold: a slug-free path cannot be traced redacted. - await expect( - client.tryRequest("GET", "/user", undefined, { redactTrace: true }), - ).rejects.toThrow(/redactTrace needs a/); - }); + // No slug is masked in this table: the mark alone decides, so a route that names no repository hides too. Each row + // is sent marked, then unmarked through the same client, which proves the seal lifts and shows what it hid. + test.each< + [ + name: string, + send: (client: GitHubApi, mark: { redactTrace?: boolean }) => Promise, + response: () => Response, + own: string, + hidden: string, + ] + >([ + [ + "the visibility probe", + (client, mark) => client.tryRequest("GET", "/repos/owner/probed", undefined, mark), + json({ private: true }), + "GET -> 200 (Nms)", + "GET /repos/owner/probed -> 200", + ], + [ + "the owner probe", + (client, mark) => client.tryRequest("GET", "/orgs/secret-org", undefined, mark), + json({ login: "secret-org" }), + "GET -> 200 (Nms)", + "GET /orgs/secret-org -> 200", + ], + [ + "the app lookup", + (client, mark) => client.tryRequest("GET", "/apps/internal-deploy-bot", undefined, mark), + json({ slug: "internal-deploy-bot" }), + "GET -> 200 (Nms)", + "GET /apps/internal-deploy-bot -> 200", + ], + [ + "a write and its payload", + (client, mark) => + client.tryRequest("PATCH", "/orgs/secret-org", { description: "CANARY" }, mark), + json({}), + "PATCH -> 200 (Nms)", + 'payload: {"description":"CANARY"}', + ], + [ + "a list walk", + (client, mark) => client.tryList("/orgs/secret-org/teams", mark), + json([]), + "GET -> 200 (Nms)", + "GET /orgs/secret-org/teams?per_page=100&page=1 -> 200", + ], + [ + "a GraphQL operation and its variables", + (client, mark) => + client.tryGraphql(READ_OP, { owner: "secret-org" }, "secret-org/private-repo", mark), + json({ data: { repository: { id: "R_1" } } }), + "", + 'variables: {"owner":"secret-org"}', + ], + ])( + "a request marked as serving a redacted target leaves no route or payload in the trace: %s", + async (_name, send, response, own, hidden) => { + stubFetch([response]); + const dbg = traceIo(); + const client = api(dbg.io); + await send(client, { redactTrace: true }); + // The request-log plugin's two lines for the attempt, then the client's own. + expect(dbg.lines.map(timeless)).toEqual(["", "", own]); + dbg.lines.length = 0; + await send(client, {}); + expect(dbg.lines.join("\n")).toContain(hidden); + }, + ); test("a rate-limited visibility probe leaks no raw slug in any trace", async () => { - // The throttle-callback trace fires on the 429 retry, before the probe result exists, so the probe's request-window hold is what redacts it. + // The throttle-callback trace fires on the 429 retry, before the probe result exists, so the seal the probe's mark sets is what redacts it. const { createVisibilityResolver } = await import("../../src/github/repo-visibility.js"); stubFetch([ rateLimited, @@ -1231,21 +1341,66 @@ describe("DELETE request bodies reach the wire", () => { }); }); -describe("TraceRedaction holds", () => { - test("overlapping holds on one slug are independent: releasing one, even twice, keeps the other's redaction", () => { +describe("TraceRedaction.during", () => { + test("overlapping requests for redacted targets keep the trace sealed until the last one settles, a rejected one included", async () => { + // A seal the first settled request lifted would print the plugin lines of the one still in flight. const trace = new TraceRedaction(traceIo().io); - const releaseFirst = trace.hold("o/probed"); - const releaseSecond = trace.hold("O/Probed"); - releaseFirst(); - releaseFirst(); // a repeated release must not consume the other hold - expect(trace.path("/repos/o/probed/labels")).toEqual({ path: "", redacted: true }); - expect(trace.message("retrying o/probed after 429")).toBe(""); - releaseSecond(); - expect(trace.path("/repos/o/probed/labels")).toEqual({ - path: "/repos/o/probed/labels", - redacted: false, - }); - expect(trace.message("retrying o/probed after 429")).toBe("retrying o/probed after 429"); - expect(trace.isRedacted("o/probed")).toBe(false); + const first = Promise.withResolvers(); + const second = Promise.withResolvers(); + const firstRequest = trace.during(true, () => first.promise); + const secondRequest = trace.during(true, () => second.promise); + first.resolve(); + await firstRequest; + expect(trace.path("/orgs/o")).toEqual({ path: "", redacted: true }); + expect(trace.message("GET /orgs/o - 200 with id A1 in 1ms")).toBe(""); + second.reject(new Error("socket hang up")); + await expect(secondRequest).rejects.toThrow("socket hang up"); + expect(trace.path("/orgs/o")).toEqual({ path: "/orgs/o", redacted: false }); + expect(trace.message("GET /orgs/o - 200 with id A1 in 1ms")).toBe( + "GET /orgs/o - 200 with id A1 in 1ms", + ); + }); +}); + +describe("redactedClient", () => { + test("every request through the view reaches the client marked, the caller's own options kept", async () => { + // A method of the view that forgot the mark would print its route; one that dropped the caller's options would + // lose carriesSecret, and with it the withholding of a secret's echo. + const seen: unknown[][] = []; + const client: GitHubClient = { + tryRequest: async (...args) => { + seen.push(args); + return { data: null }; + }, + tryList: async (...args) => { + seen.push(args); + return { data: [] }; + }, + tryGraphql: async (...args) => { + seen.push(args); + return { data: {} }; + }, + }; + const view = redactedClient(client); + await view.tryRequest( + "PUT", + "/orgs/o/teams/t", + { permission: "push" }, + { carriesSecret: true }, + ); + await view.tryList("/orgs/o/teams", { perPage: 30 }); + await view.tryGraphql(READ_OP, { owner: "o" }, "o/r"); + expect(seen).toEqual([ + [ + "PUT", + "/orgs/o/teams/t", + { permission: "push" }, + { carriesSecret: true, redactTrace: true }, + ], + ["/orgs/o/teams", { perPage: 30, redactTrace: true }], + [READ_OP, { owner: "o" }, "o/r", { redactTrace: true }], + ]); + // One view per client: the owner gate memoizes its probe per client, so a view per target would probe once per redacted target. + expect(redactedClient(client)).toBe(view); }); }); diff --git a/test/github/redacted-target-trace.test.ts b/test/github/redacted-target-trace.test.ts new file mode 100644 index 000000000..912d9de2d --- /dev/null +++ b/test/github/redacted-target-trace.test.ts @@ -0,0 +1,160 @@ +/** + * The debug trace of a whole run over a redacted target: the flow, the engine, and the real sections over GitHubApi. + * The owner probe (GET /orgs/) is the request under watch: it serves the target and carries no repository + * slug, so only the run's knowledge that the target is redacted can keep it out of the trace. + */ + +import { afterEach, describe, expect, test } from "bun:test"; +import { writeFileSync } from "node:fs"; +import { join } from "node:path"; +import { DEFAULT_DISCOVERY_FILTERS } from "../../src/discovery/discover.js"; +import { parseRepoSlug } from "../../src/discovery/targets.js"; +import { SectionSelection } from "../../src/engine/section-selection.js"; +import { runMulti } from "../../src/flows/multi.js"; +import { runSingle } from "../../src/flows/single.js"; +import { runSnapshot } from "../../src/flows/snapshot.js"; +import type { GitHubClient } from "../../src/github/api.js"; +import { type Io, silentIo } from "../../src/io.js"; +import { withTempDir } from "../temp-dir.js"; +import { api, json, restoreFetch, timeless } from "./stub.js"; + +afterEach(restoreFetch); + +const PRIVATE = "secret-org/private-repo"; +const TEAMS = "teams:\n - name: reviewers\n permission: push\n"; + +/** What GitHub answers a run of the teams section on `slug`, private or public. */ +function teamsRepo(slug: string, isPrivate: boolean): Record Response> { + const owner = slug.slice(0, slug.indexOf("/")); + return { + [`/repos/${slug}`]: json({ private: isPrivate, visibility: isPrivate ? "private" : "public" }), + [`/repos/${slug}/contents/.github/settings.yml`]: () => new Response(TEAMS), + [`/orgs/${owner}`]: json({ login: owner }), + [`/repos/${slug}/teams`]: json([]), + }; +} + +/** A fetch stub answering by path, an unrouted path as GitHub's 404; `sent` is every path requested, in order. */ +function routeFetch(routes: Record Response>): { sent: string[] } { + const state = { sent: [] as string[] }; + globalThis.fetch = (async (input: string | URL | Request) => { + const { pathname } = new URL(input instanceof Request ? input.url : String(input)); + state.sent.push(pathname); + return routes[pathname]?.() ?? new Response('{"message":"Not Found"}', { status: 404 }); + }) as unknown as typeof fetch; + return state; +} + +/** An Io whose debug trace is kept; the flow and the client share it, as the action's run does. */ +function tracedIo(): { io: Io; lines: string[] } { + const lines: string[] = []; + return { lines, io: { ...silentIo(), debug: (line) => lines.push(line) } }; +} + +/** Whether a request path, or a trace line, is the owner probe: /orgs/ and nothing under it. */ +const isOwnerProbe = (text: string): boolean => /\/orgs\/[\w-]+( |$)/.test(text); + +const REDACT = { privateRepos: "redact" as const, selfSlug: "acme/admin" }; +const CHECK = { + ...REDACT, + mode: "check" as const, + onMissingPermission: "fail" as const, + sections: SectionSelection.ALL, + privateReport: "none" as const, + reportPublicKey: "", + runUrl: "", +}; +const SNAPSHOT = { + ...REDACT, + onMissingPermission: "fail" as const, + sections: SectionSelection.of({ only: ["teams"] })._unsafeUnwrap(), +}; +const FLEET = { + reposDir: "", + adminOwner: "acme", + discoveryFilters: DEFAULT_DISCOVERY_FILTERS, + discoveryFiltersSet: [], +}; + +describe("the debug trace of a run over a redacted target", () => { + // One row per place a flow hands a target's client to the engine: a flow that handed over the run's own client + // printed the probe, since nothing in its path says whom it serves. + test.each<[flow: string, run: (client: GitHubClient, io: Io, dir: string) => Promise]>([ + [ + "a single-repo check", + async (client, io, dir) => { + const settingsFile = join(dir, "settings.yml"); + writeFileSync(settingsFile, TEAMS); + const repo = parseRepoSlug(PRIVATE)._unsafeUnwrap(); + return (await runSingle(client, { ...CHECK, repo, settingsFile }, io)).isOk(); + }, + ], + [ + "a multi-repo check", + async (client, io) => { + const fleet = { ...CHECK, ...FLEET, reposInput: PRIVATE, defaultsFile: "" }; + return (await runMulti(client, fleet, io)).isOk(); + }, + ], + [ + "a snapshot to a file", + async (client, io, dir) => { + const repo = parseRepoSlug(PRIVATE)._unsafeUnwrap(); + const snapshotFile = join(dir, "snapshot.yml"); + const file = { ...SNAPSHOT, form: "file" as const, repo, snapshotFile }; + return (await runSnapshot(client, file, io)).isOk(); + }, + ], + [ + "a snapshot to a directory", + async (client, io, dir) => { + const snapshotDir = join(dir, "out"); + const fleet = { + ...SNAPSHOT, + ...FLEET, + form: "dir" as const, + reposInput: PRIVATE, + snapshotDir, + }; + return (await runSnapshot(client, fleet, io)).isOk(); + }, + ], + ])("%s of a private target leaves only redacted lines", (_flow, run) => + withTempDir("sac-redacted-trace-", async (dir) => { + const { sent } = routeFetch(teamsRepo(PRIVATE, true)); + const trace = tracedIo(); + expect(await run(api(trace.io), trace.io, dir)).toBe(true); + // The control: the probe was sent, so its absence from the trace is redaction, not a request never made. + expect(sent.filter(isOwnerProbe)).toEqual(["/orgs/secret-org"]); + // Every request of the run serves the one redacted target, so the whole trace is the two redacted forms: a + // plugin's line, and the client's own, which keeps the method and the status. + const forms = trace.lines.map((line) => line.replace(/-> \d+ \(\d+ms\)/, "-> NNN (Nms)")); + expect([...new Set(forms)].sort()).toEqual(["", "GET -> NNN (Nms)"]); + }), + ); + + test("a multi-repo run prints the public target's owner probe beside the private target's hidden one", async () => { + // Both targets send the same slug-free probe, so this is the pair a seal left on, or never set, cannot pass. + const { sent } = routeFetch({ ...teamsRepo("acme/app", false), ...teamsRepo(PRIVATE, true) }); + const trace = tracedIo(); + const run = await runMulti( + api(trace.io), + { ...CHECK, ...FLEET, reposInput: `acme/app,${PRIVATE}`, defaultsFile: "" }, + trace.io, + ); + expect(run.map((targets) => targets.map((target) => [target.display, target.result]))).toEqual( + expect.objectContaining({ + value: [ + ["acme/app", "drift"], + ["private repository #1", "drift"], + ], + }), + ); + expect(sent.filter(isOwnerProbe)).toEqual(["/orgs/acme", "/orgs/secret-org"]); + expect(trace.lines.filter(isOwnerProbe).map(timeless)).toEqual([ + "GET /orgs/acme - 200 with id undefined in Nms", + "GET /orgs/acme -> 200 (Nms)", + ]); + expect(trace.lines.filter((line) => /secret-org|private-repo/.test(line))).toEqual([]); + }); +}); diff --git a/test/github/repo-file.test.ts b/test/github/repo-file.test.ts index ec923bfa4..f8cb3fb34 100644 --- a/test/github/repo-file.test.ts +++ b/test/github/repo-file.test.ts @@ -63,14 +63,14 @@ describe("getRepoFile", () => { test.each([ [ "404", - "is an inconclusive proof naming the grant and the empty-branch case, never a missing file", + "is an inconclusive proof naming the grant, never a missing file", notFound, 1, unproven(404), ], [ "403", - "is an inconclusive proof naming the grant and the empty-branch case, never a missing file", + "is an inconclusive proof naming the grant, never a missing file", forbidden, 1, unproven(403), @@ -102,6 +102,19 @@ describe("getRepoFile", () => { expect(result).toEqual(expected); }); + test("an empty repository is an inconclusive proof naming the uninitialized branch, never a failure to re-run", async () => { + // What GitHub answers for a repository with no commit: the contents read 404s with its own message and the ref + // read 409s. Read as a failure, the 409 told the operator to re-run a request no re-run can change. + const state = stubFetch([ + () => json(404, { message: "This repository is empty." }), + repo(), + () => json(409, { message: "Git Repository is empty." }), + ]); + const result = await getRepoFile(api(), "o/r", FILE); + expect(state.paths).toEqual([CONTENTS, REPO, REF]); + expect(result).toEqual(unproven(409)); + }); + test("a contents 404 on an invisible repo surfaces the repo-level error before any ref read", async () => { const state = stubFetch([notFound, notFound]); const result = await getRepoFile(api(), "o/r", FILE); diff --git a/test/github/stub.ts b/test/github/stub.ts index 7a877a9dd..250aeff68 100644 --- a/test/github/stub.ts +++ b/test/github/stub.ts @@ -45,6 +45,13 @@ export function stubFetch(responses: Array<() => Response>): { return state; } +/** A stubbed JSON answer as a factory, the shape stubFetch and a route table take, so every request reads a fresh body. */ +export const json = (body: unknown) => (): Response => + new Response(JSON.stringify(body), { headers: { "content-type": "application/json" } }); + +/** A trace line with its timing folded, so a whole line compares. */ +export const timeless = (line: string): string => line.replace(/\d+ms/, "Nms"); + /** A fresh trace facet per test, with an isolated mask registry so one test's masks never redact another's traces. */ export function traceIo(): { io: TraceIo & Pick; lines: string[] } { const lines: string[] = []; From e8a982f32b6d1f6831d7a7232a577d162647cfe0 Mon Sep 17 00:00:00 2001 From: Vivswan Shah <58091053+Vivswan@users.noreply.github.com> Date: Wed, 11 Nov 2026 20:57:45 -0500 Subject: [PATCH 2/2] test(github): run the redacted-trace flow tests on the e2e mock server The flow tests answered GitHub with a hand-written route table. They now run against the e2e mock server (startMockServer), which already serves the visibility probe, the contents read, the owner probe, and the teams list, and records every request. The four flows and the mixed-fleet control still fail on the unfixed flow lines. The library reference names redactTrace on the RequestMark and ListOptions rows, and the troubleshooting page says the trace collapse holds in single-repository, multi-repo, and snapshot runs. Comments only in src/: RequestMark documents both of its members, the redacted constant is commented beside its neighbour, and the visibility probe's comment says what the mark does now that the slug-keyed hold is gone. --- docs/operate/troubleshooting.md | 2 +- docs/reference/library.md | 4 +- src/github/api.ts | 15 ++- src/github/repo-visibility.ts | 2 +- src/github/trace-redaction.ts | 1 + test/github/redacted-target-trace.test.ts | 131 ++++++++++++---------- 6 files changed, 81 insertions(+), 74 deletions(-) diff --git a/docs/operate/troubleshooting.md b/docs/operate/troubleshooting.md index 03a7f1bb5..ee7e4695c 100644 --- a/docs/operate/troubleshooting.md +++ b/docs/operate/troubleshooting.md @@ -90,7 +90,7 @@ Every API call the action makes is traced as a debug line: method, path, request What the trace never shows: - **The token.** The authorization header is not part of the trace line, and a token stored as a repository secret is masked by the runner wherever it appears in output. -- **A redacted private target's detail.** For redacted private targets in multi-repo mode, the traced path collapses to `` and the payload is dropped entirely. +- **A redacted private target's detail.** For a redacted private target, whether a single-repository, multi-repo, or snapshot run reaches it, the traced path collapses to `` and the payload is dropped entirely. Failures do not need debug mode; every error already carries the API's message and the fix, so reach for the trace when you need to see the requests that succeeded. diff --git a/docs/reference/library.md b/docs/reference/library.md index 8e5aa226a..6b867e65c 100644 --- a/docs/reference/library.md +++ b/docs/reference/library.md @@ -135,8 +135,8 @@ console.log(parsed.success, SECTION_KEYS.length, schema.$schema); | `DEFAULT_API_VERSION` | const | The `X-GitHub-Api-Version` the action pins | | `ApiError` | type | A failed request as the port returns it: `status`, `message`, `body` | | `GraphqlOp` | type | A GraphQL operation as the port takes it | -| `RequestMark` | type | The per-request options; `carriesSecret` marks a payload holding a resolved secret | -| `ListOptions` | type | What `tryList` takes beside the path: `perPage` (the page size, `PAGE_SIZE` when omitted) and `until` (ends the walk after the first page it accepts) | +| `RequestMark` | type | The per-request options; `carriesSecret` marks a payload holding a resolved secret, `redactTrace` a request serving a redacted target, whose route and payload stay out of the trace | +| `ListOptions` | type | What `tryList` takes beside the path: `perPage` (the page size, `PAGE_SIZE` when omitted), `until` (ends the walk after the first page it accepts), and `redactTrace` (as on `RequestMark`) | | `PAGE_SIZE` | const | The page size a list walk asks for unless the endpoint caps lower: 100, GitHub's maximum | | `classifyApiError` | function | Which `ApiErrorKind` an `ApiError` is, the one judgment the two predicates below read | | `ApiErrorKind` | type | `"rate-limit"`, `"permission"` (any other 403, and every 404: a fine-grained token answers 404 for a resource it denies, so an absent one lands here too), or `"other"` | diff --git a/src/github/api.ts b/src/github/api.ts index 68b38885c..6105e9dc7 100644 --- a/src/github/api.ts +++ b/src/github/api.ts @@ -50,15 +50,14 @@ export interface GraphqlOp { readonly query: string; } -/** - * What a caller knows about a request that the client cannot read off it. - * - * `carriesSecret` marks a request whose payload or variables hold a resolved secret. The engine sets it from the act of - * resolving (engine/execute.ts) and withholds the request's error on its own side of this port whatever the client - * answers (sections/contract/requests.ts), so a caller-supplied client cannot leak an echoed value into an outcome or - * a report; GitHubApi honors the mark too, beside its field-name scan, for its direct callers. - */ +/** What a caller knows about a request that the client cannot read off it. */ export interface RequestMark { + /** + * The payload or variables hold a resolved secret. The engine sets it from the act of resolving (engine/execute.ts) + * and withholds the request's error on its own side of this port whatever the client answers + * (sections/contract/requests.ts), so a caller-supplied client cannot leak an echoed value into an outcome or a + * report; GitHubApi honors the mark too, beside its field-name scan, for its direct callers. + */ carriesSecret?: boolean; /** The request serves a redacted target, so its route and payload stay out of the trace; redactedClient() sets it. */ diff --git a/src/github/repo-visibility.ts b/src/github/repo-visibility.ts index c0eb9dd14..b0d88b0a6 100644 --- a/src/github/repo-visibility.ts +++ b/src/github/repo-visibility.ts @@ -26,7 +26,7 @@ export function createVisibilityResolver( async function probe(api: GitHubClient, slug: string): Promise { // The probe decides redaction, so its own trace (and any throttle-callback trace) must fail closed before the answer - // is known: redactTrace holds the slug redacted for the request's duration. + // is known: the redactTrace mark seals the trace until the request settles. const result = await api.tryRequest("GET", `/repos/${slug}`, undefined, { redactTrace: true }); if ("failed" in result || "error" in result) { return "unknown"; diff --git a/src/github/trace-redaction.ts b/src/github/trace-redaction.ts index 7f109c516..da45fe72d 100644 --- a/src/github/trace-redaction.ts +++ b/src/github/trace-redaction.ts @@ -16,6 +16,7 @@ export type TraceIo = Pick; // The segment charset stops at the segment boundary so an octokit line's trailing " - 204 with id ..." is never folded into the name, and the `i` flag keeps a mixed-case path from slipping the redaction. const REPO_SLUG = new RegExp(`/repos/(${SLUG_SEGMENT}/${SLUG_SEGMENT})`, "i"); +// A collapsed path and a collapsed plugin line read the same on every route, so the constant carries nothing of either. const REDACTED = ""; /** A traced payload is private content no mask covers, so a redacted path drops it. */ diff --git a/test/github/redacted-target-trace.test.ts b/test/github/redacted-target-trace.test.ts index 912d9de2d..b16aeb57c 100644 --- a/test/github/redacted-target-trace.test.ts +++ b/test/github/redacted-target-trace.test.ts @@ -1,59 +1,31 @@ /** - * The debug trace of a whole run over a redacted target: the flow, the engine, and the real sections over GitHubApi. - * The owner probe (GET /orgs/) is the request under watch: it serves the target and carries no repository - * slug, so only the run's knowledge that the target is redacted can keep it out of the trace. + * The debug trace of a whole run over a redacted target: the flow, the engine, and the real sections over GitHubApi, + * against the e2e mock server. The owner probe (GET /orgs/) is the request under watch: it serves the target + * and carries no repository slug, so only the run's knowledge that the target is redacted keeps it out of the trace. */ -import { afterEach, describe, expect, test } from "bun:test"; +import { describe, expect, test } from "bun:test"; import { writeFileSync } from "node:fs"; import { join } from "node:path"; +import { stringify as stringifyYaml } from "yaml"; import { DEFAULT_DISCOVERY_FILTERS } from "../../src/discovery/discover.js"; import { parseRepoSlug } from "../../src/discovery/targets.js"; import { SectionSelection } from "../../src/engine/section-selection.js"; import { runMulti } from "../../src/flows/multi.js"; import { runSingle } from "../../src/flows/single.js"; import { runSnapshot } from "../../src/flows/snapshot.js"; -import type { GitHubClient } from "../../src/github/api.js"; +import { GitHubApi, type GitHubClient } from "../../src/github/api.js"; import { type Io, silentIo } from "../../src/io.js"; +import { E2E_TOKEN } from "../e2e/constants.js"; +import type { MockHandle } from "../e2e/mock/server.js"; +import { mockServerLifecycle, scenario } from "../e2e/mock/server-test-support.js"; import { withTempDir } from "../temp-dir.js"; -import { api, json, restoreFetch, timeless } from "./stub.js"; - -afterEach(restoreFetch); +import { timeless } from "./stub.js"; +const start = mockServerLifecycle(); +const PUBLIC = "acme/app"; const PRIVATE = "secret-org/private-repo"; -const TEAMS = "teams:\n - name: reviewers\n permission: push\n"; - -/** What GitHub answers a run of the teams section on `slug`, private or public. */ -function teamsRepo(slug: string, isPrivate: boolean): Record Response> { - const owner = slug.slice(0, slug.indexOf("/")); - return { - [`/repos/${slug}`]: json({ private: isPrivate, visibility: isPrivate ? "private" : "public" }), - [`/repos/${slug}/contents/.github/settings.yml`]: () => new Response(TEAMS), - [`/orgs/${owner}`]: json({ login: owner }), - [`/repos/${slug}/teams`]: json([]), - }; -} - -/** A fetch stub answering by path, an unrouted path as GitHub's 404; `sent` is every path requested, in order. */ -function routeFetch(routes: Record Response>): { sent: string[] } { - const state = { sent: [] as string[] }; - globalThis.fetch = (async (input: string | URL | Request) => { - const { pathname } = new URL(input instanceof Request ? input.url : String(input)); - state.sent.push(pathname); - return routes[pathname]?.() ?? new Response('{"message":"Not Found"}', { status: 404 }); - }) as unknown as typeof fetch; - return state; -} - -/** An Io whose debug trace is kept; the flow and the client share it, as the action's run does. */ -function tracedIo(): { io: Io; lines: string[] } { - const lines: string[] = []; - return { lines, io: { ...silentIo(), debug: (line) => lines.push(line) } }; -} - -/** Whether a request path, or a trace line, is the owner probe: /orgs/ and nothing under it. */ -const isOwnerProbe = (text: string): boolean => /\/orgs\/[\w-]+( |$)/.test(text); - +const TEAMS = { teams: [{ name: "reviewers", permission: "push" }] }; const REDACT = { privateRepos: "redact" as const, selfSlug: "acme/admin" }; const CHECK = { ...REDACT, @@ -69,13 +41,50 @@ const SNAPSHOT = { onMissingPermission: "fail" as const, sections: SectionSelection.of({ only: ["teams"] })._unsafeUnwrap(), }; -const FLEET = { +const TARGETS = { reposDir: "", adminOwner: "acme", discoveryFilters: DEFAULT_DISCOVERY_FILTERS, discoveryFiltersSet: [], }; +/** What one run shares: the mock GitHub, a client pointed at it, and the Io whose debug trace `lines` keeps. */ +interface Run { + mock: MockHandle; + client: GitHubClient; + io: Io; + lines: string[]; +} + +/** A run over a mock GitHub holding the public and the private repository, each declaring the same team. */ +async function startRun(): Promise { + const mock = await start( + scenario({ + repos: { + [PUBLIC]: { settings: TEAMS }, + [PRIVATE]: { + settings: TEAMS, + live_state: { repo: { private: true, visibility: "private" } }, + }, + }, + }), + ); + const lines: string[] = []; + const io: Io = { ...silentIo(), debug: (line) => lines.push(line) }; + const client = new GitHubApi({ token: E2E_TOKEN, io, baseUrl: mock.url, retryBaseMs: 1 }); + return { mock, client, io, lines }; +} + +/** Whether a request path, or a trace line, is the owner probe: /orgs/ and nothing under it. */ +function isOwnerProbe(text: string): boolean { + return /\/orgs\/[\w-]+( |$)/.test(text); +} + +/** The owner probes the mock received, in order. */ +function ownerProbes(mock: MockHandle): string[] { + return mock.requests.map((request) => request.pathname).filter(isOwnerProbe); +} + describe("the debug trace of a run over a redacted target", () => { // One row per place a flow hands a target's client to the engine: a flow that handed over the run's own client // printed the probe, since nothing in its path says whom it serves. @@ -84,7 +93,7 @@ describe("the debug trace of a run over a redacted target", () => { "a single-repo check", async (client, io, dir) => { const settingsFile = join(dir, "settings.yml"); - writeFileSync(settingsFile, TEAMS); + writeFileSync(settingsFile, stringifyYaml(TEAMS)); const repo = parseRepoSlug(PRIVATE)._unsafeUnwrap(); return (await runSingle(client, { ...CHECK, repo, settingsFile }, io)).isOk(); }, @@ -92,7 +101,7 @@ describe("the debug trace of a run over a redacted target", () => { [ "a multi-repo check", async (client, io) => { - const fleet = { ...CHECK, ...FLEET, reposInput: PRIVATE, defaultsFile: "" }; + const fleet = { ...CHECK, ...TARGETS, reposInput: PRIVATE, defaultsFile: "" }; return (await runMulti(client, fleet, io)).isOk(); }, ], @@ -111,7 +120,7 @@ describe("the debug trace of a run over a redacted target", () => { const snapshotDir = join(dir, "out"); const fleet = { ...SNAPSHOT, - ...FLEET, + ...TARGETS, form: "dir" as const, reposInput: PRIVATE, snapshotDir, @@ -121,40 +130,38 @@ describe("the debug trace of a run over a redacted target", () => { ], ])("%s of a private target leaves only redacted lines", (_flow, run) => withTempDir("sac-redacted-trace-", async (dir) => { - const { sent } = routeFetch(teamsRepo(PRIVATE, true)); - const trace = tracedIo(); - expect(await run(api(trace.io), trace.io, dir)).toBe(true); - // The control: the probe was sent, so its absence from the trace is redaction, not a request never made. - expect(sent.filter(isOwnerProbe)).toEqual(["/orgs/secret-org"]); + const { mock, client, io, lines } = await startRun(); + expect(await run(client, io, dir)).toBe(true); + // The controls: the probe reached the mock, which answered every request by its contract, so the probe's + // absence from the trace is redaction, not a request never made. + expect(ownerProbes(mock)).toEqual(["/orgs/secret-org"]); + expect(mock.violations).toEqual([]); // Every request of the run serves the one redacted target, so the whole trace is the two redacted forms: a // plugin's line, and the client's own, which keeps the method and the status. - const forms = trace.lines.map((line) => line.replace(/-> \d+ \(\d+ms\)/, "-> NNN (Nms)")); + const forms = lines.map((line) => line.replace(/-> \d+ \(\d+ms\)/, "-> NNN (Nms)")); expect([...new Set(forms)].sort()).toEqual(["", "GET -> NNN (Nms)"]); }), ); test("a multi-repo run prints the public target's owner probe beside the private target's hidden one", async () => { // Both targets send the same slug-free probe, so this is the pair a seal left on, or never set, cannot pass. - const { sent } = routeFetch({ ...teamsRepo("acme/app", false), ...teamsRepo(PRIVATE, true) }); - const trace = tracedIo(); - const run = await runMulti( - api(trace.io), - { ...CHECK, ...FLEET, reposInput: `acme/app,${PRIVATE}`, defaultsFile: "" }, - trace.io, - ); + const { mock, client, io, lines } = await startRun(); + const fleet = { ...CHECK, ...TARGETS, reposInput: `${PUBLIC},${PRIVATE}`, defaultsFile: "" }; + const run = await runMulti(client, fleet, io); expect(run.map((targets) => targets.map((target) => [target.display, target.result]))).toEqual( expect.objectContaining({ value: [ - ["acme/app", "drift"], + [PUBLIC, "drift"], ["private repository #1", "drift"], ], }), ); - expect(sent.filter(isOwnerProbe)).toEqual(["/orgs/acme", "/orgs/secret-org"]); - expect(trace.lines.filter(isOwnerProbe).map(timeless)).toEqual([ + expect(ownerProbes(mock)).toEqual(["/orgs/acme", "/orgs/secret-org"]); + expect(mock.violations).toEqual([]); + expect(lines.filter(isOwnerProbe).map(timeless)).toEqual([ "GET /orgs/acme - 200 with id undefined in Nms", "GET /orgs/acme -> 200 (Nms)", ]); - expect(trace.lines.filter((line) => /secret-org|private-repo/.test(line))).toEqual([]); + expect(lines.filter((line) => /secret-org|private-repo/.test(line))).toEqual([]); }); });