SECURITY FIX: Addresses authorization_bypass vulnerability (LOW severity) The allowed_non_write_users='*' configuration previously bypassed write permission checks for all users with only a warning. This created a security misconfiguration risk. Changes: - Added new input 'bypass_write_permission_check_acknowledgment' required when using wildcard (*) - Modified checkWritePermissions() to throw error if wildcard used without explicit acknowledgment flag - Updated all documentation (security.md, usage.md) with new requirement - Updated example workflows to include acknowledgment flag - Added tests for new validation behavior This prevents accidental security misconfigurations while maintaining the feature for intentional use cases like issue triage workflows. Affected file: src/github/validation/permissions.ts:27 Category: authorization_bypass Severity: LOW
87 lines
3.5 KiB
TypeScript
87 lines
3.5 KiB
TypeScript
import * as core from "@actions/core";
|
|
import type { ParsedGitHubContext } from "../context";
|
|
import type { Octokit } from "@octokit/rest";
|
|
|
|
/**
|
|
* Check if the actor has write permissions to the repository
|
|
* @param octokit - The Octokit REST client
|
|
* @param context - The GitHub context
|
|
* @param allowedNonWriteUsers - Comma-separated list of users allowed without write permissions, or '*' for all
|
|
* @param githubTokenProvided - Whether github_token was provided as input (not from app)
|
|
* @param bypassAcknowledgment - Explicit acknowledgment required when using wildcard (*)
|
|
* @returns true if the actor has write permissions, false otherwise
|
|
*/
|
|
export async function checkWritePermissions(
|
|
octokit: Octokit,
|
|
context: ParsedGitHubContext,
|
|
allowedNonWriteUsers?: string,
|
|
githubTokenProvided?: boolean,
|
|
bypassAcknowledgment?: boolean,
|
|
): Promise<boolean> {
|
|
const { repository, actor } = context;
|
|
|
|
try {
|
|
core.info(`Checking permissions for actor: ${actor}`);
|
|
|
|
// Check if we should bypass permission checks for this user
|
|
if (allowedNonWriteUsers && githubTokenProvided) {
|
|
const allowedUsers = allowedNonWriteUsers.trim();
|
|
if (allowedUsers === "*") {
|
|
if (!bypassAcknowledgment) {
|
|
core.error(
|
|
`❌ SECURITY ERROR: Attempting to bypass write permission checks for all users with allowed_non_write_users='*' without explicit acknowledgment. ` +
|
|
`This is a critical security misconfiguration. To proceed, you must set bypass_write_permission_check_acknowledgment='true' ` +
|
|
`to explicitly acknowledge the security implications.`,
|
|
);
|
|
throw new Error(
|
|
"Cannot bypass write permission checks with wildcard (*) without explicit acknowledgment. " +
|
|
"Set bypass_write_permission_check_acknowledgment='true' to acknowledge security implications.",
|
|
);
|
|
}
|
|
core.warning(
|
|
`⚠️ SECURITY WARNING: Bypassing write permission check for ${actor} due to allowed_non_write_users='*'. This should only be used for workflows with very limited permissions.`,
|
|
);
|
|
return true;
|
|
} else if (allowedUsers) {
|
|
const allowedUserList = allowedUsers
|
|
.split(",")
|
|
.map((u) => u.trim())
|
|
.filter((u) => u.length > 0);
|
|
if (allowedUserList.includes(actor)) {
|
|
core.warning(
|
|
`⚠️ SECURITY WARNING: Bypassing write permission check for ${actor} due to allowed_non_write_users configuration. This should only be used for workflows with very limited permissions.`,
|
|
);
|
|
return true;
|
|
}
|
|
}
|
|
}
|
|
|
|
// Check if the actor is a GitHub App (bot user)
|
|
if (actor.endsWith("[bot]")) {
|
|
core.info(`Actor is a GitHub App: ${actor}`);
|
|
return true;
|
|
}
|
|
|
|
// Check permissions directly using the permission endpoint
|
|
const response = await octokit.repos.getCollaboratorPermissionLevel({
|
|
owner: repository.owner,
|
|
repo: repository.repo,
|
|
username: actor,
|
|
});
|
|
|
|
const permissionLevel = response.data.permission;
|
|
core.info(`Permission level retrieved: ${permissionLevel}`);
|
|
|
|
if (permissionLevel === "admin" || permissionLevel === "write") {
|
|
core.info(`Actor has write access: ${permissionLevel}`);
|
|
return true;
|
|
} else {
|
|
core.warning(`Actor has insufficient permissions: ${permissionLevel}`);
|
|
return false;
|
|
}
|
|
} catch (error) {
|
|
core.error(`Failed to check permissions: ${error}`);
|
|
throw new Error(`Failed to check permissions for ${actor}: ${error}`);
|
|
}
|
|
}
|