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
5 changes: 5 additions & 0 deletions .changeset/patch-validate-cross-repo-allowlist.md

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

16 changes: 15 additions & 1 deletion actions/setup/js/dynamic_checkout.cjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// @ts-check
/// <reference types="@actions/github-script" />

const { validateTargetRepo, parseAllowedRepos, getDefaultTargetRepo } = require("./repo_helpers.cjs");
const { ERR_VALIDATION } = require("./error_codes.cjs");

/**
* Dynamic repository checkout utilities for multi-repo scenarios
* Enables switching between different repositories during handler execution
Expand Down Expand Up @@ -51,6 +54,7 @@ async function getCurrentCheckoutRepo() {
* @param {string} token - GitHub token for authentication
* @param {Object} options - Additional options
* @param {string} [options.baseBranch] - Base branch to checkout (defaults to 'main')
* @param {string[]|string} [options.allowedRepos] - Allowed repository patterns for allowlist validation
* @returns {Promise<Object>} Result with success status
*/
async function checkoutRepo(repoSlug, token, options = {}) {
Expand All @@ -60,10 +64,20 @@ async function checkoutRepo(repoSlug, token, options = {}) {
if (parts.length !== 2 || !parts[0] || !parts[1]) {
return {
success: false,
error: `Invalid repository slug: ${repoSlug}. Expected format: owner/repo`,
error: `${ERR_VALIDATION}: Invalid repository slug: ${repoSlug}. Expected format: owner/repo`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Good use of ERR_VALIDATION prefix for structured error reporting — this makes it easier to grep/filter validation errors in logs. 👍

};
}

// Validate target repo against configured allowlist before any git operations
const allowedRepos = parseAllowedRepos(options.allowedRepos);
if (allowedRepos.size > 0) {
const defaultRepo = getDefaultTargetRepo();
const validation = validateTargetRepo(repoSlug, defaultRepo, allowedRepos);
if (!validation.valid) {
return { success: false, error: `${ERR_VALIDATION}: ${validation.error}` };
}
Comment on lines +73 to +78
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

New allowlist validation behavior was added here, but the module tests don't appear to exercise the allowlist paths (allowed vs rejected repo). Adding unit tests covering (1) default repo allowed, (2) non-default repo rejected when allowlist is empty, and (3) non-default repo allowed when explicitly allowlisted would help prevent regressions.

Suggested change
if (allowedRepos.size > 0) {
const defaultRepo = getDefaultTargetRepo();
const validation = validateTargetRepo(repoSlug, defaultRepo, allowedRepos);
if (!validation.valid) {
return { success: false, error: `${ERR_VALIDATION}: ${validation.error}` };
}
const defaultRepo = getDefaultTargetRepo();
const validation = validateTargetRepo(repoSlug, defaultRepo, allowedRepos);
if (!validation.valid) {
return { success: false, error: `${ERR_VALIDATION}: ${validation.error}` };

Copilot uses AI. Check for mistakes.
}
Comment on lines +71 to +79
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

The allowlist check is currently gated by if (allowedRepos.size > 0), which means a cross-repo repoSlug will proceed with git operations when allowedRepos is unset/empty. For SEC-005, validation should still run with an empty allowlist so that only defaultRepo is permitted (and any other repo is rejected). Also note that createCheckoutManager().switchTo() calls checkoutRepo() without passing allowedRepos, so this validation path will never run unless callers supply it explicitly.

Copilot uses AI. Check for mistakes.

const [owner, repo] = parts;

core.info(`Switching checkout to repository: ${repoSlug}`);
Expand Down
17 changes: 16 additions & 1 deletion actions/setup/js/extra_empty_commit.cjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// @ts-check
/// <reference types="@actions/github-script" />

const { validateTargetRepo, parseAllowedRepos, getDefaultTargetRepo } = require("./repo_helpers.cjs");

/**
* @fileoverview Extra Empty Commit Helper
*
Expand Down Expand Up @@ -47,16 +49,29 @@ function isCrossRepoTarget(repoOwner, repoName) {
* @param {number} [options.newCommitCount] - Number of new commits being pushed. Only pushes the
* empty commit when exactly 1 new commit was pushed, preventing accidental workflow-file
* modifications on multi-commit branches and reducing loop risk.
* @param {string[]|string} [options.allowedRepos] - Allowed repository patterns for allowlist validation
* @returns {Promise<{success: boolean, skipped?: boolean, error?: string}>}
*/
async function pushExtraEmptyCommit({ branchName, repoOwner, repoName, commitMessage, newCommitCount }) {
async function pushExtraEmptyCommit({ branchName, repoOwner, repoName, commitMessage, newCommitCount, allowedRepos: allowedReposInput }) {
const token = process.env.GH_AW_CI_TRIGGER_TOKEN;

if (!token || !token.trim()) {
core.info("No extra empty commit token configured - skipping");
return { success: true, skipped: true };
}

// Validate target repository against allowlist before any git operations
const allowedRepos = parseAllowedRepos(allowedReposInput);
if (allowedRepos.size > 0) {
const targetRepo = `${repoOwner}/${repoName}`;
const defaultRepo = getDefaultTargetRepo();
const validation = validateTargetRepo(targetRepo, defaultRepo, allowedRepos);
if (!validation.valid) {
core.warning(`ERR_VALIDATION: ${validation.error}`);
return { success: false, error: validation.error ?? "" };
}
}
Comment on lines +63 to +73
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

This adds allowlist validation logic but there are no corresponding tests in extra_empty_commit.test.cjs covering allowed vs rejected repositories. Adding tests for reject/allow scenarios (including when GITHUB_REPOSITORY is unset) would ensure SEC-005 stays enforced.

Copilot uses AI. Check for mistakes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using core.error instead of core.warning here since a failed validation that prevents the commit is effectively an error condition, not just a warning.

// Cross-repo guard: never push an extra empty commit to a different repository.
// A token is needed to create the PR and that will trigger events anyway.
Comment on lines +65 to 76
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

This validation is skipped entirely when allowedRepos is empty. That allows cross-repo pushes in environments where GITHUB_REPOSITORY is unset (since isCrossRepoTarget returns false), which defeats the SEC-005 allowlist guard. Consider always calling validateTargetRepo(targetRepo, defaultRepo, allowedRepos) (even when the allowlist is empty) so only the default repo is allowed unless explicitly allowlisted.

Suggested change
if (allowedRepos.size > 0) {
const targetRepo = `${repoOwner}/${repoName}`;
const defaultRepo = getDefaultTargetRepo();
const validation = validateTargetRepo(targetRepo, defaultRepo, allowedRepos);
if (!validation.valid) {
core.warning(`ERR_VALIDATION: ${validation.error}`);
return { success: false, error: validation.error ?? "" };
}
}
// Cross-repo guard: never push an extra empty commit to a different repository.
// A token is needed to create the PR and that will trigger events anyway.
const targetRepo = `${repoOwner}/${repoName}`;
const defaultRepo = getDefaultTargetRepo();
const validation = validateTargetRepo(targetRepo, defaultRepo, allowedRepos);
if (!validation.valid) {
core.warning(`ERR_VALIDATION: ${validation.error}`);
return { success: false, error: validation.error ?? "" };
}
// Cross-repo guard: never push an extra empty commit to a different repository.
// A token is needed to create the PR and that will trigger events anyway.
// A token is needed to create the PR and that will trigger events anyway.

Copilot uses AI. Check for mistakes.
if (isCrossRepoTarget(repoOwner, repoName)) {
Expand Down
15 changes: 14 additions & 1 deletion actions/setup/js/find_repo_checkout.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const fs = require("fs");
const path = require("path");
const { execGitSync } = require("./git_helpers.cjs");
const { validateTargetRepo, parseAllowedRepos, getDefaultTargetRepo } = require("./repo_helpers.cjs");

/**
* Debug logging helper - logs to stderr when DEBUG env var matches
Expand Down Expand Up @@ -129,9 +130,11 @@ function getRemoteOriginUrl(repoPath) {
*
* @param {string} repoSlug - The repository slug to find (owner/repo format)
* @param {string} [workspaceRoot] - The workspace root to search from
* @param {Object} [options] - Additional options
* @param {string[]|string} [options.allowedRepos] - Allowed repository patterns for validation
* @returns {Object} Result with success status and path or error
*/
function findRepoCheckout(repoSlug, workspaceRoot) {
function findRepoCheckout(repoSlug, workspaceRoot, options = {}) {
const ws = workspaceRoot || process.env.GITHUB_WORKSPACE || process.cwd();
const targetSlug = normalizeRepoSlug(repoSlug);

Expand All @@ -144,6 +147,16 @@ function findRepoCheckout(repoSlug, workspaceRoot) {
};
}

// Validate target repo against configured allowlist before searching
const allowedRepos = parseAllowedRepos(options.allowedRepos);
if (allowedRepos.size > 0) {
const defaultRepo = getDefaultTargetRepo();
const validation = validateTargetRepo(targetSlug, defaultRepo, allowedRepos);
if (!validation.valid) {
return { success: false, error: validation.error };
}
}
Comment on lines +150 to +158
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

Allowlist validation was introduced for findRepoCheckout, but find_repo_checkout.test.cjs doesn’t cover the new allowed/denied branches. Consider adding tests that verify the function returns a validation error for non-allowed repos and succeeds for default/allowlisted repos.

Copilot uses AI. Check for mistakes.

// Find all git directories in the workspace
const gitDirs = findGitDirectories(ws);
debugLog(`Found ${gitDirs.length} git directories: ${gitDirs.join(", ")}`);
Comment on lines +152 to 162
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

The allowlist enforcement is currently conditional on allowedRepos.size > 0, so callers who omit options.allowedRepos can search for (and potentially operate on) arbitrary repo slugs. For SEC-005, you generally want validation to run even with an empty allowlist so that only defaultRepo is permitted unless additional repos are explicitly allowlisted.

Suggested change
if (allowedRepos.size > 0) {
const defaultRepo = getDefaultTargetRepo();
const validation = validateTargetRepo(targetSlug, defaultRepo, allowedRepos);
if (!validation.valid) {
return { success: false, error: validation.error };
}
}
// Find all git directories in the workspace
const gitDirs = findGitDirectories(ws);
debugLog(`Found ${gitDirs.length} git directories: ${gitDirs.join(", ")}`);
const defaultRepo = getDefaultTargetRepo();
const validation = validateTargetRepo(targetSlug, defaultRepo, allowedRepos);
if (!validation.valid) {
return { success: false, error: validation.error };
}
// Find all git directories in the workspace
const gitDirs = findGitDirectories(ws);
debugLog(`Found ${gitDirs.length} git directories: ${gitDirs.join(", ")}`);
const gitDirs = findGitDirectories(ws);
debugLog(`Found ${gitDirs.length} git directories: ${gitDirs.join(", ")}`);

Copilot uses AI. Check for mistakes.
Expand Down
17 changes: 17 additions & 0 deletions actions/setup/js/get_base_branch.cjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// @ts-check
/// <reference types="@actions/github-script" />

const { validateTargetRepo, parseAllowedRepos, getDefaultTargetRepo } = require("./repo_helpers.cjs");

/**
* Get the base branch name, resolving dynamically based on event context.
*
Expand Down Expand Up @@ -40,6 +42,21 @@ async function getBaseBranch(targetRepo = null) {
if (typeof github !== "undefined") {
const repoOwner = targetRepo?.owner ?? context.repo.owner;
const repoName = targetRepo?.repo ?? context.repo.repo;

// Validate target repo against allowlist before any API calls
const targetRepoSlug = `${repoOwner}/${repoName}`;
const allowedRepos = parseAllowedRepos(process.env.GH_AW_ALLOWED_REPOS);
if (allowedRepos.size > 0) {
const defaultRepo = getDefaultTargetRepo();
const validation = validateTargetRepo(targetRepoSlug, defaultRepo, allowedRepos);
if (!validation.valid) {
if (typeof core !== "undefined") {
core.warning(`ERR_VALIDATION: ${validation.error}`);
}
return process.env.DEFAULT_BRANCH || "main";
}
Comment on lines +49 to +57
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

This validation is only performed when GH_AW_ALLOWED_REPOS parses to a non-empty set. If the env var is unset/empty, cross-repo calls to getBaseBranch(repoParts) will proceed with GitHub API requests without any allowlist enforcement. For SEC-005, consider always running validateTargetRepo(targetRepoSlug, defaultRepo, allowedRepos) so that only the default repo is allowed when the allowlist is empty.

Suggested change
if (allowedRepos.size > 0) {
const defaultRepo = getDefaultTargetRepo();
const validation = validateTargetRepo(targetRepoSlug, defaultRepo, allowedRepos);
if (!validation.valid) {
if (typeof core !== "undefined") {
core.warning(`ERR_VALIDATION: ${validation.error}`);
}
return process.env.DEFAULT_BRANCH || "main";
}
const defaultRepo = getDefaultTargetRepo();
const validation = validateTargetRepo(targetRepoSlug, defaultRepo, allowedRepos);
if (!validation.valid) {
if (typeof core !== "undefined") {
core.warning(`ERR_VALIDATION: ${validation.error}`);
}
return process.env.DEFAULT_BRANCH || "main";

Copilot uses AI. Check for mistakes.
}
Comment on lines +46 to +58
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

This introduces allowlist validation behavior for cross-repo base-branch lookups, but get_base_branch.test.cjs doesn’t currently cover the allow/deny outcomes. Adding tests that set GH_AW_ALLOWED_REPOS and verify the function warns/short-circuits on a disallowed targetRepo would help keep SEC-005 covered.

Copilot uses AI. Check for mistakes.

const { data: pr } = await github.rest.pulls.get({
owner: repoOwner,
repo: repoName,
Expand Down
10 changes: 10 additions & 0 deletions actions/setup/js/safe_outputs_tools_loader.cjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// @ts-check

const { getErrorMessage } = require("./error_helpers.cjs");
const { validateTargetRepo, parseAllowedRepos, getDefaultTargetRepo } = require("./repo_helpers.cjs");

const fs = require("fs");

Expand Down Expand Up @@ -100,6 +101,15 @@ function registerPredefinedTools(server, tools, config, registerTool, normalizeT
if (tool.name === "create_pull_request" && config.create_pull_request) {
const targetRepo = config.create_pull_request["target-repo"];
if (targetRepo) {
// Validate the configured target-repo against the allowed-repos list
const allowedRepos = parseAllowedRepos(config.create_pull_request.allowed_repos);
if (allowedRepos.size > 0) {
const defaultRepo = getDefaultTargetRepo(config.create_pull_request);
const validation = validateTargetRepo(targetRepo, defaultRepo, allowedRepos);
if (!validation.valid) {
server.debug(`WARNING: SEC-005: ${validation.error}`);
}
}
Comment on lines +104 to +112
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

This check only runs when allowedRepos.size > 0, so a misconfigured target-repo can slip through silently if allowed_repos is unset/empty. If the intent is to ensure only the default repo is used unless explicitly allowlisted, run validateTargetRepo(targetRepo, defaultRepo, allowedRepos) unconditionally and either warn/disable the tool description enrichment when invalid.

See below for a potential fix:

          const defaultRepo = getDefaultTargetRepo(config.create_pull_request);
          const validation = validateTargetRepo(targetRepo, defaultRepo, allowedRepos);
          if (!validation.valid) {
            server.debug(`WARNING: SEC-005: ${validation.error}`);
          } else {
            toolToRegister = JSON.parse(JSON.stringify(tool));
            toolToRegister.description += ` Note: This workflow is configured to create pull requests in '${targetRepo}'. You do not need to specify the repo parameter.`;
            if (toolToRegister.inputSchema && toolToRegister.inputSchema.properties && toolToRegister.inputSchema.properties.repo) {
              toolToRegister.inputSchema.properties.repo.description += ` Configured default: '${targetRepo}'.`;
            }
          }

Copilot uses AI. Check for mistakes.
toolToRegister = JSON.parse(JSON.stringify(tool));
toolToRegister.description += ` Note: This workflow is configured to create pull requests in '${targetRepo}'. You do not need to specify the repo parameter.`;
Comment on lines +106 to 114
Copy link

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

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

Since this now performs target-repo validation for the create_pull_request tool configuration, consider adding/expanding tests in safe_outputs_tools_loader.test.cjs to cover the cases where target-repo is (a) default, (b) allowlisted, and (c) not allowlisted, so misconfigurations are reliably surfaced.

Suggested change
if (allowedRepos.size > 0) {
const defaultRepo = getDefaultTargetRepo(config.create_pull_request);
const validation = validateTargetRepo(targetRepo, defaultRepo, allowedRepos);
if (!validation.valid) {
server.debug(`WARNING: SEC-005: ${validation.error}`);
}
}
toolToRegister = JSON.parse(JSON.stringify(tool));
toolToRegister.description += ` Note: This workflow is configured to create pull requests in '${targetRepo}'. You do not need to specify the repo parameter.`;
let repoNote = ` Note: This workflow is configured to create pull requests in '${targetRepo}'. You do not need to specify the repo parameter.`;
if (allowedRepos.size > 0) {
const defaultRepo = getDefaultTargetRepo(config.create_pull_request);
const validation = validateTargetRepo(targetRepo, defaultRepo, allowedRepos);
if (!validation.valid) {
server.debug(`WARNING: SEC-005: ${validation.error}`);
// Surface potential misconfiguration in the tool description as well
repoNote += ` WARNING: The configured target-repo may not be in the allowed list and this configuration might be ignored at runtime.`;
}
} else {
// No allowed_repos configured: surface this as a warning so misconfigurations are easier to detect
server.debug(
`WARNING: SEC-005: No allowed_repos are configured for create_pull_request; skipping validation for configured target-repo '${targetRepo}'.`
);
repoNote += ` WARNING: No allowed_repos are configured for this workflow, so this default may not be enforced.`;
}
toolToRegister = JSON.parse(JSON.stringify(tool));
toolToRegister.description += repoNote;

Copilot uses AI. Check for mistakes.
if (toolToRegister.inputSchema && toolToRegister.inputSchema.properties && toolToRegister.inputSchema.properties.repo) {
Expand Down
Loading