From c3ea2ca854f72869ae64ebd338401ecda5597ec0 Mon Sep 17 00:00:00 2001 From: sepo-agent <279869237+sepo-agent@users.noreply.github.com> Date: Sat, 30 May 2026 03:27:02 -0400 Subject: [PATCH 1/4] Harden weak issue-body auth association resolution --- .agent/docs/customization/access-policy.md | 4 +- .../src/__tests__/extract-context-cli.test.ts | 177 +++++++++++++++++- .agent/src/__tests__/triage.test.ts | 14 ++ .agent/src/cli/extract-context.ts | 172 +++++++++++++---- 4 files changed, 328 insertions(+), 39 deletions(-) diff --git a/.agent/docs/customization/access-policy.md b/.agent/docs/customization/access-policy.md index 32b18526..c123950b 100644 --- a/.agent/docs/customization/access-policy.md +++ b/.agent/docs/customization/access-policy.md @@ -93,6 +93,6 @@ Organization membership detection depends on what the agent's GitHub token can s ## Weak association normalization -For mention triggers, the runtime trusts strong `author_association` values (`OWNER`, `MEMBER`, and `COLLABORATOR`) without another lookup. When GitHub reports a weaker value such as `NONE`, `CONTRIBUTOR`, `FIRST_TIMER`, or `FIRST_TIME_CONTRIBUTOR`, Sepo checks the triggering actor with `GET /repos/{owner}/{repo}/collaborators/{username}` and treats a `204` response as `COLLABORATOR` before route authorization. This applies to all supported mention surfaces, including issue and pull request bodies, discussion bodies and comments, issue comments, pull request review comments, and pull request reviews. +For mention triggers, the runtime trusts strong `author_association` values (`OWNER`, `MEMBER`, and `COLLABORATOR`) without another lookup. When GitHub reports a weaker value such as `NONE`, `CONTRIBUTOR`, `FIRST_TIMER`, or `FIRST_TIME_CONTRIBUTOR`, Sepo checks the triggering actor with `GET /repos/{owner}/{repo}/collaborators/{username}/permission` and treats a non-empty permission other than `none` as `COLLABORATOR` before route authorization. This applies to all supported mention surfaces, including issue and pull request bodies, discussion bodies and comments, issue comments, pull request review comments, and pull request reviews. -Issue-body mentions from `issues` events also refresh `author_association` from the live issue API before the collaborator fallback. These checks cover cases where repo-scoped tokens cannot see private org membership through webhook `author_association`, but GitHub author association remains token- and visibility-dependent. The public default allowlist therefore still includes `CONTRIBUTOR` unless a repository configures a stricter `AGENT_ACCESS_POLICY`. +Issue-body mentions from `issues` events also retry `author_association` from the live issue API a few times before the permission fallback. These checks cover cases where repo-scoped tokens cannot see private org membership through webhook `author_association`, but GitHub author association remains token- and visibility-dependent. The public default allowlist therefore still includes `CONTRIBUTOR` unless a repository configures a stricter `AGENT_ACCESS_POLICY`. diff --git a/.agent/src/__tests__/extract-context-cli.test.ts b/.agent/src/__tests__/extract-context-cli.test.ts index c276fad9..9873d03f 100644 --- a/.agent/src/__tests__/extract-context-cli.test.ts +++ b/.agent/src/__tests__/extract-context-cli.test.ts @@ -305,7 +305,8 @@ test("extract-context promotes weak issue author association for repository coll " printf 'CONTRIBUTOR\\n'", " exit 0", "fi", - "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"repos/self-evolving/repo/collaborators/alice\" ]; then", + "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"repos/self-evolving/repo/collaborators/alice/permission\" ]; then", + " printf 'read\\n'", " exit 0", "fi", "printf 'unexpected gh args: %s\\n' \"$*\" >&2", @@ -339,9 +340,92 @@ test("extract-context promotes weak issue author association for repository coll } }); +for (const permission of ["admin", "write", "read"]) { + test(`extract-context promotes weak issue body association from ${permission} repository permission`, () => { + const outputs = runExtractContextCli({ + eventName: "issues", + payload: { + sender: { login: "alice", type: "User" }, + issue: { + number: 358, + title: "Auth hardening", + body: "@sepo-agent /implement harden issue body auth", + html_url: "https://github.com/self-evolving/repo/issues/358", + node_id: "I_358", + author_association: "NONE", + user: { login: "alice" }, + }, + }, + ghScript: [ + "#!/usr/bin/env bash", + "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"repos/self-evolving/repo/issues/358\" ]; then", + " printf 'NONE\\n'", + " exit 0", + "fi", + "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"repos/self-evolving/repo/collaborators/alice/permission\" ]; then", + ` printf '${permission}\\n'`, + " exit 0", + "fi", + "printf 'unexpected gh args: %s\\n' \"$*\" >&2", + "exit 1", + "", + ].join("\n"), + env: { + GITHUB_REPOSITORY: "self-evolving/repo", + }, + }); + + assert.equal(outputs.get("should_respond"), "true"); + assert.equal(outputs.get("association"), "COLLABORATOR"); + assert.equal(outputs.get("source_kind"), "issue"); + assert.equal(outputs.get("requested_by"), "alice"); + assert.equal(outputs.get("requested_route"), "implement"); + }); +} + +test("extract-context preserves weak issue body association when permission lookup fails", () => { + const outputs = runExtractContextCli({ + eventName: "issues", + payload: { + sender: { login: "alice", type: "User" }, + issue: { + number: 359, + title: "Auth hardening", + body: "@sepo-agent /implement harden issue body auth", + html_url: "https://github.com/self-evolving/repo/issues/359", + node_id: "I_359", + author_association: "NONE", + user: { login: "alice" }, + }, + }, + ghScript: [ + "#!/usr/bin/env bash", + "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"repos/self-evolving/repo/issues/359\" ]; then", + " printf 'NONE\\n'", + " exit 0", + "fi", + "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"repos/self-evolving/repo/collaborators/alice/permission\" ]; then", + " exit 1", + "fi", + "printf 'unexpected gh args: %s\\n' \"$*\" >&2", + "exit 1", + "", + ].join("\n"), + env: { + GITHUB_REPOSITORY: "self-evolving/repo", + }, + }); + + assert.equal(outputs.get("should_respond"), "true"); + assert.equal(outputs.get("association"), "NONE"); + assert.equal(outputs.get("requested_by"), "alice"); + assert.equal(outputs.get("requested_route"), "implement"); +}); + const collaboratorGhScript = [ "#!/usr/bin/env bash", - "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"repos/self-evolving/repo/collaborators/alice\" ]; then", + "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"repos/self-evolving/repo/collaborators/alice/permission\" ]; then", + " printf 'read\\n'", " exit 0", "fi", "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"graphql\" ]; then", @@ -480,7 +564,7 @@ for (const testCase of weakMentionCollaboratorCases) { const nonCollaboratorGhScript = [ "#!/usr/bin/env bash", - "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"repos/self-evolving/repo/collaborators/alice\" ]; then", + "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"repos/self-evolving/repo/collaborators/alice/permission\" ]; then", " exit 1", "fi", "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"graphql\" ]; then", @@ -1037,12 +1121,79 @@ test("extract-context responds when an edited review comment adds a mention", () } }); +test("extract-context trusts strong issue comment association without GitHub lookup", () => { + const tempDir = mkdtempSync(join(tmpdir(), "agent-extract-context-")); + + try { + const eventPath = join(tempDir, "event.json"); + const outputPath = join(tempDir, "github-output.txt"); + const fakeGh = join(tempDir, "gh"); + const ghCallLog = join(tempDir, "gh-calls.txt"); + + writeFileSync( + eventPath, + JSON.stringify({ + sender: { login: "alice", type: "User" }, + comment: { + id: 108, + node_id: "IC_108", + html_url: "https://github.com/self-evolving/repo/issues/173#issuecomment-108", + body: "@sepo-agent /answer please check this", + author_association: "COLLABORATOR", + user: { login: "alice" }, + }, + issue: { + number: 173, + html_url: "https://github.com/self-evolving/repo/issues/173", + }, + }), + "utf8", + ); + writeFileSync(outputPath, "", "utf8"); + writeFileSync(ghCallLog, "", "utf8"); + writeFileSync( + fakeGh, + [ + "#!/usr/bin/env bash", + `printf '%s\\n' "$*" >> ${JSON.stringify(ghCallLog)}`, + "exit 1", + "", + ].join("\n"), + { encoding: "utf8", mode: 0o755 }, + ); + + execFileSync("node", [".agent/dist/cli/extract-context.js"], { + cwd: repoRoot, + env: { + ...process.env, + PATH: `${tempDir}:${process.env.PATH || ""}`, + GITHUB_EVENT_PATH: eventPath, + GITHUB_EVENT_NAME: "issue_comment", + GITHUB_OUTPUT: outputPath, + GITHUB_REPOSITORY: "self-evolving/repo", + INPUT_MENTION: "@sepo-agent", + INPUT_TRIGGER_KIND: "mention", + }, + stdio: "pipe", + }); + + const outputs = parseGithubOutput(outputPath); + assert.equal(outputs.get("should_respond"), "true"); + assert.equal(outputs.get("association"), "COLLABORATOR"); + assert.equal(outputs.get("requested_route"), "answer"); + assert.equal(readFileSync(ghCallLog, "utf8"), ""); + } finally { + rmSync(tempDir, { recursive: true, force: true }); + } +}); + test("extract-context lets public contributor mentions reach dispatch triage", () => { const tempDir = mkdtempSync(join(tmpdir(), "agent-extract-context-")); try { const eventPath = join(tempDir, "event.json"); const outputPath = join(tempDir, "github-output.txt"); + const fakeGh = join(tempDir, "gh"); writeFileSync( eventPath, @@ -1065,14 +1216,20 @@ test("extract-context lets public contributor mentions reach dispatch triage", ( "utf8", ); writeFileSync(outputPath, "", "utf8"); + writeFileSync(fakeGh, nonCollaboratorGhScript, { + encoding: "utf8", + mode: 0o755, + }); execFileSync("node", [".agent/dist/cli/extract-context.js"], { cwd: repoRoot, env: { ...process.env, + PATH: `${tempDir}:${process.env.PATH || ""}`, GITHUB_EVENT_PATH: eventPath, GITHUB_EVENT_NAME: "issue_comment", GITHUB_OUTPUT: outputPath, + GITHUB_REPOSITORY: "self-evolving/repo", INPUT_MENTION: "@sepo-agent", INPUT_TRIGGER_KIND: "mention", }, @@ -1094,6 +1251,7 @@ test("extract-context preserves explicit routes for later policy checks", () => try { const eventPath = join(tempDir, "event.json"); const outputPath = join(tempDir, "github-output.txt"); + const fakeGh = join(tempDir, "gh"); writeFileSync( eventPath, @@ -1116,14 +1274,20 @@ test("extract-context preserves explicit routes for later policy checks", () => "utf8", ); writeFileSync(outputPath, "", "utf8"); + writeFileSync(fakeGh, nonCollaboratorGhScript, { + encoding: "utf8", + mode: 0o755, + }); execFileSync("node", [".agent/dist/cli/extract-context.js"], { cwd: repoRoot, env: { ...process.env, + PATH: `${tempDir}:${process.env.PATH || ""}`, GITHUB_EVENT_PATH: eventPath, GITHUB_EVENT_NAME: "issue_comment", GITHUB_OUTPUT: outputPath, + GITHUB_REPOSITORY: "self-evolving/repo", INPUT_MENTION: "@sepo-agent", INPUT_TRIGGER_KIND: "mention", }, @@ -1144,6 +1308,7 @@ test("extract-context keeps known associations available for later policy checks try { const eventPath = join(tempDir, "event.json"); const outputPath = join(tempDir, "github-output.txt"); + const fakeGh = join(tempDir, "gh"); writeFileSync( eventPath, @@ -1165,14 +1330,20 @@ test("extract-context keeps known associations available for later policy checks "utf8", ); writeFileSync(outputPath, "", "utf8"); + writeFileSync(fakeGh, nonCollaboratorGhScript, { + encoding: "utf8", + mode: 0o755, + }); execFileSync("node", [".agent/dist/cli/extract-context.js"], { cwd: repoRoot, env: { ...process.env, + PATH: `${tempDir}:${process.env.PATH || ""}`, GITHUB_EVENT_PATH: eventPath, GITHUB_EVENT_NAME: "issue_comment", GITHUB_OUTPUT: outputPath, + GITHUB_REPOSITORY: "self-evolving/repo", INPUT_MENTION: "@sepo-agent", INPUT_TRIGGER_KIND: "mention", }, diff --git a/.agent/src/__tests__/triage.test.ts b/.agent/src/__tests__/triage.test.ts index 31898c78..3281a25b 100644 --- a/.agent/src/__tests__/triage.test.ts +++ b/.agent/src/__tests__/triage.test.ts @@ -403,6 +403,20 @@ test("applyDispatchPolicy denies explicit implement when access policy restricts assert.equal(d.needsApproval, false); }); +test("applyDispatchPolicy rejects explicit implement for true unauthorized none association", () => { + const d = applyDispatchPolicy( + buildRequestedRouteDecision("implement", "@sepo-agent /implement add foo"), + "issue", + "NONE", + parseAccessPolicy(""), + false, + true, + ); + assert.equal(d.route, "unsupported"); + assert.equal(d.needsApproval, false); + assert.match(d.summary, /implement requests currently require OWNER, MEMBER, COLLABORATOR, CONTRIBUTOR/); +}); + test("applyDispatchPolicy dispatches fix-pr on PR without approval", () => { const d = applyDispatchPolicy( normalizeDispatch('{"route":"fix-pr","needs_approval":true,"summary":"fix"}'), diff --git a/.agent/src/cli/extract-context.ts b/.agent/src/cli/extract-context.ts index a40d3e86..f8d29ea3 100644 --- a/.agent/src/cli/extract-context.ts +++ b/.agent/src/cli/extract-context.ts @@ -10,7 +10,7 @@ import { readFileSync } from "node:fs"; import { isKnownAuthorAssociation } from "../access-policy.js"; -import { ghApi, ghApiOk } from "../github.js"; +import { gh, ghApi, ghApiOk } from "../github.js"; import { setOutput } from "../output.js"; import { DEFAULT_MENTION, @@ -42,11 +42,41 @@ const WEAK_ASSOCIATIONS_FOR_COLLABORATOR_FALLBACK = new Set([ "FIRST_TIMER", "NONE", ]); +const ISSUE_ASSOCIATION_REFRESH_ATTEMPTS = 3; +const ISSUE_ASSOCIATION_REFRESH_BACKOFF_MS = 100; +const SLEEP_BUFFER = new Int32Array(new SharedArrayBuffer(4)); + +interface RepositoryPermissionLookup { + login: string; + permission: string; + allowed: boolean; + error: boolean; + skippedReason: string; +} + +interface IssueAssociationLookup { + association: string; + error: boolean; +} function normalizeAssociation(association: string): string { return String(association || "").trim().toUpperCase(); } +function isWeakMentionAssociation(association: string): boolean { + return WEAK_ASSOCIATIONS_FOR_COLLABORATOR_FALLBACK.has(normalizeAssociation(association)); +} + +function sleep(ms: number): void { + if (ms > 0) { + Atomics.wait(SLEEP_BUFFER, 0, 0, ms); + } +} + +function logMentionAssociationDiagnostic(message: string): void { + console.log(`[extract-context] ${message}`); +} + function hasOrgMembership(orgLogin: string, userLogin: string): boolean { const membershipState = ghApi([ `orgs/${orgLogin}/memberships/${userLogin}`, @@ -62,27 +92,45 @@ function hasOrgMembership(orgLogin: string, userLogin: string): boolean { return ghApiOk([`orgs/${orgLogin}/members/${userLogin}`]); } -function hasRepositoryPermission(userLogin: string): boolean { - if (!repository || !userLogin) { - return false; - } - - const permission = ghApi([ - `repos/${repository}/collaborators/${userLogin}/permission`, - "--jq", - ".permission // .role_name // empty", - ]).toLowerCase(); - - return Boolean(permission) && permission !== "none"; -} - -function hasRepositoryCollaborator(userLogin: string): boolean { +function getRepositoryPermission(userLogin: string): RepositoryPermissionLookup { const login = String(userLogin || "").trim(); if (!repository || !login) { - return false; + return { + login, + permission: "", + allowed: false, + error: false, + skippedReason: !repository ? "missing repository" : "missing login", + }; + } + + try { + const permission = gh([ + "api", + `repos/${repository}/collaborators/${login}/permission`, + "--jq", + ".permission // .role_name // empty", + ]).trim().toLowerCase(); + return { + login, + permission, + allowed: Boolean(permission) && permission !== "none", + error: false, + skippedReason: "", + }; + } catch { + return { + login, + permission: "", + allowed: false, + error: true, + skippedReason: "", + }; } +} - return ghApiOk([`repos/${repository}/collaborators/${login}`]); +function hasRepositoryPermission(userLogin: string): boolean { + return getRepositoryPermission(userLogin).allowed; } // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -114,7 +162,7 @@ function resolveLabelActorAssociation(payload: Record): string { return "NONE"; } -function refreshIssueAssociation( +function refreshIssueAssociationWithRetry( association: string, issueNumber: string, ): string { @@ -126,38 +174,94 @@ function refreshIssueAssociation( return normalizeAssociation(association) || association; } - const refreshed = ghApi([ - `repos/${repository}/issues/${issueNumber}`, - "--jq", - ".author_association // empty", - ]).toUpperCase(); - return refreshed || normalizeAssociation(association) || association; + let resolved = normalizeAssociation(association) || association; + for (let attempt = 1; attempt <= ISSUE_ASSOCIATION_REFRESH_ATTEMPTS; attempt += 1) { + const refreshed = getIssueAssociation(issueNumber); + logMentionAssociationDiagnostic( + `refreshed issue association attempt ${attempt}/${ISSUE_ASSOCIATION_REFRESH_ATTEMPTS}: ${refreshed.association || "empty"}${refreshed.error ? " error=true" : ""}`, + ); + + resolved = refreshed.association || resolved; + if (!isWeakMentionAssociation(resolved)) { + return resolved; + } + + if (attempt < ISSUE_ASSOCIATION_REFRESH_ATTEMPTS) { + sleep(ISSUE_ASSOCIATION_REFRESH_BACKOFF_MS); + } + } + + return resolved; +} + +function getIssueAssociation(issueNumber: string): IssueAssociationLookup { + try { + return { + association: normalizeAssociation(gh([ + "api", + `repos/${repository}/issues/${issueNumber}`, + "--jq", + ".author_association // empty", + ])), + error: false, + }; + } catch { + return { + association: "", + error: true, + }; + } +} + +function describePermissionLookup(result: RepositoryPermissionLookup): string { + const parts = [ + `login=${result.login || "empty"}`, + `permission=${result.permission || "empty"}`, + `allowed=${String(result.allowed)}`, + ]; + if (result.error) { + parts.push("error=true"); + } + if (result.skippedReason) { + parts.push(`skipped=${result.skippedReason}`); + } + return parts.join(" "); } // eslint-disable-next-line @typescript-eslint/no-explicit-any function normalizeMentionAuthorAssociation(association: string, payload: Record): string { + const rawAssociation = String(association || "").trim(); const normalized = normalizeAssociation(association); - if (authorAssociationOverride || ASSOCIATIONS_TRUSTED_WITHOUT_REFRESH.has(normalized)) { - return normalized || association; + logMentionAssociationDiagnostic(`raw event association: ${rawAssociation || "empty"}`); + + if (authorAssociationOverride || !isWeakMentionAssociation(normalized)) { + const finalAssociation = normalized || association; + logMentionAssociationDiagnostic(`final normalized association: ${finalAssociation || "empty"}`); + return finalAssociation; } - const resolved = refreshIssueAssociation( + const resolved = refreshIssueAssociationWithRetry( normalized || association, String(payload.issue?.number || ""), ); const resolvedNormalized = normalizeAssociation(resolved); if (ASSOCIATIONS_TRUSTED_WITHOUT_REFRESH.has(resolvedNormalized)) { + logMentionAssociationDiagnostic(`final normalized association: ${resolvedNormalized}`); return resolvedNormalized; } - if ( - WEAK_ASSOCIATIONS_FOR_COLLABORATOR_FALLBACK.has(resolvedNormalized) && - hasRepositoryCollaborator(getRequestedBy(eventName, payload)) - ) { - return "COLLABORATOR"; + if (isWeakMentionAssociation(resolvedNormalized)) { + const permission = getRepositoryPermission(getRequestedBy(eventName, payload)); + logMentionAssociationDiagnostic(`permission fallback result: ${describePermissionLookup(permission)}`); + if (permission.allowed) { + logMentionAssociationDiagnostic("final normalized association: COLLABORATOR"); + return "COLLABORATOR"; + } } - return resolvedNormalized || resolved; + const finalAssociation = resolvedNormalized || resolved; + logMentionAssociationDiagnostic(`final normalized association: ${finalAssociation || "empty"}`); + return finalAssociation; } if (!eventPath || !eventName) { From 5d9256cbc4eb25ad53e79fdbced24877f5e9d8e4 Mon Sep 17 00:00:00 2001 From: sepo-agent <279869237+sepo-agent@users.noreply.github.com> Date: Sat, 30 May 2026 03:43:37 -0400 Subject: [PATCH 2/4] Add retry-resolution auth regression test --- .../src/__tests__/extract-context-cli.test.ts | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/.agent/src/__tests__/extract-context-cli.test.ts b/.agent/src/__tests__/extract-context-cli.test.ts index 9873d03f..0205559b 100644 --- a/.agent/src/__tests__/extract-context-cli.test.ts +++ b/.agent/src/__tests__/extract-context-cli.test.ts @@ -272,6 +272,89 @@ test("extract-context refreshes contributor issue author association from the Gi } }); +test("extract-context resolves stale issue body association on a later retry", () => { + const tempDir = mkdtempSync(join(tmpdir(), "agent-extract-context-")); + + try { + const eventPath = join(tempDir, "event.json"); + const outputPath = join(tempDir, "github-output.txt"); + const fakeGh = join(tempDir, "gh"); + const ghCallLog = join(tempDir, "gh-calls.txt"); + const issueCallCount = join(tempDir, "issue-call-count.txt"); + + writeFileSync( + eventPath, + JSON.stringify({ + sender: { login: "alice", type: "User" }, + issue: { + number: 6, + title: "Investigate auth", + body: "@sepo-agent /implement can you fix this?", + html_url: "https://github.com/self-evolving/repo/issues/6", + node_id: "I_6", + author_association: "NONE", + user: { login: "alice" }, + }, + }), + "utf8", + ); + writeFileSync(outputPath, "", "utf8"); + writeFileSync(ghCallLog, "", "utf8"); + writeFileSync(issueCallCount, "0", "utf8"); + writeFileSync( + fakeGh, + [ + "#!/usr/bin/env bash", + `printf '%s\\n' "$*" >> ${JSON.stringify(ghCallLog)}`, + "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"repos/self-evolving/repo/issues/6\" ]; then", + ` count=$(cat ${JSON.stringify(issueCallCount)})`, + " count=$((count + 1))", + ` printf '%s' "$count" > ${JSON.stringify(issueCallCount)}`, + " if [ \"$count\" = \"1\" ]; then", + " printf 'NONE\\n'", + " else", + " printf 'COLLABORATOR\\n'", + " fi", + " exit 0", + "fi", + "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"repos/self-evolving/repo/collaborators/alice/permission\" ]; then", + " printf 'permission fallback should not be called\\n' >&2", + " exit 1", + "fi", + "printf 'unexpected gh args: %s\\n' \"$*\" >&2", + "exit 1", + "", + ].join("\n"), + { encoding: "utf8", mode: 0o755 }, + ); + + execFileSync("node", [".agent/dist/cli/extract-context.js"], { + cwd: repoRoot, + env: { + ...process.env, + PATH: `${tempDir}:${process.env.PATH || ""}`, + GITHUB_EVENT_PATH: eventPath, + GITHUB_EVENT_NAME: "issues", + GITHUB_OUTPUT: outputPath, + GITHUB_REPOSITORY: "self-evolving/repo", + INPUT_MENTION: "@sepo-agent", + INPUT_TRIGGER_KIND: "mention", + }, + stdio: "pipe", + }); + + const outputs = parseGithubOutput(outputPath); + const ghCalls = readFileSync(ghCallLog, "utf8"); + assert.equal(outputs.get("should_respond"), "true"); + assert.equal(outputs.get("association"), "COLLABORATOR"); + assert.equal(outputs.get("requested_route"), "implement"); + assert.equal(readFileSync(issueCallCount, "utf8"), "2"); + assert.doesNotMatch(ghCalls, /collaborators\/alice\/permission/); + } finally { + rmSync(tempDir, { recursive: true, force: true }); + } +}); + test("extract-context promotes weak issue author association for repository collaborators", () => { const tempDir = mkdtempSync(join(tmpdir(), "agent-extract-context-")); From aeab498b15335719c518777d2d27e48464ea8974 Mon Sep 17 00:00:00 2001 From: sepo-agent <279869237+sepo-agent@users.noreply.github.com> Date: Sat, 30 May 2026 03:58:58 -0400 Subject: [PATCH 3/4] Harden weak auth permission fallback --- .agent/docs/customization/access-policy.md | 2 +- .../src/__tests__/extract-context-cli.test.ts | 46 +++++++++++++++++-- .agent/src/cli/extract-context.ts | 8 +++- 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/.agent/docs/customization/access-policy.md b/.agent/docs/customization/access-policy.md index c123950b..9ebf9dba 100644 --- a/.agent/docs/customization/access-policy.md +++ b/.agent/docs/customization/access-policy.md @@ -93,6 +93,6 @@ Organization membership detection depends on what the agent's GitHub token can s ## Weak association normalization -For mention triggers, the runtime trusts strong `author_association` values (`OWNER`, `MEMBER`, and `COLLABORATOR`) without another lookup. When GitHub reports a weaker value such as `NONE`, `CONTRIBUTOR`, `FIRST_TIMER`, or `FIRST_TIME_CONTRIBUTOR`, Sepo checks the triggering actor with `GET /repos/{owner}/{repo}/collaborators/{username}/permission` and treats a non-empty permission other than `none` as `COLLABORATOR` before route authorization. This applies to all supported mention surfaces, including issue and pull request bodies, discussion bodies and comments, issue comments, pull request review comments, and pull request reviews. +For mention triggers, the runtime trusts strong `author_association` values (`OWNER`, `MEMBER`, and `COLLABORATOR`) without another lookup. When GitHub reports a weaker value such as `NONE`, `CONTRIBUTOR`, `FIRST_TIMER`, or `FIRST_TIME_CONTRIBUTOR`, Sepo checks the triggering actor with `GET /repos/{owner}/{repo}/collaborators/{username}/permission` and treats `triage`, `write`, `maintain`, or `admin` permission as `COLLABORATOR` before route authorization. Public `read` permission alone is not trusted because it can apply to non-collaborators on public repositories. This applies to all supported mention surfaces, including issue and pull request bodies, discussion bodies and comments, issue comments, pull request review comments, and pull request reviews. Issue-body mentions from `issues` events also retry `author_association` from the live issue API a few times before the permission fallback. These checks cover cases where repo-scoped tokens cannot see private org membership through webhook `author_association`, but GitHub author association remains token- and visibility-dependent. The public default allowlist therefore still includes `CONTRIBUTOR` unless a repository configures a stricter `AGENT_ACCESS_POLICY`. diff --git a/.agent/src/__tests__/extract-context-cli.test.ts b/.agent/src/__tests__/extract-context-cli.test.ts index 0205559b..98524fdc 100644 --- a/.agent/src/__tests__/extract-context-cli.test.ts +++ b/.agent/src/__tests__/extract-context-cli.test.ts @@ -389,7 +389,7 @@ test("extract-context promotes weak issue author association for repository coll " exit 0", "fi", "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"repos/self-evolving/repo/collaborators/alice/permission\" ]; then", - " printf 'read\\n'", + " printf 'write\\n'", " exit 0", "fi", "printf 'unexpected gh args: %s\\n' \"$*\" >&2", @@ -423,7 +423,7 @@ test("extract-context promotes weak issue author association for repository coll } }); -for (const permission of ["admin", "write", "read"]) { +for (const permission of ["admin", "maintain", "write", "triage"]) { test(`extract-context promotes weak issue body association from ${permission} repository permission`, () => { const outputs = runExtractContextCli({ eventName: "issues", @@ -466,6 +466,46 @@ for (const permission of ["admin", "write", "read"]) { }); } +test("extract-context preserves weak issue body association for public read permission", () => { + const outputs = runExtractContextCli({ + eventName: "issues", + payload: { + sender: { login: "alice", type: "User" }, + issue: { + number: 360, + title: "Auth hardening", + body: "@sepo-agent /implement harden issue body auth", + html_url: "https://github.com/self-evolving/repo/issues/360", + node_id: "I_360", + author_association: "NONE", + user: { login: "alice" }, + }, + }, + ghScript: [ + "#!/usr/bin/env bash", + "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"repos/self-evolving/repo/issues/360\" ]; then", + " printf 'NONE\\n'", + " exit 0", + "fi", + "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"repos/self-evolving/repo/collaborators/alice/permission\" ]; then", + " printf 'read\\n'", + " exit 0", + "fi", + "printf 'unexpected gh args: %s\\n' \"$*\" >&2", + "exit 1", + "", + ].join("\n"), + env: { + GITHUB_REPOSITORY: "self-evolving/repo", + }, + }); + + assert.equal(outputs.get("should_respond"), "true"); + assert.equal(outputs.get("association"), "NONE"); + assert.equal(outputs.get("requested_by"), "alice"); + assert.equal(outputs.get("requested_route"), "implement"); +}); + test("extract-context preserves weak issue body association when permission lookup fails", () => { const outputs = runExtractContextCli({ eventName: "issues", @@ -508,7 +548,7 @@ test("extract-context preserves weak issue body association when permission look const collaboratorGhScript = [ "#!/usr/bin/env bash", "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"repos/self-evolving/repo/collaborators/alice/permission\" ]; then", - " printf 'read\\n'", + " printf 'write\\n'", " exit 0", "fi", "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"graphql\" ]; then", diff --git a/.agent/src/cli/extract-context.ts b/.agent/src/cli/extract-context.ts index f8d29ea3..96749edf 100644 --- a/.agent/src/cli/extract-context.ts +++ b/.agent/src/cli/extract-context.ts @@ -42,6 +42,12 @@ const WEAK_ASSOCIATIONS_FOR_COLLABORATOR_FALLBACK = new Set([ "FIRST_TIMER", "NONE", ]); +const TRUSTED_REPOSITORY_PERMISSIONS = new Set([ + "admin", + "maintain", + "write", + "triage", +]); const ISSUE_ASSOCIATION_REFRESH_ATTEMPTS = 3; const ISSUE_ASSOCIATION_REFRESH_BACKOFF_MS = 100; const SLEEP_BUFFER = new Int32Array(new SharedArrayBuffer(4)); @@ -114,7 +120,7 @@ function getRepositoryPermission(userLogin: string): RepositoryPermissionLookup return { login, permission, - allowed: Boolean(permission) && permission !== "none", + allowed: TRUSTED_REPOSITORY_PERMISSIONS.has(permission), error: false, skippedReason: "", }; From 66b199bb7e7e0db7cbb0d0b230245c3c99dd6634 Mon Sep 17 00:00:00 2001 From: sepo-agent <279869237+sepo-agent@users.noreply.github.com> Date: Sat, 30 May 2026 04:14:56 -0400 Subject: [PATCH 4/4] Parse role_name in permission fallback --- .agent/docs/customization/access-policy.md | 2 +- .../src/__tests__/extract-context-cli.test.ts | 40 +++++++++++++++++++ .agent/src/cli/extract-context.ts | 40 ++++++++++++++++--- 3 files changed, 76 insertions(+), 6 deletions(-) diff --git a/.agent/docs/customization/access-policy.md b/.agent/docs/customization/access-policy.md index 9ebf9dba..cf731fab 100644 --- a/.agent/docs/customization/access-policy.md +++ b/.agent/docs/customization/access-policy.md @@ -93,6 +93,6 @@ Organization membership detection depends on what the agent's GitHub token can s ## Weak association normalization -For mention triggers, the runtime trusts strong `author_association` values (`OWNER`, `MEMBER`, and `COLLABORATOR`) without another lookup. When GitHub reports a weaker value such as `NONE`, `CONTRIBUTOR`, `FIRST_TIMER`, or `FIRST_TIME_CONTRIBUTOR`, Sepo checks the triggering actor with `GET /repos/{owner}/{repo}/collaborators/{username}/permission` and treats `triage`, `write`, `maintain`, or `admin` permission as `COLLABORATOR` before route authorization. Public `read` permission alone is not trusted because it can apply to non-collaborators on public repositories. This applies to all supported mention surfaces, including issue and pull request bodies, discussion bodies and comments, issue comments, pull request review comments, and pull request reviews. +For mention triggers, the runtime trusts strong `author_association` values (`OWNER`, `MEMBER`, and `COLLABORATOR`) without another lookup. When GitHub reports a weaker value such as `NONE`, `CONTRIBUTOR`, `FIRST_TIMER`, or `FIRST_TIME_CONTRIBUTOR`, Sepo checks the triggering actor with `GET /repos/{owner}/{repo}/collaborators/{username}/permission` and treats `triage`, `write`, `maintain`, or `admin` from either `permission` or `role_name` as `COLLABORATOR` before route authorization. Public `read` permission alone is not trusted because it can apply to non-collaborators on public repositories. This applies to all supported mention surfaces, including issue and pull request bodies, discussion bodies and comments, issue comments, pull request review comments, and pull request reviews. Issue-body mentions from `issues` events also retry `author_association` from the live issue API a few times before the permission fallback. These checks cover cases where repo-scoped tokens cannot see private org membership through webhook `author_association`, but GitHub author association remains token- and visibility-dependent. The public default allowlist therefore still includes `CONTRIBUTOR` unless a repository configures a stricter `AGENT_ACCESS_POLICY`. diff --git a/.agent/src/__tests__/extract-context-cli.test.ts b/.agent/src/__tests__/extract-context-cli.test.ts index 98524fdc..f2cf6648 100644 --- a/.agent/src/__tests__/extract-context-cli.test.ts +++ b/.agent/src/__tests__/extract-context-cli.test.ts @@ -506,6 +506,46 @@ test("extract-context preserves weak issue body association for public read perm assert.equal(outputs.get("requested_route"), "implement"); }); +test("extract-context promotes weak issue body association for triage role_name", () => { + const outputs = runExtractContextCli({ + eventName: "issues", + payload: { + sender: { login: "alice", type: "User" }, + issue: { + number: 361, + title: "Auth hardening", + body: "@sepo-agent /implement harden issue body auth", + html_url: "https://github.com/self-evolving/repo/issues/361", + node_id: "I_361", + author_association: "NONE", + user: { login: "alice" }, + }, + }, + ghScript: [ + "#!/usr/bin/env bash", + "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"repos/self-evolving/repo/issues/361\" ]; then", + " printf 'NONE\\n'", + " exit 0", + "fi", + "if [ \"$1\" = \"api\" ] && [ \"$2\" = \"repos/self-evolving/repo/collaborators/alice/permission\" ]; then", + " printf '{\"permission\":\"read\",\"role_name\":\"triage\"}\\n'", + " exit 0", + "fi", + "printf 'unexpected gh args: %s\\n' \"$*\" >&2", + "exit 1", + "", + ].join("\n"), + env: { + GITHUB_REPOSITORY: "self-evolving/repo", + }, + }); + + assert.equal(outputs.get("should_respond"), "true"); + assert.equal(outputs.get("association"), "COLLABORATOR"); + assert.equal(outputs.get("requested_by"), "alice"); + assert.equal(outputs.get("requested_route"), "implement"); +}); + test("extract-context preserves weak issue body association when permission lookup fails", () => { const outputs = runExtractContextCli({ eventName: "issues", diff --git a/.agent/src/cli/extract-context.ts b/.agent/src/cli/extract-context.ts index 96749edf..59cc6f4e 100644 --- a/.agent/src/cli/extract-context.ts +++ b/.agent/src/cli/extract-context.ts @@ -55,6 +55,7 @@ const SLEEP_BUFFER = new Int32Array(new SharedArrayBuffer(4)); interface RepositoryPermissionLookup { login: string; permission: string; + roleName: string; allowed: boolean; error: boolean; skippedReason: string; @@ -83,6 +84,31 @@ function logMentionAssociationDiagnostic(message: string): void { console.log(`[extract-context] ${message}`); } +function isTrustedRepositoryPermission(permission: string, roleName: string): boolean { + return TRUSTED_REPOSITORY_PERMISSIONS.has(permission) || + TRUSTED_REPOSITORY_PERMISSIONS.has(roleName); +} + +function parseRepositoryPermissionResponse(raw: string): { permission: string; roleName: string } { + const trimmed = String(raw || "").trim(); + if (!trimmed) { + return { permission: "", roleName: "" }; + } + + try { + const parsed = JSON.parse(trimmed) as { permission?: unknown; role_name?: unknown; roleName?: unknown }; + return { + permission: String(parsed.permission || "").trim().toLowerCase(), + roleName: String(parsed.role_name || parsed.roleName || "").trim().toLowerCase(), + }; + } catch { + return { + permission: trimmed.toLowerCase(), + roleName: "", + }; + } +} + function hasOrgMembership(orgLogin: string, userLogin: string): boolean { const membershipState = ghApi([ `orgs/${orgLogin}/memberships/${userLogin}`, @@ -104,6 +130,7 @@ function getRepositoryPermission(userLogin: string): RepositoryPermissionLookup return { login, permission: "", + roleName: "", allowed: false, error: false, skippedReason: !repository ? "missing repository" : "missing login", @@ -111,16 +138,17 @@ function getRepositoryPermission(userLogin: string): RepositoryPermissionLookup } try { - const permission = gh([ + const parsed = parseRepositoryPermissionResponse(gh([ "api", `repos/${repository}/collaborators/${login}/permission`, "--jq", - ".permission // .role_name // empty", - ]).trim().toLowerCase(); + '{permission: (.permission // ""), role_name: (.role_name // "")}', + ])); return { login, - permission, - allowed: TRUSTED_REPOSITORY_PERMISSIONS.has(permission), + permission: parsed.permission, + roleName: parsed.roleName, + allowed: isTrustedRepositoryPermission(parsed.permission, parsed.roleName), error: false, skippedReason: "", }; @@ -128,6 +156,7 @@ function getRepositoryPermission(userLogin: string): RepositoryPermissionLookup return { login, permission: "", + roleName: "", allowed: false, error: true, skippedReason: "", @@ -223,6 +252,7 @@ function describePermissionLookup(result: RepositoryPermissionLookup): string { const parts = [ `login=${result.login || "empty"}`, `permission=${result.permission || "empty"}`, + `role_name=${result.roleName || "empty"}`, `allowed=${String(result.allowed)}`, ]; if (result.error) {