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
7 changes: 7 additions & 0 deletions .github/workflows/smoke-copilot-aoai-apikey.lock.yml

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

7 changes: 7 additions & 0 deletions .github/workflows/smoke-copilot-aoai-entra.lock.yml

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

7 changes: 7 additions & 0 deletions .github/workflows/smoke-copilot-arm.lock.yml

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

7 changes: 7 additions & 0 deletions .github/workflows/smoke-copilot.lock.yml

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

92 changes: 84 additions & 8 deletions actions/setup/js/dispatch_workflow.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
const HANDLER_TYPE = "dispatch_workflow";

const { getErrorMessage } = require("./error_helpers.cjs");
const { globPatternToRegex } = require("./glob_pattern_helpers.cjs");
const { createAuthenticatedGitHubClient } = require("./handler_auth.cjs");
const { resolveTargetRepoConfig, parseRepoSlug, validateTargetRepo } = require("./repo_helpers.cjs");
const { logStagedPreviewInfo } = require("./staged_preview.cjs");
Expand All @@ -29,6 +30,8 @@ async function main(config = {}) {
const awContextWorkflows = new Set(config.aw_context_workflows || []); // Workflows that accept aw_context input
const githubClient = await createAuthenticatedGitHubClient(config);
const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config);
const allowedRefPatterns = parseAllowedRefPatterns(config.allowed_refs);
const allowedRefRegexes = allowedRefPatterns.map(pattern => globPatternToRegex(pattern, { pathMode: true, caseSensitive: true }));

// Resolve the dispatch destination repository from target-repo config, falling back to context.repo
const contextRepoSlug = `${context.repo.owner}/${context.repo.repo}`;
Expand Down Expand Up @@ -107,23 +110,23 @@ async function main(config = {}) {
// GITHUB_HEAD_REF which contains the actual PR branch name.
// For cross-repo dispatch (workflow_call relay), the caller's GITHUB_REF has no meaning on
// the target repository, so we use the compiler-injected target-ref instead.
let ref;
let defaultRef;
if (config["target-ref"]) {
// Compiler-injected target ref for cross-repo dispatch (workflow_call relay pattern).
// Takes precedence over all environment variables to avoid using the caller's ref.
ref = config["target-ref"];
core.info(`Using configured target-ref: ${ref}`);
defaultRef = config["target-ref"];
core.info(`Using configured target-ref: ${defaultRef}`);
} else if (process.env.GITHUB_HEAD_REF) {
// We're in a pull_request event, use the PR branch ref
ref = `refs/heads/${process.env.GITHUB_HEAD_REF}`;
core.info(`Using PR branch ref: ${ref}`);
defaultRef = `refs/heads/${process.env.GITHUB_HEAD_REF}`;
core.info(`Using PR branch ref: ${defaultRef}`);
} else if (process.env.GITHUB_REF || context.ref) {
// Use GITHUB_REF for non-PR contexts (push, workflow_dispatch, etc.)
ref = process.env.GITHUB_REF || context.ref;
defaultRef = process.env.GITHUB_REF || context.ref;
} else {
// Last resort: fetch the repository's default branch
ref = await getDefaultBranchRef();
core.info(`Using default branch ref: ${ref}`);
defaultRef = await getDefaultBranchRef();
core.info(`Using default branch ref: ${defaultRef}`);
}

/**
Expand Down Expand Up @@ -177,6 +180,31 @@ async function main(config = {}) {

core.info(`Dispatching workflow: ${workflowName}`);

if (message.ref !== undefined && message.ref !== null && typeof message.ref !== "string") {
core.warning(`message.ref must be a string; ignoring non-string value (type: ${typeof message.ref})`);
}
const outputRef = typeof message.ref === "string" ? message.ref.trim() : "";
let ref = defaultRef;
if (outputRef) {
ref = normalizeRef(outputRef);
if (allowedRefRegexes.length === 0) {
const error = "message.ref is not allowed unless 'allowed-refs' is configured in safe-outputs.dispatch-workflow";
core.warning(error);
return {
success: false,
error,
};
}
if (!allowedRefRegexes.some(pattern => pattern.test(ref))) {
const error = `Ref '${ref}' is not in allowed-refs: ${allowedRefPatterns.join(", ")}`;
core.warning(error);
return {
success: false,
error,

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.

normalizeRefPattern silently misroutes short tag patterns

normalizeRefPattern("tags/*") returns "refs/heads/tags/*" instead of "refs/tags/*", so an allowed-refs entry like ["tags/*"] will never match a real tag ref after normalization.

Note that normalizeRef has the same gap: normalizeRef("v1.2.3")"refs/heads/v1.2.3", which also would not match a tag ref. This inconsistency means users writing short-form tag patterns in allowed-refs get silently wrong matching.

Consider checking the tags/ prefix:

function normalizeRef(refOrBranch) {
  if (refOrBranch.startsWith("refs/")) return refOrBranch;
  if (refOrBranch.startsWith("tags/")) return `refs/${refOrBranch}`;
  return `refs/heads/${refOrBranch}`;
}

At minimum, add a test that covers bare tags/v* patterns in allowed-refs so the current behavior is explicit and not accidentally "fixed" later.

@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.

Fixed in the latest commit. Both normalizeRef and normalizeRefPattern now check for the tags/ prefix before defaulting to refs/heads/:

function normalizeRef(refOrBranch) {
  if (refOrBranch.startsWith("refs/")) return refOrBranch;
  if (refOrBranch.startsWith("tags/")) return `refs/${refOrBranch}`;
  return `refs/heads/${refOrBranch}`;
}

A test case covering tags/v* in allowed-refs and a tags/v1.2.3 message ref was added to dispatch_workflow.test.cjs.

};
}
}

// Prepare inputs - convert all values to strings as required by workflow_dispatch
// and resolve any #temporary_id references before dispatching
/** @type {Record<string, string>} */
Expand Down Expand Up @@ -320,4 +348,52 @@ async function main(config = {}) {
};
}

/**
* @param {string[]|string|undefined} allowedRefsValue
* @returns {string[]}
*/
function parseAllowedRefPatterns(allowedRefsValue) {
/** @type {string[]} */
const refs = [];
if (Array.isArray(allowedRefsValue)) {
for (const pattern of allowedRefsValue) {
if (typeof pattern === "string") {
const trimmed = pattern.trim();
if (trimmed) {
refs.push(normalizeRefPattern(trimmed));
}
}
}
return refs;
}
if (typeof allowedRefsValue === "string") {
return allowedRefsValue
.split(",")
.map(pattern => pattern.trim())
.filter(Boolean)
.map(normalizeRefPattern);
}
return refs;
}

/**
* @param {string} refOrBranch
* @returns {string}
*/
function normalizeRef(refOrBranch) {
if (refOrBranch.startsWith("refs/")) return refOrBranch;
if (refOrBranch.startsWith("tags/")) return `refs/${refOrBranch}`;
return `refs/heads/${refOrBranch}`;
}

/**
* @param {string} pattern
* @returns {string}
*/
function normalizeRefPattern(pattern) {
if (pattern.startsWith("refs/")) return pattern;
if (pattern.startsWith("tags/")) return `refs/${pattern}`;
return `refs/heads/${pattern}`;
}

module.exports = { main };
Loading
Loading