mirror of
https://github.com/pocketpaw/pocketpaw.git
synced 2026-05-13 21:21:53 +00:00
49ff6da813
Patching pocketpaw.security.guardian.get_settings fails because get_settings is imported lazily inside GuardianAgent.__init__ to avoid a circular import (config → security.url_validators → security/__init__ → guardian). Patch pocketpaw.config.get_settings (the real source) instead. Also add tests/test_logging_scrub.py to the secrets-scan exclude list alongside test_redact.py and test_pii.py — the xoxb- string is a required scrubber-test fixture, not a real credential. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
418 lines
18 KiB
YAML
418 lines
18 KiB
YAML
# PR quality gate — enforces contribution standards and security checks.
|
|
# Updated: 2026-03-05 — Fixed duplicate const bug, added security scan, dependency alerts, sensitive file alerts, PR size gate.
|
|
name: PR Quality Gate
|
|
|
|
on:
|
|
pull_request:
|
|
types: [opened, edited, synchronize, reopened]
|
|
|
|
permissions:
|
|
pull-requests: write
|
|
issues: write
|
|
contents: read
|
|
|
|
jobs:
|
|
check-quality:
|
|
runs-on: ubuntu-latest
|
|
# Skip bot PRs (Dependabot, Renovate, etc.)
|
|
if: >-
|
|
github.actor != 'dependabot[bot]' &&
|
|
github.actor != 'renovate[bot]' &&
|
|
github.actor != 'github-actions[bot]'
|
|
steps:
|
|
- name: Check PR quality
|
|
uses: actions/github-script@v7
|
|
with:
|
|
script: |
|
|
const pr = context.payload.pull_request;
|
|
const body = pr.body || '';
|
|
const title = pr.title || '';
|
|
const base = pr.base.ref;
|
|
const additions = pr.additions || 0;
|
|
const deletions = pr.deletions || 0;
|
|
const changedFiles = pr.changed_files || 0;
|
|
|
|
const issues = [];
|
|
const warnings = [];
|
|
const MARKER = '<!-- pr-quality-gate -->';
|
|
|
|
// ── Hard block: non-maintainer PRs targeting main ─────────
|
|
if (base === 'main') {
|
|
const { data: permData } = await github.rest.repos.getCollaboratorPermissionLevel({
|
|
owner: context.repo.owner,
|
|
repo: context.repo.repo,
|
|
username: pr.user.login
|
|
});
|
|
const perm = permData.permission;
|
|
const isMaintainer = perm === 'admin' || perm === 'maintain' || perm === 'write';
|
|
|
|
if (!isMaintainer) {
|
|
const message = `${MARKER}\n` +
|
|
`## PR targets \`main\` — closing automatically\n\n` +
|
|
`Only maintainers can open PRs against \`main\`. All other PRs must target the \`dev\` branch.\n\n` +
|
|
`Please retarget your PR to \`dev\`:\n` +
|
|
`\`\`\`bash\n` +
|
|
`gh pr edit ${pr.number} --base dev\n` +
|
|
`\`\`\`\n\n` +
|
|
`See [CONTRIBUTING.md](../blob/dev/CONTRIBUTING.md#branch-strategy) for details.`;
|
|
|
|
await github.rest.issues.createComment({
|
|
owner: context.repo.owner,
|
|
repo: context.repo.repo,
|
|
issue_number: pr.number,
|
|
body: message
|
|
});
|
|
|
|
await github.rest.pulls.update({
|
|
owner: context.repo.owner,
|
|
repo: context.repo.repo,
|
|
pull_number: pr.number,
|
|
state: 'closed'
|
|
});
|
|
|
|
return;
|
|
}
|
|
}
|
|
|
|
// ── Reopen guard: block reopening of previously closed PRs ──
|
|
if (context.payload.action === 'reopened') {
|
|
const { data: prComments } = await github.rest.issues.listComments({
|
|
owner: context.repo.owner,
|
|
repo: context.repo.repo,
|
|
issue_number: pr.number,
|
|
});
|
|
|
|
const wasBotClosed = prComments.some(
|
|
c => c.user.type === 'Bot' && c.body.includes(MARKER) && c.body.includes('closing')
|
|
);
|
|
|
|
const { data: events } = await github.rest.issues.listEventsForTimeline({
|
|
owner: context.repo.owner,
|
|
repo: context.repo.repo,
|
|
issue_number: pr.number,
|
|
per_page: 100
|
|
});
|
|
|
|
const maintainerClosed = events.some(e =>
|
|
e.event === 'closed' &&
|
|
e.actor &&
|
|
e.actor.login !== pr.user.login &&
|
|
e.actor.type !== 'Bot'
|
|
);
|
|
|
|
if (wasBotClosed || maintainerClosed) {
|
|
const reason = wasBotClosed
|
|
? 'This PR was previously closed by the quality gate bot.'
|
|
: 'This PR was previously closed by a maintainer.';
|
|
|
|
await github.rest.issues.createComment({
|
|
owner: context.repo.owner,
|
|
repo: context.repo.repo,
|
|
issue_number: pr.number,
|
|
body: `${MARKER}\n## Reopened PR closed automatically\n\n${reason} Please don't reopen closed PRs — if you've addressed the feedback, open a new PR instead.\n\nSee [CONTRIBUTING.md](../blob/dev/CONTRIBUTING.md) for guidelines.`
|
|
});
|
|
|
|
await github.rest.pulls.update({
|
|
owner: context.repo.owner,
|
|
repo: context.repo.repo,
|
|
pull_number: pr.number,
|
|
state: 'closed'
|
|
});
|
|
|
|
return;
|
|
}
|
|
}
|
|
|
|
// Check for conventional commit title (feat:, fix:, docs:, etc.)
|
|
const conventionalRe = /^(feat|fix|docs|style|refactor|perf|test|build|ci|chore|revert)(\(.+\))?!?:\s.+/;
|
|
if (!conventionalRe.test(title)) {
|
|
issues.push('- PR title does not follow [Conventional Commits](https://www.conventionalcommits.org/) format (e.g. `feat: add login page`, `fix(auth): handle expired tokens`).');
|
|
}
|
|
|
|
// ── Linked issue ────────────────────────────────────────────
|
|
const hasIssueRef = /(?:fixes|closes|resolves)\s*#\d+/i.test(body) ||
|
|
/#\d+/.test(body);
|
|
if (!hasIssueRef) {
|
|
issues.push('- No linked issue found. PRs should reference an issue (`Fixes #123`). Open an issue first if one doesn\'t exist.');
|
|
}
|
|
|
|
// ── Description quality ─────────────────────────────────────
|
|
const strippedBody = body
|
|
.replace(/<!--[\s\S]*?-->/g, '')
|
|
.replace(/#+\s*.*/g, '')
|
|
.replace(/- \[ \].*/g, '')
|
|
.replace(/\s/g, '');
|
|
if (strippedBody.length < 50) {
|
|
issues.push('- PR description is too short. Please describe what this PR does and how to test it.');
|
|
}
|
|
|
|
// ── Testing evidence ────────────────────────────────────────
|
|
const hasTestEvidence = /terminal|output|screenshot|tested|test result/i.test(body) ||
|
|
/```[\s\S]*```/.test(body);
|
|
if (!hasTestEvidence) {
|
|
issues.push('- No evidence of local testing found. Please include terminal output or screenshots.');
|
|
}
|
|
|
|
// ── Get changed file list from API ──────────────────────────
|
|
const { data: files } = await github.rest.pulls.listFiles({
|
|
owner: context.repo.owner,
|
|
repo: context.repo.repo,
|
|
pull_number: pr.number,
|
|
per_page: 100
|
|
});
|
|
const fileNames = files.map(f => f.filename);
|
|
|
|
// ── Unrelated changes detection ─────────────────────────────
|
|
const hasReadmeChange = fileNames.some(f => f === 'README.md');
|
|
const hasSourceChange = fileNames.some(f => f.startsWith('src/'));
|
|
const hasTestChange = fileNames.some(f => f.startsWith('tests/'));
|
|
|
|
if (hasReadmeChange && hasSourceChange && !title.toLowerCase().includes('docs')) {
|
|
warnings.push('- This PR modifies both source code and README.md. If the README change is unrelated to the code change, please split it into a separate PR.');
|
|
}
|
|
|
|
// ── PR size gate ────────────────────────────────────────────
|
|
const totalLines = additions + deletions;
|
|
if (totalLines > 500 || changedFiles > 15) {
|
|
warnings.push(`- Large PR detected (${totalLines} lines across ${changedFiles} files). Consider splitting into smaller, focused PRs for easier review.`);
|
|
}
|
|
|
|
// ── Sensitive file change alerts ────────────────────────────
|
|
const sensitivePatterns = [
|
|
{ pattern: /^\.github\//, label: 'CI/CD workflows' },
|
|
{ pattern: /^src\/pocketpaw\/security\//, label: 'security subsystem' },
|
|
{ pattern: /^src\/pocketpaw\/config\.py$/, label: 'app configuration' },
|
|
{ pattern: /^src\/pocketpaw\/api\/oauth2\//, label: 'OAuth/auth' },
|
|
{ pattern: /^pyproject\.toml$/, label: 'project dependencies' },
|
|
{ pattern: /^uv\.lock$/, label: 'lock file' },
|
|
];
|
|
const touchedSensitive = [];
|
|
for (const { pattern, label } of sensitivePatterns) {
|
|
if (fileNames.some(f => pattern.test(f))) {
|
|
touchedSensitive.push(label);
|
|
}
|
|
}
|
|
if (touchedSensitive.length > 0) {
|
|
warnings.push(`- This PR touches sensitive files (${touchedSensitive.join(', ')}). These paths require maintainer review via CODEOWNERS.`);
|
|
}
|
|
|
|
// ── Dependency change alert ─────────────────────────────────
|
|
const depFiles = fileNames.filter(f => f === 'pyproject.toml' || f === 'uv.lock');
|
|
if (depFiles.length > 0) {
|
|
const depChanges = files.filter(f => depFiles.includes(f.filename));
|
|
const depDetail = depChanges.map(f => `\`${f.filename}\` (+${f.additions}/-${f.deletions})`).join(', ');
|
|
warnings.push(`- Dependency files changed: ${depDetail}. Reviewers: check for added/removed/changed packages.`);
|
|
}
|
|
|
|
// ── Find existing bot comment ───────────────────────────────
|
|
const { data: comments } = await github.rest.issues.listComments({
|
|
owner: context.repo.owner,
|
|
repo: context.repo.repo,
|
|
issue_number: pr.number,
|
|
});
|
|
const botComment = comments.find(
|
|
c => c.user.type === 'Bot' && c.body.includes(MARKER)
|
|
);
|
|
|
|
if (issues.length > 0 || warnings.length > 0) {
|
|
let message = `${MARKER}\n`;
|
|
|
|
if (issues.length > 0) {
|
|
// Add needs-work label
|
|
await github.rest.issues.addLabels({
|
|
owner: context.repo.owner,
|
|
repo: context.repo.repo,
|
|
issue_number: pr.number,
|
|
labels: ['needs-work']
|
|
});
|
|
|
|
message += `### Issues (must fix)\n\n${issues.join('\n')}\n\n`;
|
|
}
|
|
|
|
if (warnings.length > 0) {
|
|
message += `### Heads up\n\n${warnings.join('\n')}\n\n`;
|
|
}
|
|
|
|
message += `Please update your PR to address these points. PRs that don't meet contribution standards will be closed after 7 days.`;
|
|
|
|
if (botComment) {
|
|
await github.rest.issues.updateComment({
|
|
owner: context.repo.owner,
|
|
repo: context.repo.repo,
|
|
comment_id: botComment.id,
|
|
body: message
|
|
});
|
|
} else {
|
|
await github.rest.issues.createComment({
|
|
owner: context.repo.owner,
|
|
repo: context.repo.repo,
|
|
issue_number: pr.number,
|
|
body: message
|
|
});
|
|
}
|
|
} else {
|
|
// All checks pass
|
|
try {
|
|
await github.rest.issues.removeLabel({
|
|
owner: context.repo.owner,
|
|
repo: context.repo.repo,
|
|
issue_number: pr.number,
|
|
name: 'needs-work'
|
|
});
|
|
} catch (e) { /* label wasn't present */ }
|
|
|
|
if (botComment) {
|
|
await github.rest.issues.updateComment({
|
|
owner: context.repo.owner,
|
|
repo: context.repo.repo,
|
|
comment_id: botComment.id,
|
|
body: `${MARKER}\nAll quality checks passed. Thanks for the clean PR!`
|
|
});
|
|
}
|
|
}
|
|
|
|
security-scan:
|
|
runs-on: ubuntu-latest
|
|
if: >-
|
|
github.actor != 'dependabot[bot]' &&
|
|
github.actor != 'renovate[bot]' &&
|
|
github.actor != 'github-actions[bot]'
|
|
steps:
|
|
- name: Checkout PR code
|
|
uses: actions/checkout@v4
|
|
with:
|
|
fetch-depth: 0
|
|
|
|
- name: Get changed files
|
|
id: changed
|
|
run: |
|
|
FILES=$(gh pr diff ${{ github.event.pull_request.number }} --name-only 2>/dev/null || git diff --name-only origin/${{ github.event.pull_request.base.ref }}...HEAD)
|
|
echo "files<<EOF" >> "$GITHUB_OUTPUT"
|
|
echo "$FILES" >> "$GITHUB_OUTPUT"
|
|
echo "EOF" >> "$GITHUB_OUTPUT"
|
|
env:
|
|
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
|
|
|
- name: Scan for dangerous code patterns
|
|
run: |
|
|
CHANGED_PY=$(echo "${{ steps.changed.outputs.files }}" | grep '\.py$' || true)
|
|
if [ -z "$CHANGED_PY" ]; then
|
|
echo "No Python files changed — skipping security scan."
|
|
exit 0
|
|
fi
|
|
|
|
echo "Scanning changed Python files for dangerous patterns..."
|
|
FOUND=0
|
|
REPORT=""
|
|
|
|
# Patterns that indicate potential code injection or unsafe operations
|
|
PATTERNS=(
|
|
'eval\s*\('
|
|
'exec\s*\('
|
|
'os\.system\s*\('
|
|
'subprocess\..*shell\s*=\s*True'
|
|
'__import__\s*\('
|
|
'pickle\.loads?\s*\('
|
|
'yaml\.load\s*\([^)]*$'
|
|
'yaml\.load\s*\([^)]*\)\s*$'
|
|
'marshal\.loads?\s*\('
|
|
'shelve\.open\s*\('
|
|
'compile\s*\([^)]*exec'
|
|
)
|
|
|
|
for file in $CHANGED_PY; do
|
|
if [ ! -f "$file" ]; then
|
|
continue
|
|
fi
|
|
for pattern in "${PATTERNS[@]}"; do
|
|
MATCHES=$(grep -nE "$pattern" "$file" 2>/dev/null || true)
|
|
if [ -n "$MATCHES" ]; then
|
|
FOUND=1
|
|
REPORT+="### $file\n"
|
|
REPORT+='```\n'
|
|
REPORT+="$MATCHES\n"
|
|
REPORT+='```\n\n'
|
|
fi
|
|
done
|
|
done
|
|
|
|
if [ "$FOUND" -eq 1 ]; then
|
|
echo "::warning::Dangerous code patterns detected in changed files. See details below."
|
|
echo -e "## Security scan: patterns detected\n\nThe following potentially dangerous patterns were found in changed files. This is not necessarily a problem — some patterns have legitimate uses — but a maintainer should verify.\n\n$REPORT"
|
|
|
|
# Post as a PR comment
|
|
COMMENT_BODY=$(cat <<'COMMENT_EOF'
|
|
<!-- security-scan -->
|
|
## Security scan: review needed
|
|
|
|
Potentially dangerous code patterns detected in changed files. A maintainer should verify these are intentional and safe.
|
|
|
|
COMMENT_EOF
|
|
)
|
|
COMMENT_BODY+=$(echo -e "$REPORT")
|
|
|
|
gh pr comment ${{ github.event.pull_request.number }} \
|
|
--body "$COMMENT_BODY" \
|
|
--edit-last 2>/dev/null || \
|
|
gh pr comment ${{ github.event.pull_request.number }} \
|
|
--body "$COMMENT_BODY"
|
|
|
|
# Don't fail the check — just warn. Maintainers review via CODEOWNERS.
|
|
echo "::warning::Security patterns found. Review required."
|
|
else
|
|
echo "No dangerous patterns found."
|
|
fi
|
|
env:
|
|
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
|
|
|
- name: Check for secrets in diff
|
|
run: |
|
|
DIFF=$(git diff origin/${{ github.event.pull_request.base.ref }}...HEAD -- . ':!uv.lock' ':!*.lock' ':!src/pocketpaw/security/redact.py' ':!tests/test_redact.py' ':!tests/test_pii.py' ':!tests/test_logging_scrub.py' 2>/dev/null || true)
|
|
if [ -z "$DIFF" ]; then
|
|
echo "No diff to scan."
|
|
exit 0
|
|
fi
|
|
|
|
FOUND=0
|
|
# Common secret patterns
|
|
PATTERNS=(
|
|
'AKIA[0-9A-Z]{16}' # AWS Access Key
|
|
'sk-[a-zA-Z0-9]{20,}' # OpenAI/Anthropic API key
|
|
'ghp_[a-zA-Z0-9]{36}' # GitHub PAT
|
|
'gho_[a-zA-Z0-9]{36}' # GitHub OAuth
|
|
'xoxb-[0-9]+-[a-zA-Z0-9]+' # Slack bot token
|
|
'xoxp-[0-9]+-[a-zA-Z0-9]+' # Slack user token
|
|
'sk_live_[a-zA-Z0-9]+' # Stripe secret key
|
|
'-----BEGIN .*PRIVATE KEY-----' # Private keys
|
|
)
|
|
|
|
for pattern in "${PATTERNS[@]}"; do
|
|
if echo "$DIFF" | grep -qE "$pattern"; then
|
|
FOUND=1
|
|
echo "::error::Potential secret detected matching pattern: $pattern"
|
|
fi
|
|
done
|
|
|
|
if [ "$FOUND" -eq 1 ]; then
|
|
gh pr comment ${{ github.event.pull_request.number }} \
|
|
--body "<!-- security-scan-secrets -->
|
|
## Security scan: potential secrets detected
|
|
|
|
This PR diff appears to contain secrets or API keys. Please remove them immediately and rotate any exposed credentials.
|
|
|
|
If these are false positives (e.g., placeholder patterns in tests), a maintainer will verify." \
|
|
--edit-last 2>/dev/null || \
|
|
gh pr comment ${{ github.event.pull_request.number }} \
|
|
--body "<!-- security-scan-secrets -->
|
|
## Security scan: potential secrets detected
|
|
|
|
This PR diff appears to contain secrets or API keys. Please remove them immediately and rotate any exposed credentials.
|
|
|
|
If these are false positives (e.g., placeholder patterns in tests), a maintainer will verify."
|
|
exit 1
|
|
fi
|
|
|
|
echo "No secrets detected in diff."
|
|
env:
|
|
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|