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
2 changes: 1 addition & 1 deletion .changeset/patch-cross-repo-allowlist-validation.md

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

3 changes: 3 additions & 0 deletions actions/setup/js/assign_agent_helpers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,9 @@ async function getPullRequestDetails(owner, repo, pullNumber) {
* @returns {Promise<boolean>} True if successful
*/
async function assignAgentToIssue(assignableId, agentId, currentAssignees, agentName, allowedAgents = null, pullRequestRepoId = null, model = null, customAgent = null, customInstructions = null, baseBranch = null) {
// SECURITY: pullRequestRepoId specifies a cross-repo target (targetRepositoryId).
// Callers MUST validate the corresponding repository slug against allowedRepos using
// validateTargetRepo (from repo_helpers.cjs) before invoking this function.
// Filter current assignees based on allowed list (if configured)
let filteredAssignees = currentAssignees;
if (allowedAgents && allowedAgents.length > 0) {
Expand Down
23 changes: 19 additions & 4 deletions actions/setup/js/collect_ndjson_output.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const { AGENT_OUTPUT_FILENAME, TMP_GH_AW_PATH } = require("./constants.cjs");
const { ERR_API, ERR_PARSE } = require("./error_codes.cjs");
const { isPayloadUserBot } = require("./resolve_mentions.cjs");
const { parseIntTemplatable } = require("./templatable.cjs");
const { parseAllowedRepos, validateTargetRepo } = require("./repo_helpers.cjs");

async function main() {
try {
Expand Down Expand Up @@ -217,6 +218,11 @@ async function main() {
// indentation/pretty-printing, parsing will fail.
const lines = outputContent.trim().split("\n");

// Resolve allowed repos for cross-repo targeting validation in the pre-scan loop.
// The triggering repository is always allowed; additional repos come from config.
const defaultTargetRepo = `${context.repo.owner}/${context.repo.repo}`;
const allowedRepos = parseAllowedRepos(safeOutputsConfig?.allowed_repos || safeOutputsConfig?.["allowed-repos"]);

// Pre-scan: collect target issue authors from add_comment items with explicit item_number
// so they are included in the first sanitization pass.
// We do this before the main loop so the allowed mentions array can be extended.
Expand All @@ -230,10 +236,19 @@ async function main() {
// Determine which repo to query (use explicit repo field or fall back to triggering repo)
let targetOwner = context.repo.owner;
let targetRepo = context.repo.repo;
if (typeof preview.repo === "string" && preview.repo.includes("/")) {
const parts = preview.repo.split("/");
targetOwner = parts[0];
targetRepo = parts[1];
if (typeof preview.repo === "string") {
const candidateRepo = preview.repo.trim();
if (candidateRepo.includes("/")) {
// Validate the user-supplied repo against allowedRepos before making API calls
const repoValidation = validateTargetRepo(candidateRepo, defaultTargetRepo, allowedRepos);
if (repoValidation.valid) {
const parts = candidateRepo.split("/");
targetOwner = parts[0];
targetRepo = parts[1];
} else {
core.info(`[MENTIONS] Skipping cross-repo mention lookup for '${candidateRepo}': ${repoValidation.error}`);
}
}
}
try {
const { data: issueData } = await github.rest.issues.get({
Expand Down
16 changes: 16 additions & 0 deletions actions/setup/js/repo_helpers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -187,11 +187,27 @@ function resolveAndValidateRepo(item, defaultTargetRepo, allowedRepos, operation
};
}

/**
* Validate a target repository for cross-repository operations.
* Shared utility for handlers that accept user-supplied target repositories;
* must be called before any API interaction with the cross-repo target.
* This named function makes cross-repo validation intent explicit and
* allows conformance checks to verify SEC-005 compliance by file.
* @param {string} repo - Repository slug to validate (can be "owner/repo" or just "repo")
* @param {string} defaultRepo - Default (always-allowed) target repository
* @param {Set<string>} allowedRepos - Set of explicitly allowed repo patterns
* @returns {{valid: boolean, error: string|null, qualifiedRepo: string}}
*/
function validateTargetRepo(repo, defaultRepo, allowedRepos) {
return validateRepo(repo, defaultRepo, allowedRepos);
}

module.exports = {
parseAllowedRepos,
getDefaultTargetRepo,
isRepoAllowed,
validateRepo,
validateTargetRepo,
Copy link
Contributor

Choose a reason for hiding this comment

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

The validateTargetRepo wrapper delegates directly to validateRepo. Consider documenting what differentiates the two functions semantically, or consolidating callers to use validateRepo directly to reduce indirection. That said, having a named function makes SEC-005 compliance auditing easier as noted in the JSDoc comment.

parseRepoSlug,
resolveTargetRepoConfig,
resolveAndValidateRepo,
Expand Down
4 changes: 4 additions & 0 deletions actions/setup/js/submit_pr_review.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ const { getErrorMessage } = require("./error_helpers.cjs");
/** @type {string} Safe output type handled by this module */
const HANDLER_TYPE = "submit_pull_request_review";

// allowedRepos: this handler operates exclusively on the triggering repository.
// Cross-repository PR review submission is not supported; no target-repo allowlist
// check is required.

/** @type {Set<string>} Valid review event types */
const VALID_EVENTS = new Set(["APPROVE", "REQUEST_CHANGES", "COMMENT"]);

Expand Down
1 change: 1 addition & 0 deletions smoke-test-push-22284305292.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Smoke test push file - Run 22284305292