Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions docs/ado-script.md
Original file line number Diff line number Diff line change
Expand Up @@ -404,9 +404,15 @@ its own App JWT / minted installation token, so it carries no
`SYSTEM_ACCESSTOKEN` and no ADO predefined mirrors. Its only env var is the
masked private-key secret (`GH_APP_PRIVATE_KEY`, and `GH_APP_TOKEN` for revoke);
every other, non-secret input (App ID, owner, repositories, output-variable
name, api-url) is a single-quoted **argv flag** rather than an env var, so a
pipeline variable can never shadow it (ADO injects pipeline variables into a
step's env, but argv comes only from the compiler-authored script).
name, optional actor-output-variable name, api-url) is a single-quoted **argv
flag** rather than an env var, so a pipeline variable can never shadow it (ADO
injects pipeline variables into a step's env, but argv comes only from the
compiler-authored script). When GitHub App-backed SafeOutputs enable
`hide-older-comments`, the bundle also captures `app_slug` from the
JWT-authenticated installation lookup and emits the derived `<slug>[bot]`
login as a non-secret same-job variable. The Stage 3 executor uses that
identity for exact matching instead of sending the installation access token
to actor-discovery endpoints it cannot call.


## End-to-end data flow
Expand Down
13 changes: 10 additions & 3 deletions docs/safe-outputs.md
Original file line number Diff line number Diff line change
Expand Up @@ -752,9 +752,16 @@ JSON uses the snake_case parameter names below.
marker and, by default, a trace footer (`footer: false` disables the visible
footer). `hide-older-comments: true` first minimizes older comments carrying
the same pipeline marker **and** authored by the authenticated actor. It never
trusts agent-authored marker text. `allowed-reasons` restricts minimization
reasons; supported reasons are `SPAM`, `ABUSE`, `OFF_TOPIC`, `OUTDATED`
(default), `RESOLVED`, and `LOW_QUALITY`.
trusts agent-authored marker text. With PAT authentication, Stage 3 resolves
the actor through GitHub's `GET /user` endpoint. With
`safe-outputs.github-app`, the JWT-authenticated token-mint step captures the
installation's App slug and passes the derived `<slug>[bot]` login to Stage 3;
installation access tokens are not sent to `/user` or `/installation` for
actor discovery. Actor resolution, older-comment discovery, and minimization
remain fail-closed and complete before the replacement comment is posted.
`allowed-reasons` restricts minimization reasons; supported reasons are
`SPAM`, `ABUSE`, `OFF_TOPIC`, `OUTDATED` (default), `RESOLVED`, and
`LOW_QUALITY`.

`hide-github-issue-comment` accepts either a numeric REST comment ID or a
GraphQL node ID. It resolves the owning issue, PR, or discussion and applies
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,14 +174,15 @@ describe("resolveGithubIssueEnv", () => {
describe("registry", () => {
it("registers the complete GitHub issue scenario family with unique ids", () => {
const ids = githubIssueScenarios.map((s) => s.id ?? s.tool);
expect(new Set(ids).size).toBe(20);
expect(new Set(ids).size).toBe(21);
expect(ids).toEqual([
"create-github-issue",
"create-github-issue-label-denied",
"set-github-issue-type",
"set-github-issue-type-clear",
"create-github-issue-temporary-id-handoff",
"comment-on-github-issue",
"comment-on-github-issue-hide-older",
"hide-github-issue-comment",
"add-github-issue-labels",
"remove-github-issue-labels",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ describe("scenario registry", () => {
"set-github-issue-type-clear",
"create-github-issue-temporary-id-handoff",
"comment-on-github-issue",
"comment-on-github-issue-hide-older",
"hide-github-issue-comment",
"add-github-issue-labels",
"remove-github-issue-labels",
Expand Down
103 changes: 103 additions & 0 deletions scripts/ado-script/src/executor-e2e/scenarios/github-issue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,108 @@ export const commentOnGithubIssue: Scenario<MutationIssueState> = {
.run(),
};

interface HideOlderCommentState extends MutationIssueState {
olderCommentId: number;
olderCommentNodeId: string;
definitionId: string;
}

export const commentOnGithubIssueHideOlder: Scenario<HideOlderCommentState> = {
id: "comment-on-github-issue-hide-older",
tool: "comment-on-github-issue",
config: (_ctx, state) =>
mutationConfig(state, {
"hide-older-comments": true,
"allowed-reasons": ["OUTDATED"],
}),
setup: async (ctx) => {
const id = "comment-on-github-issue-hide-older";
const env = resolveGithubIssueEnv(id);
await requireIssueWrite(env, id);
await requireGraphqlFeature(env, id, [["Mutation", "minimizeComment"]]);
const state = await seedMutationIssue(ctx, env, id);
const definitionId = "2079";
try {
const older = await createIssueComment(
env.gh,
state.issueNumber,
[
detBody(ctx, "comment-on-github-issue-hide-older-previous"),
`<!-- ado-aw:github-comment:pipeline-definition-id=${definitionId} -->`,
].join("\n\n"),
);
return {
...state,
olderCommentId: older.id,
olderCommentNodeId: older.nodeId,
definitionId,
};
} catch (err) {
await closeMutationIssue(state).catch(() => {});
throw err;
}
},
ndjson: async (ctx, state) => ({
issue_number: state.issueNumber,
body: detBody(ctx, "comment-on-github-issue-hide-older-replacement"),
}),
env: async (_ctx, state) => ({
...executeEnv(state),
SYSTEM_DEFINITIONID: state.definitionId,
}),
assert: async (ctx, state, record) => {
const minimized = await getCommentMinimization(
state.gh,
state.olderCommentNodeId,
);
if (!minimized.isMinimized) {
throw new Error(`older comment ${state.olderCommentId} was not minimized`);
}
if (minimized.reason?.toUpperCase() !== "OUTDATED") {
throw new Error(
`older comment ${state.olderCommentId} has unexpected minimized reason '${minimized.reason ?? ""}'`,
);
}

const expected = detBody(
ctx,
"comment-on-github-issue-hide-older-replacement",
);
const replacement = (
await listIssueComments(state.gh, state.issueNumber)
).find((comment) => comment.body.includes(expected));
if (!replacement) {
throw new Error(
`issue #${state.issueNumber} has no replacement executor comment`,
);
}
state.commentId = replacement.id;
const expectedMarker = `<!-- ado-aw:github-comment:pipeline-definition-id=${state.definitionId} -->`;
if (!replacement.body.includes(expectedMarker)) {
throw new Error("replacement comment is missing the pipeline marker");
}
if (record.result?.hidden_older_comments !== 1) {
throw new Error(
`executor reported hidden_older_comments=${String(record.result?.hidden_older_comments)}`,
);
}
},
cleanup: async (ctx, state) =>
new Teardown()
.add("delete replacement comment", () =>
deleteMatchingComments(
ctx,
state,
"comment-on-github-issue-hide-older-replacement",
),
)
.add("delete older comment", () =>
deleteIssueComment(state.gh, state.olderCommentId),
)
.add("close scratch issue", () => closeMutationIssue(state))
.run(),
};

interface HiddenCommentState extends MutationIssueState {
commentNodeId: string;
}
Expand Down Expand Up @@ -1240,6 +1342,7 @@ export const githubIssueScenarios: Scenario<unknown>[] = [
setGithubIssueTypeClear,
createGithubIssueTemporaryIdHandoff,
commentOnGithubIssue,
commentOnGithubIssueHideOlder,
hideGithubIssueComment,
addGithubIssueLabels,
removeGithubIssueLabels,
Expand Down
93 changes: 84 additions & 9 deletions scripts/ado-script/src/github-app-token/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
parseArgs,
parsePermissions,
parseRepositories,
resolveInstallationId,
resolveInstallation,
mintInstallationToken,
revoke,
main,
Expand Down Expand Up @@ -161,16 +161,20 @@ describe("parseRepositories", () => {
});
});

describe("resolveInstallationId", () => {
it("returns the id from the org endpoint when it succeeds", async () => {
const fetchFn = vi.fn().mockResolvedValueOnce(jsonResponse(200, { id: 42 }));
const id = await resolveInstallationId(
describe("resolveInstallation", () => {
it("returns installation metadata from the org endpoint", async () => {
const fetchFn = vi
.fn()
.mockResolvedValueOnce(
jsonResponse(200, { id: 42, app_slug: "ado-aw-app" }),
);
const installation = await resolveInstallation(
fetchFn as never,
"https://api.github.com",
"jwt",
"octo-org",
);
expect(id).toBe(42);
expect(installation).toEqual({ id: 42, appSlug: "ado-aw-app" });
expect(fetchFn).toHaveBeenCalledTimes(1);
expect(fetchFn.mock.calls[0]![0]).toContain("/orgs/octo-org/installation");
});
Expand All @@ -180,13 +184,13 @@ describe("resolveInstallationId", () => {
.fn()
.mockResolvedValueOnce(jsonResponse(404, "not found"))
.mockResolvedValueOnce(jsonResponse(200, { id: 7 }));
const id = await resolveInstallationId(
const installation = await resolveInstallation(
fetchFn as never,
"https://api.github.com",
"jwt",
"octo-user",
);
expect(id).toBe(7);
expect(installation).toEqual({ id: 7, appSlug: undefined });
expect(fetchFn).toHaveBeenCalledTimes(2);
expect(fetchFn.mock.calls[1]![0]).toContain("/users/octo-user/installation");
});
Expand All @@ -196,7 +200,7 @@ describe("resolveInstallationId", () => {
.fn()
.mockResolvedValue(jsonResponse(404, "nope"));
await expect(
resolveInstallationId(
resolveInstallation(
fetchFn as never,
"https://api.github.com",
"jwt",
Expand Down Expand Up @@ -284,6 +288,8 @@ describe("parseArgs", () => {
"octo-org",
"--output-var",
"GITHUB_APP_TOKEN",
"--actor-output-var",
"GITHUB_APP_ACTOR_LOGIN",
"--repositories",
"repo-a repo-b",
"--permissions-json",
Expand All @@ -295,6 +301,7 @@ describe("parseArgs", () => {
appId: "1234567",
owner: "octo-org",
outputVar: "GITHUB_APP_TOKEN",
actorOutputVar: "GITHUB_APP_ACTOR_LOGIN",
repositories: "repo-a repo-b",
permissionsJson: '{"issues":"write"}',
apiUrl: "https://ghe.example.com/api/v3",
Expand Down Expand Up @@ -371,6 +378,74 @@ describe("main", () => {
});
});

it("emits the App bot login when --actor-output-var is set", async () => {
const { privateKey } = makeKeyPair();
const fetchFn = vi
.fn()
.mockResolvedValueOnce(
jsonResponse(200, { id: 55, app_slug: "ado-aw-app" }),
)
.mockResolvedValueOnce(jsonResponse(201, { token: "ghs_minted" }));
const writes: string[] = [];
const spy = vi
.spyOn(process.stdout, "write")
.mockImplementation((chunk: string | Uint8Array): boolean => {
writes.push(chunk.toString());
return true;
});

const rc = await main(
{
appId: "123",
owner: "octo-org",
outputVar: "GITHUB_APP_TOKEN",
actorOutputVar: "GITHUB_APP_ACTOR_LOGIN",
},
{ GH_APP_PRIVATE_KEY: privateKey } as NodeJS.ProcessEnv,
fetchFn as never,
);
spy.mockRestore();

expect(rc).toBe(0);
const out = writes.join("");
expect(out).toContain(
"##vso[task.setvariable variable=GITHUB_APP_TOKEN;issecret=true]ghs_minted",
);
expect(out).toContain(
"##vso[task.setvariable variable=GITHUB_APP_ACTOR_LOGIN]ado-aw-app[bot]",
);
});

it("fails before minting when actor output is requested without app_slug", async () => {
const { privateKey } = makeKeyPair();
const fetchFn = vi
.fn()
.mockResolvedValueOnce(jsonResponse(200, { id: 55 }));
const writes: string[] = [];
const spy = vi
.spyOn(process.stdout, "write")
.mockImplementation((chunk: string | Uint8Array): boolean => {
writes.push(chunk.toString());
return true;
});

const rc = await main(
{
appId: "123",
owner: "octo-org",
actorOutputVar: "GITHUB_APP_ACTOR_LOGIN",
},
{ GH_APP_PRIVATE_KEY: privateKey } as NodeJS.ProcessEnv,
fetchFn as never,
);
spy.mockRestore();

expect(rc).toBe(1);
expect(fetchFn).toHaveBeenCalledTimes(1);
expect(writes.join("")).toContain("returned no app_slug");
expect(writes.join("")).not.toContain("issecret=true");
});

it("honours --api-url and --output-var", async () => {
const { privateKey } = makeKeyPair();
const fetchFn = vi
Expand Down
Loading
Loading