Skip to content

fix(security): reduce prompt injection guard false positives #126

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
stranske merged 2 commits into from
Dec 24, 2025
Merged

fix(security): reduce prompt injection guard false positives #126

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a1d526
fix(security): drastically reduce false positives in prompt injection…
stranske Dec 24, 2025
53d7b46
test: update prompt injection guard tests for minimal patterns
stranske Dec 24, 2025
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
60 changes: 27 additions & 33 deletions .github/scripts/prompt_injection_guard.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,45 +21,24 @@ const DEFAULT_ALLOWED_BOTS = [
'renovate[bot]',
];

// MINIMAL red-flag patterns - only catch OBVIOUS injection attempts
// Collaborators bypass content scanning entirely, so this only matters for forks/strangers
const DEFAULT_RED_FLAG_PATTERNS = [
// Instruction override attempts
/ignore\s+(all\s+)?(previous|prior|above)\s+(instructions?|prompts?|rules?)/i,
/disregard\s+(all\s+)?(previous|prior|above)/i,
/forget\s+(everything|all)\s+(you\s+)?(know|learned)/i,
/new\s+instructions?:\s*$/im,
// Explicit injection attempts (very specific phrases)
/ignore\s+all\s+previous\s+instructions/i,
/disregard\s+all\s+previous/i,
/system\s*:\s*you\s+are\s+now/i,
/\bpretend\s+you\s+are\b/i,
/\bact\s+as\s+(if\s+you\s+are\s+)?a?\s*(different|new)/i,

// Hidden content / obfuscation
/<!--[\s\S]*?(ignore|instruction|prompt|secret|token|password)[\s\S]*?-->/i,
/\[comment\]:\s*#/i, // GitHub markdown comments

// Base64 encoded content (potential hidden instructions)
/[A-Za-z0-9+/]{50,}={0,2}/,

// Unicode tricks and homoglyphs
/[\u200B-\u200D\uFEFF]/, // Zero-width characters
/[\u2060-\u2064]/, // Invisible formatting
/[\u00AD]/, // Soft hyphen (invisible)

// Dangerous shell/code patterns that might be injected
/\$\(\s*curl\b/i,
/\beval\s*\(/i,
/`[^`]*\$\{/,

// Secrets/credentials patterns
/\b(api[_-]?key|secret|token|password|credential)s?\s*[:=]\s*['"]?[A-Za-z0-9_-]{20,}/i,
// Actual leaked secrets (not the word "secret", but actual tokens)
/\bghp_[A-Za-z0-9]{36}\b/, // GitHub personal access token
/\bgho_[A-Za-z0-9]{36}\b/, // GitHub OAuth token
/\bgho_[A-Za-z0-9]{36}\b/, // GitHub OAuth token

Copilot AI Dec 24, 2025

Copy link

Choose a reason for hiding this comment

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

Trailing whitespace found at the end of this line. This should be removed for consistency with code style standards.

Suggested change
/\bgho_[A-Za-z0-9]{36}\b/, // GitHub OAuth token
/\bgho_[A-Za-z0-9]{36}\b/, // GitHub OAuth token

Copilot uses AI. Check for mistakes.
/\bghs_[A-Za-z0-9]{36}\b/, // GitHub app token
/\bsk-[A-Za-z0-9]{48}\b/, // OpenAI API key pattern

// Workflow/action manipulation
/\bgithub\.event\.pull_request\.head\.sha\b/,
/\bsecrets\.[A-Z_]+\b/,
];

// Label that bypasses security gate content scanning

Copilot AI Dec 24, 2025

Copy link

Choose a reason for hiding this comment

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

The comment says "Label that bypasses security gate content scanning" but based on the implementation at lines 314-325, the bypass label actually bypasses all security checks including fork detection and collaborator checks, not just content scanning. The comment should be updated to accurately reflect that it bypasses the entire security gate, or the implementation should be changed to only bypass content scanning as the comment suggests.

Suggested change
// Label that bypasses security gate content scanning
// Label that bypasses the entire security gate (fork detection, collaborator checks, and content scanning)

Copilot uses AI. Check for mistakes.
const BYPASS_LABEL = 'security:bypass-guard';

// ---------------------------------------------------------------------------
// Fork detection
// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -332,8 +311,22 @@ async function evaluatePromptInjectionGuard({
}
}

// 4. Content red-flag scanning
if (scanContent && promptContent) {
// 4. Check for bypass label
const prLabels = (pr?.labels || []).map(l => (typeof l === 'string' ? l : l.name || '').toLowerCase());
const hasBypassLabel = prLabels.includes(BYPASS_LABEL.toLowerCase());
if (hasBypassLabel) {
if (core) core.info(`Security gate bypassed via ${BYPASS_LABEL} label`);
return {
allowed: true,
blocked: false,
reason: 'bypass-label',
details,
};
}
Comment on lines +314 to +325

Copilot AI Dec 24, 2025

Copy link

Choose a reason for hiding this comment

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

The bypass label check occurs after fork and collaborator checks, which means forked PRs and non-collaborators will be blocked before the bypass label can take effect. If the bypass label is intended to override all security checks (including fork blocking), it should be moved earlier in the evaluation logic, right after extracting the PR labels and before the fork detection check.

Copilot uses AI. Check for mistakes.

// 5. Content red-flag scanning - SKIP for collaborators (they're trusted)
const isCollaborator = details.actor.allowed || details.collaborator.isCollaborator;
if (scanContent && promptContent && !isCollaborator) {
Comment on lines +327 to +329

@chatgpt-codex-connector chatgpt-codex-connector Bot Dec 24, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Re-enable content scanning when allowlist is empty

The new collaborator skip at lines 327‑329 uses details.actor.allowed to decide whether to scan, but validateActorAllowList returns allowed: true whenever no explicit allowlist is configured (the default). As a result isCollaborator is always true in the default configuration and scanForRedFlags never runs, effectively disabling all prompt scanning for every actor (including fork contributors if blockForks is set to false or any non-collaborator trigger) — a regression from the previous behavior where content was always scanned. This removes the main protection against prompt injections in the default setup.

Useful? React with 👍 / 👎.

Comment on lines +314 to +329

Copilot AI Dec 24, 2025

Copy link

Choose a reason for hiding this comment

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

The newly introduced bypass label mechanism and the updated collaborator-based content scanning skip logic lack test coverage. Since the test file exists at .github/scripts/__tests__/prompt-injection-guard.test.js and tests other functions in this module, these new behaviors should have corresponding tests to verify: 1) the bypass label properly allows PRs through, 2) collaborators skip content scanning, and 3) non-collaborators still undergo content scanning.

Copilot uses AI. Check for mistakes.
details.content = scanForRedFlags(promptContent);

if (details.content.flagged) {
Expand Down Expand Up @@ -378,4 +371,5 @@ module.exports = {
// Constants for testing/customization
DEFAULT_ALLOWED_BOTS,
DEFAULT_RED_FLAG_PATTERNS,
BYPASS_LABEL,
};
Loading