fix: restrict bot permission bypass (#89)
## Summary - stop treating every `[bot]` actor as trusted when `allow-bots` is enabled - add `allow-bot-users` for explicit custom bot allowlists without wildcard support - document bot allowlist semantics and add CLI regression tests ## Test plan - `pnpm run check` - `pnpm test` - `git diff --check`
This commit is contained in:
@@ -30,6 +30,9 @@ jobs:
|
||||
- name: Typecheck
|
||||
run: pnpm run check
|
||||
|
||||
- name: Test
|
||||
run: pnpm test
|
||||
|
||||
- name: Build bundles
|
||||
run: pnpm run build
|
||||
|
||||
|
||||
@@ -1,2 +1,3 @@
|
||||
/node_modules/
|
||||
/.pnpm-store/
|
||||
*.log
|
||||
|
||||
@@ -113,7 +113,8 @@ jobs:
|
||||
| `safety-strategy` | Controls how the action restricts Codex privileges. See [Safety strategy](#safety-strategy). | `drop-sudo` |
|
||||
| `codex-user` | Username to run Codex as when `safety-strategy` is `unprivileged-user`. | `""` |
|
||||
| `allow-users` | List of GitHub usernames who can trigger the action in addition to those who have write access to the repo. | `""` |
|
||||
| `allow-bots` | Allow runs triggered by GitHub Apps/bot accounts to bypass the write-access check. | `false` |
|
||||
| `allow-bots` | Allow runs triggered by trusted GitHub bot accounts (`dependabot[bot]` and `github-actions[bot]`) to bypass the write-access check. | `false` |
|
||||
| `allow-bot-users` | List of GitHub bot usernames that can bypass the write-access check. `*` is not supported; list trusted bots explicitly. | `""` |
|
||||
|
||||
## Safety Strategy
|
||||
|
||||
|
||||
+7
-1
@@ -94,9 +94,13 @@ inputs:
|
||||
required: false
|
||||
default: ""
|
||||
allow-bots:
|
||||
description: "Allow runs triggered by GitHub Apps/bot accounts to bypass the write-access check."
|
||||
description: "Allow runs triggered by trusted GitHub bot accounts (dependabot[bot] and github-actions[bot]) to bypass the write-access check."
|
||||
required: false
|
||||
default: "false"
|
||||
allow-bot-users:
|
||||
description: "Comma-separated list of GitHub bot usernames that can bypass the write-access check. '*' is not supported; list trusted bots explicitly. Entries may include or omit the trailing [bot] suffix."
|
||||
required: false
|
||||
default: ""
|
||||
outputs:
|
||||
final-message:
|
||||
description: "Raw output emitted by `codex exec`."
|
||||
@@ -128,11 +132,13 @@ runs:
|
||||
ACTION_PATH: ${{ github.action_path }}
|
||||
GITHUB_TOKEN: ${{ github.token }}
|
||||
ALLOW_BOTS: ${{ inputs['allow-bots'] }}
|
||||
ALLOW_BOT_USERS: ${{ inputs['allow-bot-users'] }}
|
||||
ALLOW_USERS: ${{ inputs['allow-users'] }}
|
||||
shell: bash
|
||||
run: |
|
||||
node "$ACTION_PATH/dist/main.js" check-write-access \
|
||||
--allow-bots "$ALLOW_BOTS" \
|
||||
--allow-bot-users "$ALLOW_BOT_USERS" \
|
||||
--allow-users "$ALLOW_USERS"
|
||||
|
||||
- name: Install Codex CLI
|
||||
|
||||
Vendored
+54
-10
File diff suppressed because one or more lines are too long
+1
-1
@@ -4,7 +4,7 @@ Running Codex as part of a GitHub workflow can be a powerful tool, but it is imp
|
||||
|
||||
## Limiting who can run your workflow
|
||||
|
||||
One of the most fundamental ways to safeguard your workflow is to limit who can run it in the first place. By default, `openai/codex-action` can only be run by a user who has _write_ access to your repository. While you can expand this list via the `allow-users` and `allow-bots` options, do so with caution.
|
||||
One of the most fundamental ways to safeguard your workflow is to limit who can run it in the first place. By default, `openai/codex-action` can only be run by a user who has _write_ access to your repository. While you can expand this list via `allow-users`, allow trusted GitHub bot actors (`dependabot[bot]` and `github-actions[bot]`) via `allow-bots`, or list custom trusted bots via `allow-bot-users`, do so with caution. `allow-bot-users` does not support `*`; list each trusted bot explicitly.
|
||||
|
||||
Further, while you may design your workflow such that those with _write_ access can trigger it on content from arbitrary users (i.e., by adding a label to an issue created by an external user) such that you rely on manual approval as a means of defense, it is still important to consider other potential exploits, such as untrusted input.
|
||||
|
||||
|
||||
+3
-2
@@ -5,7 +5,8 @@
|
||||
"private": true,
|
||||
"scripts": {
|
||||
"build": "esbuild src/main.ts --sourcemap=inline --format=cjs --bundle --platform=node --target=node20 --outdir=dist",
|
||||
"check": "tsc --noEmit"
|
||||
"check": "tsc --noEmit",
|
||||
"test": "pnpm run build && node --test test/*.test.mjs"
|
||||
},
|
||||
"dependencies": {
|
||||
"@actions/core": "^1.11.1",
|
||||
@@ -19,5 +20,5 @@
|
||||
"esbuild": "^0.23.0",
|
||||
"typescript": "^5.4.5"
|
||||
},
|
||||
"packageManager": "pnpm@10.17.1"
|
||||
"packageManager": "pnpm@10.33.0+sha512.10568bb4a6afb58c9eb3630da90cc9516417abebd3fabbe6739f0ae795728da1491e9db5a544c76ad8eb7570f5c4bb3d6c637b2cb41bfdcdb47fa823c8649319"
|
||||
}
|
||||
|
||||
@@ -18,9 +18,9 @@ type EnsureWriteAccessOptions = {
|
||||
actor?: string;
|
||||
repository?: string;
|
||||
/**
|
||||
* When true (default), bot actors such as dependabot are allowed without
|
||||
* checking collaborator permissions. Set to false to require bots to pass the
|
||||
* same checks as human users.
|
||||
* When true, trusted GitHub bot actors such as dependabot are allowed without
|
||||
* checking collaborator permissions. Other bots must pass the same checks as
|
||||
* human users.
|
||||
*/
|
||||
allowBotActors?: boolean;
|
||||
/**
|
||||
@@ -28,6 +28,11 @@ type EnsureWriteAccessOptions = {
|
||||
* Case-insensitive; empty string or undefined disables this override.
|
||||
*/
|
||||
allowUsers?: string;
|
||||
/**
|
||||
* Comma-separated list of allowed GitHub bot usernames. '*' is not supported.
|
||||
* Entries may include or omit the trailing [bot] suffix.
|
||||
*/
|
||||
allowBotUsers?: string;
|
||||
};
|
||||
|
||||
/**
|
||||
@@ -39,7 +44,7 @@ export async function ensureActorHasWriteAccess(
|
||||
): Promise<WriteAccessCheck> {
|
||||
const actor = options.actor ?? process.env.GITHUB_ACTOR;
|
||||
const repository = options.repository ?? process.env.GITHUB_REPOSITORY;
|
||||
const allowBotActors = options.allowBotActors ?? true;
|
||||
const allowBotActors = options.allowBotActors ?? false;
|
||||
|
||||
if (!actor || actor.trim().length === 0) {
|
||||
return {
|
||||
@@ -66,10 +71,23 @@ export async function ensureActorHasWriteAccess(
|
||||
};
|
||||
}
|
||||
|
||||
// GitHub-built workflows (e.g. dependabot, github-actions[bot]) do not have a
|
||||
// meaningful write permission concept. They implicitly run with the token's permissions.
|
||||
if (allowBotActors && actor.endsWith("[bot]")) {
|
||||
core.info(`Actor '${actor}' is a bot account; skipping explicit permission check.`);
|
||||
// GitHub-built workflows do not have a meaningful collaborator permission
|
||||
// level. Only trust the built-in bot actors GitHub owns.
|
||||
if (allowBotActors && isTrustedGitHubBotActor(actor)) {
|
||||
core.info(`Actor '${actor}' is a trusted GitHub bot account; skipping explicit permission check.`);
|
||||
return { status: "approved", actor };
|
||||
}
|
||||
|
||||
const allowedBotActors = parseAllowedBotActors(options.allowBotUsers ?? "");
|
||||
if (allowedBotActors instanceof Error) {
|
||||
return {
|
||||
status: "rejected",
|
||||
actor,
|
||||
reason: allowedBotActors.message,
|
||||
};
|
||||
}
|
||||
if (isBotActor(actor) && allowedBotActors.has(normalizeBotActor(actor))) {
|
||||
core.info(`Actor '${actor}' is explicitly allowed via allow-bot-users.`);
|
||||
return { status: "approved", actor };
|
||||
}
|
||||
|
||||
@@ -160,6 +178,36 @@ function getTokenFromEnv(): string {
|
||||
return token && token.trim().length > 0 ? token : "";
|
||||
}
|
||||
|
||||
const TRUSTED_GITHUB_BOT_ACTORS = new Set(["dependabot[bot]", "github-actions[bot]"]);
|
||||
|
||||
function isTrustedGitHubBotActor(actor: string): boolean {
|
||||
return isBotActor(actor) && TRUSTED_GITHUB_BOT_ACTORS.has(normalizeBotActor(actor));
|
||||
}
|
||||
|
||||
function parseAllowedBotActors(allowBotUsers: string): Set<string> | Error {
|
||||
const allowed = new Set<string>();
|
||||
for (const entry of allowBotUsers.split(",")) {
|
||||
const bot = entry.trim();
|
||||
if (bot.length === 0) {
|
||||
continue;
|
||||
}
|
||||
if (bot.includes("*")) {
|
||||
return new Error("allow-bot-users does not support '*'; list trusted bot usernames explicitly.");
|
||||
}
|
||||
allowed.add(normalizeBotActor(bot));
|
||||
}
|
||||
return allowed;
|
||||
}
|
||||
|
||||
function isBotActor(actor: string): boolean {
|
||||
return actor.toLowerCase().endsWith("[bot]");
|
||||
}
|
||||
|
||||
function normalizeBotActor(actor: string): string {
|
||||
const normalized = actor.toLowerCase();
|
||||
return isBotActor(normalized) ? normalized : `${normalized}[bot]`;
|
||||
}
|
||||
|
||||
function isNotFoundError(error: unknown): boolean {
|
||||
return Boolean(
|
||||
error && typeof error === "object" && "status" in error && (error as { status?: number }).status === 404,
|
||||
|
||||
+10
-2
@@ -261,26 +261,34 @@ export async function main() {
|
||||
)
|
||||
.option(
|
||||
"--allow-bots <boolean>",
|
||||
"Allow GitHub App and bot actors to bypass the write-access check (default: true).",
|
||||
"Allow trusted GitHub bot actors to bypass the write-access check (default: false).",
|
||||
parseBoolean,
|
||||
true
|
||||
false
|
||||
)
|
||||
.option(
|
||||
"--allow-users <users>",
|
||||
"Comma-separated list of GitHub usernames who can run this action, or '*' to allow all users.",
|
||||
""
|
||||
)
|
||||
.option(
|
||||
"--allow-bot-users <users>",
|
||||
"Comma-separated list of GitHub bot usernames that can bypass the write-access check. '*' is not supported.",
|
||||
""
|
||||
)
|
||||
.action(
|
||||
async ({
|
||||
allowBots,
|
||||
allowUsers,
|
||||
allowBotUsers,
|
||||
}: {
|
||||
allowBots: boolean;
|
||||
allowUsers: string;
|
||||
allowBotUsers: string;
|
||||
}) => {
|
||||
const result = await ensureActorHasWriteAccess({
|
||||
allowBotActors: allowBots,
|
||||
allowUsers,
|
||||
allowBotUsers,
|
||||
});
|
||||
switch (result.status) {
|
||||
case "approved": {
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { spawnSync } from "node:child_process";
|
||||
import { test } from "node:test";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
const mainPath = fileURLToPath(new URL("../dist/main.js", import.meta.url));
|
||||
|
||||
function runCheckWriteAccess(actor, args = []) {
|
||||
const env = {
|
||||
...process.env,
|
||||
GITHUB_ACTOR: actor,
|
||||
GITHUB_REPOSITORY: "openai/codex-action",
|
||||
};
|
||||
delete env.GH_TOKEN;
|
||||
delete env.GITHUB_TOKEN;
|
||||
|
||||
return spawnSync(process.execPath, [mainPath, "check-write-access", ...args], {
|
||||
encoding: "utf8",
|
||||
env,
|
||||
});
|
||||
}
|
||||
|
||||
test("does not trust arbitrary bot actor suffixes", () => {
|
||||
const result = runCheckWriteAccess("openai-internal[bot]", ["--allow-bots", "true"]);
|
||||
|
||||
assert.notEqual(result.status, 0);
|
||||
assert.match(result.stderr, /A GitHub token is required/);
|
||||
});
|
||||
|
||||
test("allows trusted GitHub bot actors when enabled", () => {
|
||||
const result = runCheckWriteAccess("dependabot[bot]", ["--allow-bots", "true"]);
|
||||
|
||||
assert.equal(result.status, 0, result.stderr);
|
||||
assert.match(result.stdout, /permitted to continue/);
|
||||
});
|
||||
|
||||
test("requires explicit opt-in for trusted GitHub bot actors", () => {
|
||||
const result = runCheckWriteAccess("dependabot[bot]");
|
||||
|
||||
assert.notEqual(result.status, 0);
|
||||
assert.match(result.stderr, /A GitHub token is required/);
|
||||
});
|
||||
|
||||
test("allows custom bot actors when explicitly listed", () => {
|
||||
const result = runCheckWriteAccess("renovate[bot]", ["--allow-bot-users", "renovate"]);
|
||||
|
||||
assert.equal(result.status, 0, result.stderr);
|
||||
assert.match(result.stdout, /permitted to continue/);
|
||||
});
|
||||
|
||||
test("does not allow unlisted custom bot actors", () => {
|
||||
const result = runCheckWriteAccess("openai-internal[bot]", ["--allow-bot-users", "renovate"]);
|
||||
|
||||
assert.notEqual(result.status, 0);
|
||||
assert.match(result.stderr, /A GitHub token is required/);
|
||||
});
|
||||
|
||||
test("does not apply custom bot allowlists to human actors", () => {
|
||||
const result = runCheckWriteAccess("renovate", ["--allow-bot-users", "renovate"]);
|
||||
|
||||
assert.notEqual(result.status, 0);
|
||||
assert.match(result.stderr, /A GitHub token is required/);
|
||||
});
|
||||
|
||||
test("rejects wildcard custom bot allowlists", () => {
|
||||
const result = runCheckWriteAccess("openai-internal[bot]", ["--allow-bot-users", "*"]);
|
||||
|
||||
assert.notEqual(result.status, 0);
|
||||
assert.match(result.stderr, /allow-bot-users does not support '\*'/);
|
||||
});
|
||||
Reference in New Issue
Block a user