Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
72a976c
Initial plan
Copilot Jul 16, 2026
e136e17
Fix safe-outputs handler step token precedence
Copilot Jul 16, 2026
9ae4b93
chore: outline plan for safe-output token pollution tests
Copilot Jul 16, 2026
65dfd4e
test: cover handler-manager token isolation across safe outputs
Copilot Jul 16, 2026
ea7118f
fix: preserve project handler token isolation at runtime
Copilot Jul 16, 2026
bb6e74c
test: clarify project token handler assertions and mocks
Copilot Jul 16, 2026
8866a06
fix: restore global github state in project handler wrapper
Copilot Jul 16, 2026
5af8ac4
refactor: extract project github client rebinding helper
Copilot Jul 16, 2026
60abf42
refactor: centralize project handler github-token config key
Copilot Jul 16, 2026
0b13b43
test: tighten safe-output handler token isolation coverage
Copilot Jul 16, 2026
454e9c1
test: avoid vacuous handler iteration assertions
Copilot Jul 16, 2026
9e55343
Merge branch 'main' into copilot/aw-safe-outputs-update-project-token…
pelikhan Jul 16, 2026
a653044
chore: start CI failure triage
Copilot Jul 16, 2026
e21b4d9
fix cjs typecheck for handler manager global github binding
Copilot Jul 16, 2026
34ad470
Merge remote-tracking branch 'origin/main' into copilot/aw-safe-outpu…
Copilot Jul 16, 2026
f079e87
Merge branch 'main' into copilot/aw-safe-outputs-update-project-token…
pelikhan Jul 16, 2026
6e445b0
test: align mentions config spy expectations with handler client arg
Copilot Jul 16, 2026
f13f053
Merge branch 'main' into copilot/aw-safe-outputs-update-project-token…
pelikhan Jul 16, 2026
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
2 changes: 1 addition & 1 deletion .github/workflows/smoke-project.lock.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion actions/setup/js/handler_auth.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* 2. global github — step-level token set in the github-script with.github-token
*
* The step-level token itself follows (as set by the Go compiler):
* project token > global safe-outputs.github-token > magic secrets
* global safe-outputs.github-token > magic secrets
*/

/**
Expand Down
50 changes: 48 additions & 2 deletions actions/setup/js/safe_output_handler_manager.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const { checkRateLimitHeadroom } = require("./rate_limit_helpers.cjs");
const { redactSensitiveConfig } = require("./safe_outputs_config_redact.cjs");
const nodePath = require("path");
const fs = require("fs");
const GITHUB_TOKEN_CONFIG_KEY = "github-token";

/**
* Handler map configuration
Expand Down Expand Up @@ -101,6 +102,9 @@ const STANDALONE_STEP_TYPES = new Set(["upload_asset", "noop"]);
*/
const CODE_PUSH_TYPES = new Set(["push_to_pull_request_branch", "create_pull_request"]);

/** @type {Set<string>} Project-safe-output handlers that should default to GH_AW_PROJECT_GITHUB_TOKEN when no per-handler github-token is configured. */
const PROJECT_HANDLER_TYPES = new Set(["create_project", "create_project_status_update", "update_project"]);

// Threat-detection warn-mode requirement IDs from safe-outputs specification:
// - WTD2: Convertible outputs must be mapped to a reviewable type.
// - WTD3: Non-reviewable outputs must be aborted.
Expand Down Expand Up @@ -283,6 +287,37 @@ function loadConfig() {
/** @type {Set<string>} Handler types that participate in the PR review buffer */
const PR_REVIEW_HANDLER_TYPES = new Set(["create_pull_request_review_comment", "submit_pull_request_review"]);

/**
* Wrap a handler so project-safe-output execution can temporarily bind global.github
* to the handler-specific authenticated client.
*
* @param {string} type - Safe-output handler type
* @param {Function} messageHandler - Loaded handler function
* @param {Object|null} handlerGithubClient - Optional per-handler GitHub client
* @returns {Function} Wrapped handler function
*/
function wrapWithClientRebinding(type, messageHandler, handlerGithubClient) {
if (!PROJECT_HANDLER_TYPES.has(type) || !handlerGithubClient) {
return messageHandler;
}
return async (...args) => {
/** @type {any} */
const globalState = global;
const hadGithub = Object.prototype.hasOwnProperty.call(globalState, "github");
const previousGithub = globalState.github;
globalState.github = handlerGithubClient;
try {
return await messageHandler(...args);
} finally {
if (hadGithub) {
globalState.github = previousGithub;
} else {
delete globalState.github;
}
}
};
}

/**
* Load and initialize handlers for enabled safe output types
* Calls each handler's factory function (main) to get message processors
Expand All @@ -306,6 +341,10 @@ async function loadHandlers(config, prReviewBufferRegistry, resolvedAllowedMenti
// Call the factory function with config to get the message handler
const handlerConfig = { ...(config[type] || {}) };

if (PROJECT_HANDLER_TYPES.has(type) && !handlerConfig[GITHUB_TOKEN_CONFIG_KEY] && process.env.GH_AW_PROJECT_GITHUB_TOKEN) {
handlerConfig[GITHUB_TOKEN_CONFIG_KEY] = process.env.GH_AW_PROJECT_GITHUB_TOKEN;
}

// Pass top-level mentions policy through so handlers can preserve
// the same allowed mention aliases used during collection.
if (handlerConfig.mentions == null && config.mentions != null) {
Expand All @@ -320,7 +359,14 @@ async function loadHandlers(config, prReviewBufferRegistry, resolvedAllowedMenti
handlerConfig._prReviewBufferRegistry = prReviewBufferRegistry;
}

const messageHandler = await handlerModule.main(handlerConfig);
/** @type {any|null} */
let handlerGithubClient = null;
/** @type {any} */
const globalState = global;
if (handlerConfig[GITHUB_TOKEN_CONFIG_KEY] && typeof globalState.getOctokit === "function") {
handlerGithubClient = globalState.getOctokit(handlerConfig[GITHUB_TOKEN_CONFIG_KEY]);
}
const messageHandler = await handlerModule.main(handlerConfig, handlerGithubClient);

if (typeof messageHandler !== "function") {
// This is a fatal error - the handler is misconfigured
Expand All @@ -330,7 +376,7 @@ async function loadHandlers(config, prReviewBufferRegistry, resolvedAllowedMenti
throw error;
}

messageHandlers.set(type, messageHandler);
messageHandlers.set(type, wrapWithClientRebinding(type, messageHandler, handlerGithubClient));
core.info(`✓ Loaded and initialized handler for: ${type}`);
} else {
core.warning(`Handler module ${type} does not export a main function`);
Expand Down
125 changes: 123 additions & 2 deletions actions/setup/js/safe_output_handler_manager.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -277,20 +277,141 @@ describe("Safe Output Handler Manager", () => {
max: 1,
mentions: mentionsConfig,
allowedMentionAliases: ["copilot", "octocat"],
})
}),
null
);
expect(createIssueMainSpy).toHaveBeenCalledWith(
expect.objectContaining({
max: 1,
mentions: mentionsConfig,
allowedMentionAliases: ["copilot", "octocat"],
})
}),
null
);
} finally {
addCommentMainSpy.mockRestore();
createIssueMainSpy.mockRestore();
}
});

it("injects GH_AW_PROJECT_GITHUB_TOKEN for project handlers when github-token is missing", async () => {
process.env.GH_AW_PROJECT_GITHUB_TOKEN = "projects-token";
global.getOctokit = vi.fn().mockReturnValue({ client: "project-client" });

const updateProjectModule = require("./update_project.cjs");
const updateProjectMainSpy = vi.spyOn(updateProjectModule, "main").mockImplementation(async () => async () => ({ success: true }));

try {
const handlers = await loadHandlers({
update_project: { project: "https://github.com/orgs/myorg/projects/1" },
});

expect(handlers.has("update_project")).toBe(true);
expect(global.getOctokit).toHaveBeenCalledWith("projects-token");
expect(updateProjectMainSpy).toHaveBeenCalledWith(
expect.objectContaining({
project: "https://github.com/orgs/myorg/projects/1",
"github-token": "projects-token",
}),
expect.objectContaining({ client: "project-client" })
);
} finally {
updateProjectMainSpy.mockRestore();
delete process.env.GH_AW_PROJECT_GITHUB_TOKEN;
delete global.getOctokit;
}
});

it("preserves explicit project handler github-token over GH_AW_PROJECT_GITHUB_TOKEN fallback", async () => {
process.env.GH_AW_PROJECT_GITHUB_TOKEN = "fallback-project-token";
global.getOctokit = vi.fn().mockReturnValue({ client: "project-client" });

const updateProjectModule = require("./update_project.cjs");
const updateProjectMainSpy = vi.spyOn(updateProjectModule, "main").mockImplementation(async () => async () => ({ success: true }));

try {
await loadHandlers({
update_project: {
project: "https://github.com/orgs/myorg/projects/1",
"github-token": "explicit-project-token",
},
});

expect(global.getOctokit).toHaveBeenCalledWith("explicit-project-token");
expect(updateProjectMainSpy).toHaveBeenCalledWith(
expect.objectContaining({
"github-token": "explicit-project-token",
}),
expect.objectContaining({ client: "project-client" })
);
} finally {
updateProjectMainSpy.mockRestore();
delete process.env.GH_AW_PROJECT_GITHUB_TOKEN;
delete global.getOctokit;
}
});

it("wraps project handlers so global.github is set to the project client during execution", async () => {
process.env.GH_AW_PROJECT_GITHUB_TOKEN = "projects-token";
const projectClient = { client: "project-client" };
global.getOctokit = vi.fn().mockReturnValue(projectClient);

const previousGithub = { client: "shared-client" };
global.github = previousGithub;
let seenGithub = null;

const updateProjectModule = require("./update_project.cjs");
const updateProjectMainSpy = vi.spyOn(updateProjectModule, "main").mockImplementation(async () => async () => {
// outer function: handler factory at loadHandlers() time; inner function: message handler at processMessages() time
seenGithub = global.github;
return { success: true };
});

try {
const handlers = await loadHandlers({
update_project: { project: "https://github.com/orgs/myorg/projects/1" },
});
const handler = handlers.get("update_project");
expect(typeof handler).toBe("function");

await handler({ type: "update_project" });

expect(seenGithub).toBe(projectClient);
expect(global.github).toBe(previousGithub);
} finally {
updateProjectMainSpy.mockRestore();
delete process.env.GH_AW_PROJECT_GITHUB_TOKEN;
delete global.getOctokit;
global.github = previousGithub;
}
});

it("restores global.github to absent state after wrapped project handler execution", async () => {
process.env.GH_AW_PROJECT_GITHUB_TOKEN = "projects-token";
const projectClient = { client: "project-client" };
global.getOctokit = vi.fn().mockReturnValue(projectClient);
delete global.github;

const updateProjectModule = require("./update_project.cjs");
const updateProjectMainSpy = vi.spyOn(updateProjectModule, "main").mockImplementation(async () => async () => ({ success: true }));

try {
const handlers = await loadHandlers({
update_project: { project: "https://github.com/orgs/myorg/projects/1" },
});
const handler = handlers.get("update_project");
expect(typeof handler).toBe("function");

await handler({ type: "update_project" });

expect("github" in global).toBe(false);
} finally {
updateProjectMainSpy.mockRestore();
delete process.env.GH_AW_PROJECT_GITHUB_TOKEN;
delete global.getOctokit;
delete global.github;
}
});
});

describe("loadHandlers - path traversal sanitization", () => {
Expand Down
16 changes: 5 additions & 11 deletions pkg/workflow/compiler_safe_outputs_steps.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,24 +272,18 @@ func (c *Compiler) buildHandlerManagerStep(data *WorkflowData) ([]string, error)
}

// With section for github-token
// Use the standard safe outputs token for all operations.
// If project operations are configured, prefer the project token for the github-script client.
// Rationale: update_project/create_project_status_update call the Projects v2 GraphQL API, which
// cannot be accessed with the default GITHUB_TOKEN. GH_AW_PROJECT_GITHUB_TOKEN is the required
// token for Projects v2 operations.
// Use the standard safe-outputs token for the shared github-script client.
// Project operations use GH_AW_PROJECT_GITHUB_TOKEN from env with dedicated handler logic.
Comment on lines 274 to +276

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 60abf42 and prior commits: runtime project-handler auth is now wired through per-handler token fallback/client rebinding, so project operations no longer depend on the shared step-level client token path.

steps = append(steps, " with:\n")
// Token precedence for the handler manager step:
// 1. Project token (if project operations are configured) - already set above
// 2. Safe-outputs level token (so.GitHubToken)
// 3. Magic secret fallback via getEffectiveSafeOutputGitHubToken()
// 1. Safe-outputs level token (so.GitHubToken)
// 2. Magic secret fallback via getEffectiveSafeOutputGitHubToken()
//

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness regression: project handlers lose project PAT when no per-handler github-token is configured

This change decouples the step-level with.github-token from the project PAT, but the project handlers (update_project, create_project_status_update, create_project) still rely on global.github (initialized from with.github-token) when no per-handler github-token is set.

Broken scenario:

safe-outputs:
  update-project:
    project: https://github.com/orgs/myorg/projects/1
    # No github-token — user expects secrets.GH_AW_PROJECT_GITHUB_TOKEN to be used

Previously, with.github-token was elevated to secrets.GH_AW_PROJECT_GITHUB_TOKEN, giving global.github Projects v2 GraphQL access. After this change, with.github-token = safe-outputs/fallback token, and GH_AW_PROJECT_GITHUB_TOKEN is only an env var used in error messages — not used to initialize any Octokit client. update_project.main() falls back to global.github (update_project.cjs line 1263), which now lacks project access.

Suggested fix: In safe_output_handler_manager.cjs, when building handlerConfig for project-type handlers and the config has no explicit github-token, inject process.env.GH_AW_PROJECT_GITHUB_TOKEN as handlerConfig['github-token']. This preserves backward compatibility without re-coupling the step-level token.

Also, the comment in handler_auth.cjs line 17 is now stale (project token > global safe-outputs token) and needs updating.

@copilot please address this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 60abf42 and prior commits: project handlers now get GH_AW_PROJECT_GITHUB_TOKEN as per-handler fallback when github-token is unset, are initialized with a dedicated Octokit client, and execute with temporary global.github rebinding so project GraphQL calls do not fall back to the shared safe-outputs client token.

// Note: We do NOT fall back to per-output tokens (add-comment, create-issue, etc.)
// because those are specific to their operations. The handler manager needs a
// general-purpose token for the github-script client.
configToken := ""
if projectToken != "" {
configToken = projectToken
} else if data.SafeOutputs != nil && data.SafeOutputs.GitHubToken != "" {
if data.SafeOutputs != nil && data.SafeOutputs.GitHubToken != "" {
configToken = data.SafeOutputs.GitHubToken
}
c.addSafeOutputGitHubTokenForConfig(&steps, data, configToken)
Expand Down
Loading
Loading