[yeongseon]

Summary

make security ran bandit -r src and trusted its exit status. Bandit exits 0 for an empty directory, a missing path and a file it cannot parse (they are only listed as skipped), so the required bandit check could go green without analysing anything; all three are reproduced in #578. It also never looked at examples/, scripts/ or tools/.

make security now scans src examples scripts tools in one Bandit run with every test enabled and no skip list, and fails unless Bandit analysed everything in that scope and found nothing. The eight existing findings (all LOW, all in two helper files) are suppressed by six individually reasoned inline # nosec Bxxx # <reason> markers, the only kind of exception #578 allows.

This changes the command behind a REQUIRED status check. The bandit job keeps its id (so the required context bandit is unchanged), triggers, permissions and existing steps, but what it runs (make security) is different: a new Makefile recipe plus a new guard script (scripts/check_bandit_report.py) now decide whether the check is green. A green CI run on this PR only shows the success path; the failure paths are covered by the new tests and the manual runs below. Please review the recipe, the guard and the new security.yml step before merging (AGENTS.md: independently scrutinize changes to "other high-impact security-sensitive paths").

Review finding fixed in 07a6b62. Earlier heads of this PR skipped B404, B603 and B607 for scripts tools with one blanket -s in the Makefile. The Copilot review (thread on Makefile) and a review that executed it found the consequence: a seeded tools/zz.py containing subprocess.run([sys.argv[1]], check=True) passed make security, while an unskipped Bandit reports B404 and B603 for it. That skip is gone. There is no -s, no per-directory flag and no second invocation: the helper directories are scanned exactly like src and examples, and the reviewed calls carry their own markers. A new test runs the real recipe and fails when a skip list comes back (see Verification). The review thread is left unresolved for the reviewer.

Changes

• scripts/check_bandit_report.py (new, 44 lines; unchanged by the latest four commits): reads Bandit's own JSON report and exits 1 unless all of these hold, printing one bandit: ... line per problem (explicit checks, no assert):

  • some code was analysed (loc > 0)
  • no file was skipped or unparsable (Bandit's errors list)
  • every .py file under the declared scope appears in the report (catches a scope narrowed with -x or a .bandit file's exclude, which Bandit follows silently)
  • there are no findings

  A missing or unreadable report also fails (it is never treated as clean).

• Makefile security: deletes test-results/bandit/ first, runs Bandit once over BANDIT_SCOPE := src examples scripts tools with -q -f json -o test-results/bandit/bandit.json, runs the guard with the same scope variable, and exits 1 if Bandit or the guard failed. Before this change there were two invocations (BANDIT_STRICT, BANDIT_HELPERS) with different flags; with one invocation a directory cannot get a different skip list. -q keeps the progress bar out of the log: Bandit 1.9.4 writes a Working... 100% bar to stdout above 50 files even with -o (measured earlier: 60-file scan, no -q: bar on stdout; -q: stdout and stderr empty).

• scripts/check_wheel_schemas.py (+5/-3) and tools/check_pr_format.py (+5/-3): six inline markers in the documented form (# nosec Bxxx # <reason>, IDs separated by spaces) for the eight findings that bandit --ignore-nosec -r scripts tools reports (B404 x2, B603 x4, B607 x2, all LOW severity, HIGH confidence). Nothing else is annotated, and there is no blanket # nosec.

  Line	IDs	Reason
  scripts/check_wheel_schemas.py:11	B404	runs only the temp venv's python as argument lists, no shell
  scripts/check_wheel_schemas.py:71	B603	temp-venv python, literal args plus the wheel path, no shell
  scripts/check_wheel_schemas.py:76	B603	temp-venv python, fixed smoke_code literal, no shell
  tools/check_pr_format.py:5	B404	runs only git and ruff, as argument lists, no shell
  tools/check_pr_format.py:10	B603 B607	arg list, no shell; PR revisions; git on PATH
  tools/check_pr_format.py:37	B603 B607	arg list, no shell; paths after --; ruff on PATH

  The pip install call and the ruff format --check call were one-liners; each is wrapped onto three lines only so its marker fits the 100-column line width the repository otherwise keeps (ruff itself ignores pragma comments when measuring width). No behaviour change; tests/test_check_wheel_schemas.py and tests/test_check_pr_format.py still pass.

• .github/workflows/security.yml (+8): if: always() upload of test-results/bandit/ (now one file, bandit.json), 14-day retention. actions/upload-artifact is pinned to 043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1, the pin sbom.yml already uses (git ls-remote resolves v7.0.1 to that SHA; its action.yml runs on node24). The v4 pin used by the release workflows runs on node20 and would put a deprecation annotation on a required check; tools/lint_release_workflows.py only fixes the pin in publish-pypi.yml and e2e-azure.yml. The artifact name carries github.run_attempt because artifacts are immutable per workflow run.

• .gitignore: test-results/.

• pyproject.toml (-1): removes the security Hatch script (python -m bandit -r src, no guard). Only docs/security.md referenced it, so it would have stayed a second, vacuous way to run the scan. Outside the planned file list; say so if you would rather keep it.

• tests/test_check_bandit_report.py (new): the guard cases (real Bandit, same flags as the Makefile, on scratch trees: empty directory, only empty files, missing directory, unparsable file, a file excluded by .bandit and a new shell=True call must fail with the matching reason; a clean tree, a 60-file tree and a finding suppressed inline must pass; a missing report must fail) plus four new cases that run the real make security recipe (a pass-through script stands in for hatch, true for pip) on a scratch copy of the layout it scans, with the same new code in scripts/ and tools/: clean (passes); an unannotated subprocess.run([sys.argv[1]], check=True) (fails, B404 and B603 reported for both directories); subprocess.call('echo', shell=True) (fails, B602 reported for both); the same call with reasoned inline markers (passes). They are skipped only where make or /bin/sh is missing (Windows); CI runs on ubuntu.

• docs/security.md, docs/development.md: describe the real scope, the guard and the suppression convention (they still said hatch run security and python -m bandit -r src). The convention is # nosec Bxxx # <reason> with IDs separated by spaces. Bandit 1.9.4 prints a Test in comment: <word> is not a test name or id warning for every word after the IDs unless the reason sits behind a second #, and a comma list such as B404,B602 registers only its last ID (NOSEC_COMMENT_TESTS keeps the last repetition of its group; measured, and read in bandit/core/manager.py). The - <reason> form works but is noisy, so it is not the documented one. The latest commit rewrites the paragraph that described the helper skip list.

Size: 10 files, +228/-17 (tests +142, check_bandit_report.py +44, Makefile +15/-1, docs +8/-9, security.yml +8, the two helper files +10/-6, .gitignore +1, pyproject.toml -1). Of the latest four commits (3336611, 57afe4e, 28e74d5, 07a6b62): 5 files, +81/-21.

Not in this PR: Semgrep (#579), the release advisory gate, any policy/exception file, the Makefile bootstrap rework. The 20 existing # nosec B101 markers in src/azure_functions_validation/conformance.py keep their current form (no reason text): they sit in the shipped conformance suite, so adding 20 honest reasons touches runtime source and is not a trivial comment edit; left as is (--ignore-nosec over the whole scope reports exactly those 20 plus the 8 helper findings, 28 in total, and the report's skipped_tests is 28). #568 and #569 also edit Makefile, security.yml, pyproject.toml and .gitignore; whichever lands second needs a rebase.

Verification

Local rows: Python 3.11.15, Bandit 1.9.4, Hatch 1.18.1 (worktree .venv from CI=1 make install), on head 07a6b62 (tree clean, merged with origin/main 934d60d, which had not moved: "Already up to date").

Command	Result
make check-all	exit 0: schema pin and 3 workflow lints OK; ruff check and ruff format --check OK; mypy strict src tests OK (64 files); 727 passed, 27 deselected, 4 warnings (723 on cf810eb: +4 new cases, no new warnings); coverage 97.15% (gate 95%); make security printed bandit: src examples scripts tools: 3544 lines in 33 files, no findings
make build; make docs; mkdocs build --strict	exit 0 each
pytest tests --no-cov	exit 0 on Python 3.12.13, 3.13.13 and 3.14.4 (Bandit 1.9.4; run at -qq, so no counts)
ruff check, ruff format --check on the 3 changed Python files with ruff 0.16.8 (the format job's pin) and 0.17.1 (dev extra)	OK
bandit --ignore-nosec -r scripts tools before and after the markers	the same 8 findings both times (B404 x2, B603 x4, B607 x2; LOW severity, HIGH confidence), identical file, test, severity, confidence and text; only one call's line moves (74 to 76, because the pip call was wrapped)
same without --ignore-nosec	0 findings, 0 errors, no blanket # nosec (_totals.nosec is 0), 8 ID-specific skips, stderr empty
each of the 8 (marker, ID) pairs removed on its own	exactly that one finding comes back, every time, and nothing else: no marker hides anything it was not written for
bandit --ignore-nosec -r src examples scripts tools	28 findings = 20 B101 in conformance.py (existing markers) + the 8 above; report skipped_tests 28, nosec 0
Baseline on origin/main (934d60d) before this PR	bandit -r src examples: 24 files / 2582 LOC, 0 findings; bandit -r scripts tools without any skip list: 8 files / 921 LOC, 8 LOW findings

Red then green for the reviewer's case (the real-recipe cases from 28e74d5, run first against the Makefile of cf810eb, i.e. with the helper -s B404,B603,B607, before it was edited):

Run	Result
new cases against the cf810eb recipe	new-subprocess-call-without-nosec failed: make security exited 0 for the seeded call (bandit: scripts tools: 45 lines in 5 files, no findings). clean-helpers, shell-true (B602 was never skipped) and subprocess-call-with-reasoned-nosec passed
same cases against the new recipe	14 passed in tests/test_check_bandit_report.py (10 guard + 4 real-recipe)

End-to-end make security (real Hatch and Bandit; each case created in the worktree and reverted, working tree identical afterwards):

Case	Bandit's own exit	make security	Log
clean tree	0	exit 0	bandit: src examples scripts tools: 3544 lines in 33 files, no findings
subprocess.call('echo', shell=True) in examples/	1	non-zero	B404, B607, B602 printed
same in scripts/ (helper scope)	1	non-zero	B404, B607, B602 printed (before this change only B602 was printed there)
new unannotated subprocess.run([sys.argv[1]], check=True) in tools/zz.py (the reviewed case)	1	non-zero	B404 and B603 printed (this passed make security on cf810eb)
same in scripts/zz.py	1	non-zero	B404 and B603 printed
same call in tools/zz.py with # nosec B404 # <reason> and # nosec B603 # <reason>	0	exit 0	bandit: ... 3547 lines in 34 files, no findings
unparsable .py in examples/	0	non-zero	skipped examples/zz_bad.py: syntax error while parsing AST from file
examples/ missing (renamed)	0	non-zero	skipped ./examples: No such file or directory
empty scope directory (make security BANDIT_SCOPE=<empty dir>)	0	non-zero	no code was analysed
missing scope path (BANDIT_SCOPE=<nonexistent>)	0	non-zero	no code was analysed and skipped ...: No such file or directory
scope holding only an empty .py file	0	non-zero	no code was analysed
scope narrowed by tools/.bandit (exclude = tools/lint_workflow_pins.py)	0	non-zero	not scanned: tools/lint_workflow_pins.py
60 extra files in examples/ (93 files in the scope)	0	exit 0	parsed; no progress bar anywhere in the log
clean report left from an earlier run, then Bandit refuses to run (scripts/.bandit and tools/.bandit both present)	2	non-zero	cannot read test-results/bandit/bandit.json: Expecting value: line 1 column 1 (char 0) (Bandit leaves an empty file; the stale clean one was deleted first)
clean report left from an earlier run, then Bandit unavailable (make security BANDIT=false)	n/a	non-zero	cannot read test-results/bandit/bandit.json: [Errno 2] No such file or directory; the stale report is gone

Mutation checks (original restored after each; working tree clean afterwards):

Mutation	Result
guard: loc check off / skipped-files check off / not-scanned check off / findings check off / unreadable report treated as OK, one at a time	empty-dir + only-empty-files / missing-dir + unparsable-file / narrowed-scope / shell-true-finding plus the real-recipe new-subprocess-call-without-nosec and shell-true / test_missing_report... fail
Makefile: -s B404,B603,B607 re-added to the Bandit call	real-recipe new-subprocess-call-without-nosec fails
Makefile: scripts or tools dropped from BANDIT_SCOPE	real-recipe new-subprocess-call-without-nosec and shell-true fail (the guard cannot notice this itself: it reads the same variable)
recipe without the delete-first, stale clean report, Bandit unavailable	guard reads the stale report and prints no findings; the target is still non-zero only because Bandit's own status is kept
recipe without the delete-first and ignoring Bandit's status, same situation	make security exits 0, a false green. This is the case the delete-first plus the kept status prevent

GitHub Actions on head 07a6b62 (every check-run on that exact SHA, all result pages: 29 at the time of the first read, 24 success and 5 skipped (dependabot-auto-merge, stale-branches, stale-issues), none failed, none pending):

Check	Result
required contexts ci-required, bandit, semgrep, Analyze, PR title	all success. The contexts from branch protection diffed against the successful check-runs on this head SHA: none missing. Editing this description re-runs PR title, branch-naming-validation and the stale workflows, so each of those appears more than once
Test (Py 3.11/3.12/3.13/3.14) and Minimum dependencies (Py 3.11)	727 passed, 27 deselected on each; 4 warnings on 3.11, 3.12, 3.13 and the minimum-dependencies job, 7 on 3.14 (cf810eb had 723 passed with 4 and 7 on 3.11 and 3.14); coverage 97.15% on 3.11
Ruff format check (changed Python files)	success: 4 files already formatted (ruff 0.16.8; this PR's own tools/check_pr_format.py on the 4 changed Python files)
bandit job log (Security Scans run 40103144509, attempt 1)	Run make security printed bandit: src examples scripts tools: 3544 lines in 33 files, no findings; Upload Bandit reports ended with Artifact bandit-reports-1 has been successfully uploaded! Final size is 839 bytes (one file, bandit.json); no ##[warning] annotation

Not run and known edges

• "Re-run failed jobs" of Security Scans: the run_attempt artifact name follows the documented per-run immutability, but no failed run was re-run here.
• make security on Python other than 3.11 (the CI job pins 3.11); on 3.12-3.14 only the test suite ran. macOS and Windows were not run (the real-recipe test cases are skipped on Windows).
• The weekly scheduled runs of security.yml and maintenance.yml, which also call make security.
• Known edge: Bandit scans hidden directories, so a local examples/e2e_app/.python_packages/ (gitignored; only the Azure e2e workflow's pre-install step creates it) would now be scanned by make security and its vendored code could fail the target (reproduced with a seeded file, then removed). CI's bandit job never has that directory.
• Two bypasses of the same kind, found while verifying this fix and not changed here (this PR is limited to the blanket skip; each needs a visible edit to a config or source file, but neither is caught automatically): (1) a .bandit file with skips or tests inside a scanned directory is honoured by Bandit, and the guard only notices exclude (seeded tools/.bandit with skips = B404,B603 plus the unreviewed call from above: make security exit 0, Bandit logs Found project level .bandit file and Using ini file for skipped tests); (2) a blanket # nosec without IDs suppresses silently (same call: exit 0; Bandit counts these in _totals.nosec, which is 0 today). A follow-up could pass --ini an empty file and have the guard fail when _totals.nosec is above 0.

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update
• Refactoring (no functional changes)
• Chore (maintenance, dependencies, CI, etc.)

Checklist

• My code follows the project's code style
• I have run make check (lint + typecheck) (as part of make check-all)
• I have run make test and all tests pass
• I have added tests for new functionality (if applicable)
• I have updated documentation (if applicable)
• My changes do not introduce new warnings (4 warnings before and after on Python 3.11, 7 before and after on 3.14)

Release Impact

• This change affects the public API, supported versions, or generated output
• Breaking change (describe the migration below)
• I have updated PRD.md if product scope or goals changed
• Notes the release owner should act on: none. ci: does not cut a release; hatch run security no longer exists (use make security).

Related Issues

Closes #578

Review Evidence

• Review source and reviewed commit SHA: Copilot review (advisory, COMMENTED) of de77db0, whose Makefile thread is anchored to cf810eb; a second review that executed the seeded tools/zz.py case against cf810eb. The fix is 07a6b62 (with 3336611, 57afe4e, 28e74d5); it has only the author's own verification (tables above). Independent human review requested.
• Material findings and disposition (fixed / acknowledged-wont-fix / deferred-with-issue), or "none": one finding reported twice (Copilot, and the executed case): the blanket -s B404,B603,B607 for scripts tools contradicts ci(security): Bandit gate exits 0 for empty, missing or unparsable scan scope #578's inline-# nosec wording and hides new subprocess calls. Fixed in 3336611, 57afe4e (markers), 28e74d5 (Makefile and tests) and 07a6b62 (docs). The review thread is left unresolved for the reviewer, with a reply naming the commits. Deferred, no issue yet: the two bypasses listed under known edges, and reasons for the 20 existing # nosec B101 markers.
• Unresolved blockers, or "none": the one review thread (branch protection requires conversation resolution). Do not merge before an independent review of the required-check command change.
• R3 (workflow permissions, release/publishing, security-sensitive filtering/disclosure, review-policy files): no by the R3 list (no permissions: change, no secrets, no release/publish file, no input/response filtering, no policy file). It still changes the command of a required security check, so it is flagged above for explicit independent scrutiny.

[Copilot]

🟡 Changes recommended

Helper-wide rule exclusions need reconciliation with issue #578’s requirement for reasoned inline exceptions.

1 open issue

• Avoid blanket subprocess security-check exclusions · New

What changed in this PR

This PR strengthens the required Bandit security check by expanding scan coverage and rejecting empty, incomplete, or finding-bearing scans.

Changes:

• Scan src, examples, scripts, and tools, with selected subprocess checks skipped for helpers.
• Validate Bandit JSON reports and add regression tests for clean and failing scans.
• Upload scan reports and align documentation around make security.

File	Description
tests/​test_check_bandit_report.py	Tests report validation using real Bandit scans.
scripts/​check_bandit_report.py	Rejects empty, incomplete, or finding-bearing reports.
pyproject.toml	Removes the unguarded Hatch security command.
Makefile	Expands scanning and combines scanner and guard results.
docs/​security.md	Documents scan scope, validation, and suppressions.
docs/​development.md	Distinguishes pre-commit scanning from make security.
.gitignore	Ignores generated test and scan reports.
.github/​workflows/​security.yml	Uploads Bandit reports even after failures.

_{Review effort: Balanced}

[yeongseon]

R3 authorization for head 07a6b62fc920bbdaa4c26a1ff50a299dfa6f19f3.

The maintainer delegated the merge decision to an Oracle review. The Oracle (AI) reviewed this change at head 07a6b62fc920bbdaa4c26a1ff50a299dfa6f19f3 through code inspection and independent probes and returned GO (clean make security passes; seeded subprocess and shell=True calls fail in all four scopes; empty, missing, unparsable, narrowed and stale-report cases fail closed; restoring the skip list or disabling any guard fails the tests; 727 passed 97.15%). Changes since that head are merges of main only; required checks were re-run on this exact head and are green. This is advisory AI evidence, not line-by-line human review or human approval. Closes #578. Required check bandit changed command: independently reviewed. The blanket skip is gone, exactly eight reviewed findings carry individual inline reasons, and a seeded unsuppressed subprocess call now fails in scripts, tools, examples and src. The addressed Copilot thread was resolved.