Skip to content

[jsweep] Clean validate_lockdown_requirements.cjs #39498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 7 commits into from
Jun 16, 2026
+150 −45
Merged

[jsweep] Clean validate_lockdown_requirements.cjs #39498

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
52122d0
chore: narrow POC scope to PR outcomes and safe output outcomes only …
github-actions[bot] Jun 16, 2026
0c964b4
jsweep: clean validate_lockdown_requirements.cjs
github-actions[bot] Jun 16, 2026
4052555
refactor lockdown validation messages into templates
Copilot Jun 16, 2026
22aa72f
style: move lockdown template import to file top
Copilot Jun 16, 2026
625bc90
test: focus lockdown error assertions on behavior
Copilot Jun 16, 2026
c6cd0ba
test: remove redundant pull_request_target assertion
Copilot Jun 16, 2026
6c7309c
Merge branch 'main' into signed/jsweep/validate-lockdown-requirements…
pelikhan Jun 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
90 changes: 45 additions & 45 deletions actions/setup/js/validate_lockdown_requirements.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,17 @@
* @param {any} core - GitHub Actions core library
* @returns {void}
*/
const { ERR_VALIDATION } = require("./error_codes.cjs");
function validateLockdownRequirements(core) {
/**
* @param {string} message
* @returns {never}
*/
function failWithError(message) {
core.setOutput("lockdown_check_failed", "true");
core.setFailed(message);
throw new Error(message);
}

// Check if lockdown mode is explicitly enabled (set to "true" in frontmatter)
const lockdownEnabled = process.env.GITHUB_MCP_LOCKDOWN_EXPLICIT === "true";

Expand All @@ -46,22 +55,19 @@ function validateLockdownRequirements(core) {
core.info(`Custom github-token configured: ${hasCustomToken}`);

if (!hasAnyCustomToken) {
const errorMessage =
"Lockdown mode is enabled (lockdown: true) but no custom GitHub token is configured.\\n" +
"\\n" +
"Please configure one of the following as a repository secret:\\n" +
" - GH_AW_GITHUB_TOKEN (recommended)\\n" +
" - GH_AW_GITHUB_MCP_SERVER_TOKEN (alternative)\\n" +
" - Custom github-token in your workflow frontmatter\\n" +
"\\n" +
"See: https://github.com/github/gh-aw/blob/main/docs/src/content/docs/reference/auth.mdx\\n" +
"\\n" +
"To set a token:\\n" +
' gh aw secrets set GH_AW_GITHUB_TOKEN --value "YOUR_FINE_GRAINED_PAT"';

core.setOutput("lockdown_check_failed", "true");
core.setFailed(errorMessage);
throw new Error(errorMessage);
failWithError(
`Lockdown mode is enabled (lockdown: true) but no custom GitHub token is configured.\n` +
Comment thread
pelikhan marked this conversation as resolved.
Outdated

@pelikhan pelikhan Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot refactor to template file and renderTemplate

Copilot AI Jun 16, 2026

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.

Addressed in commit 6c9ce2b. I moved the lockdown/strict/pull_request_target error text into a dedicated template module and render it via renderTemplate.

`\n` +
`Please configure one of the following as a repository secret:\n` +
` - GH_AW_GITHUB_TOKEN (recommended)\n` +
` - GH_AW_GITHUB_MCP_SERVER_TOKEN (alternative)\n` +
` - Custom github-token in your workflow frontmatter\n` +
`\n` +
`See: https://github.com/github/gh-aw/blob/main/docs/src/content/docs/reference/auth.mdx\n` +
`\n` +
`To set a token:\n` +
` gh aw secrets set GH_AW_GITHUB_TOKEN --value "YOUR_FINE_GRAINED_PAT"`
);
}

core.info("✓ Lockdown mode requirements validated: Custom GitHub token is configured");
Expand All @@ -77,20 +83,17 @@ function validateLockdownRequirements(core) {
core.info(`Compiled with strict mode: ${isStrict}`);

if (isPublic && !isStrict) {
const errorMessage =
"This workflow is running on a public repository but was not compiled with strict mode.\\n" +
"\\n" +
"Public repository workflows must be compiled with strict mode enabled to meet\\n" +
"the security requirements for public exposure.\\n" +
"\\n" +
"To fix this, recompile the workflow with strict mode:\\n" +
" gh aw compile --strict\\n" +
"\\n" +
"See: https://github.com/github/gh-aw/blob/main/docs/src/content/docs/reference/security.mdx";

core.setOutput("lockdown_check_failed", "true");
core.setFailed(errorMessage);
throw new Error(errorMessage);
failWithError(
`This workflow is running on a public repository but was not compiled with strict mode.\n` +

@pelikhan pelikhan Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot refactor to template file

Copilot AI Jun 16, 2026

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.

Addressed in commit 6c9ce2b. The strict-mode public-repo failure message now comes from the new template module and is rendered through renderTemplate.

`\n` +
`Public repository workflows must be compiled with strict mode enabled to meet\n` +
`the security requirements for public exposure.\n` +
`\n` +
`To fix this, recompile the workflow with strict mode:\n` +
` gh aw compile --strict\n` +
`\n` +
`See: https://github.com/github/gh-aw/blob/main/docs/src/content/docs/reference/security.mdx`
);
}

if (isPublic && isStrict) {
Expand All @@ -104,20 +107,17 @@ function validateLockdownRequirements(core) {
// and potentially exfiltrate secrets or cause unintended side effects.
const eventName = process.env.GITHUB_EVENT_NAME;
if (isPublic && eventName === "pull_request_target") {
const errorMessage =
"This workflow is triggered by the pull_request_target event on a public repository.\\n" +
"\\n" +
"The pull_request_target event is not allowed on public repositories because it runs\\n" +
"workflows with access to repository secrets even when triggered from a fork, which\\n" +
'creates a significant security risk (known as a "pwn request").\\n' +
"\\n" +
"To fix this, use the pull_request event instead, or migrate to a private repository.\\n" +
"\\n" +
"See: https://github.com/github/gh-aw/blob/main/docs/src/content/docs/reference/security.mdx";

core.setOutput("lockdown_check_failed", "true");
core.setFailed(errorMessage);
throw new Error(errorMessage);
failWithError(
`This workflow is triggered by the pull_request_target event on a public repository.\n` +

@pelikhan pelikhan Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot refactor to template file

Copilot AI Jun 16, 2026

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.

Addressed in commit 6c9ce2b. The pull_request_target public-repo failure message was moved to the template module and rendered through renderTemplate.

`\n` +
`The pull_request_target event is not allowed on public repositories because it runs\n` +
`workflows with access to repository secrets even when triggered from a fork, which\n` +
`creates a significant security risk (known as a "pwn request").\n` +
`\n` +
`To fix this, use the pull_request event instead, or migrate to a private repository.\n` +
`\n` +
`See: https://github.com/github/gh-aw/blob/main/docs/src/content/docs/reference/security.mdx`
);
}
}

Expand Down
82 changes: 82 additions & 0 deletions actions/setup/js/validate_lockdown_requirements.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,4 +304,86 @@ describe("validate_lockdown_requirements", () => {
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("not compiled with strict mode"));
});
});

describe("setOutput side effects", () => {
it("should not set lockdown_check_failed output on a fully successful run", () => {
// All conditions pass: no lockdown, private repo, no special event
process.env.GITHUB_REPOSITORY_VISIBILITY = "private";
process.env.GITHUB_EVENT_NAME = "push";

validateLockdownRequirements(mockCore);

expect(mockCore.setOutput).not.toHaveBeenCalled();
expect(mockCore.setFailed).not.toHaveBeenCalled();
});

it("should not set lockdown_check_failed when lockdown is enabled with token and repo is private", () => {
process.env.GITHUB_MCP_LOCKDOWN_EXPLICIT = "true";
process.env.GH_AW_GITHUB_TOKEN = "ghp_test";
process.env.GITHUB_REPOSITORY_VISIBILITY = "private";

validateLockdownRequirements(mockCore);

expect(mockCore.setOutput).not.toHaveBeenCalled();
});
});

describe("multiple tokens", () => {
it("should pass when all three tokens are configured simultaneously", () => {
process.env.GITHUB_MCP_LOCKDOWN_EXPLICIT = "true";
process.env.GH_AW_GITHUB_TOKEN = "ghp_token1";
process.env.GH_AW_GITHUB_MCP_SERVER_TOKEN = "ghp_token2";
process.env.CUSTOM_GITHUB_TOKEN = "ghp_token3";

validateLockdownRequirements(mockCore);

expect(mockCore.setFailed).not.toHaveBeenCalled();
expect(mockCore.info).toHaveBeenCalledWith("GH_AW_GITHUB_TOKEN configured: true");
expect(mockCore.info).toHaveBeenCalledWith("GH_AW_GITHUB_MCP_SERVER_TOKEN configured: true");
expect(mockCore.info).toHaveBeenCalledWith("Custom github-token configured: true");
expect(mockCore.info).toHaveBeenCalledWith("✓ Lockdown mode requirements validated: Custom GitHub token is configured");
});
});

describe("error message formatting", () => {
it("should include newlines in lockdown error message for readability", () => {
process.env.GITHUB_MCP_LOCKDOWN_EXPLICIT = "true";

expect(() => {
validateLockdownRequirements(mockCore);
}).toThrow();

@github-actions github-actions Bot Jun 16, 2026

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.

[/tdd] .toThrow() without an argument does not verify which error is thrown — inconsistent with the existing suite (line 301 uses .toThrow("not compiled with strict mode")).

💡 Match the specificity of existing tests

The pre-existing test block uses:

}).toThrow("not compiled with strict mode");

Applying the same pattern here makes all three new formatting tests consistent and catches regressions in the thrown message, not just the fact that something was thrown:

// lockdown test
}).toThrow("no custom GitHub token is configured");

// strict mode test
}).toThrow("not compiled with strict mode");

// pull_request_target test
}).toThrow("pwn request");


const errorMsg = mockCore.setFailed.mock.calls[0][0];
expect(errorMsg).toContain("\n");
expect(errorMsg).toContain("GH_AW_GITHUB_TOKEN (recommended)");
expect(errorMsg).toContain("GH_AW_GITHUB_MCP_SERVER_TOKEN (alternative)");
});

it("should include newlines in strict mode error message for readability", () => {
process.env.GITHUB_REPOSITORY_VISIBILITY = "public";
process.env.GH_AW_COMPILED_STRICT = "false";

expect(() => {
validateLockdownRequirements(mockCore);
}).toThrow();

const errorMsg = mockCore.setFailed.mock.calls[0][0];
expect(errorMsg).toContain("\n");
expect(errorMsg).toContain("gh aw compile --strict");
});

it("should include newlines in pull_request_target error message for readability", () => {
process.env.GITHUB_REPOSITORY_VISIBILITY = "public";
process.env.GH_AW_COMPILED_STRICT = "true";
process.env.GITHUB_EVENT_NAME = "pull_request_target";

expect(() => {
validateLockdownRequirements(mockCore);
}).toThrow();

const errorMsg = mockCore.setFailed.mock.calls[0][0];
expect(errorMsg).toContain("\n");
expect(errorMsg).toContain("pwn request");
});
});
});
Loading