diff --git a/.github/workflows/bandit.yml b/.github/workflows/bandit.yml new file mode 100644 index 000000000..6db7276da --- /dev/null +++ b/.github/workflows/bandit.yml @@ -0,0 +1,35 @@ +name: bandit + +on: + push: + branches: + - develop + - main + pull_request: + branches: + - develop + - main + +permissions: + contents: read + +env: + GIT_CONFIG_COUNT: "1" + GIT_CONFIG_KEY_0: init.defaultBranch + GIT_CONFIG_VALUE_0: develop + +jobs: + bandit-scan: + name: Bandit Security Scan + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + - uses: astral-sh/setup-uv@11f9893b081a58869d3b5fccaea48c9e9e46f990 # v8.3.2 + with: + version: "0.8.6" + enable-cache: false + - name: Sync Python dependencies + run: uv sync --project services/analysis-engine --group dev --frozen + - name: Run Bandit + working-directory: services/analysis-engine + run: uv run bandit -c pyproject.toml -r src diff --git a/.github/workflows/build-baseline.yml b/.github/workflows/build-baseline.yml index 13de8e648..abec57b6b 100644 --- a/.github/workflows/build-baseline.yml +++ b/.github/workflows/build-baseline.yml @@ -12,12 +12,6 @@ on: tags: - "v*" -concurrency: - group: >- - ${{ github.workflow }}-${{ github.repository }}-${{ - github.event_name == 'pull_request' && github.event.pull_request.number || github.run_id }} - cancel-in-progress: ${{ github.event_name == 'pull_request' }} - permissions: contents: read @@ -294,11 +288,9 @@ jobs: - name: Explain non-blocking macOS amd64 artifact upload failure if: ${{ steps.upload-macos-amd64.outcome == 'failure' }} run: | - { - echo "Artifact upload failed after the macOS amd64 bundle was packaged." - echo "Pull request builds keep artifact upload non-blocking because GitHub artifact service or DNS failures do not invalidate the build evidence." - echo "Tag and release builds remain blocking because release publication requires uploaded artifacts." - } >> "$GITHUB_STEP_SUMMARY" + echo "Artifact upload failed after the macOS amd64 bundle was packaged." >> "$GITHUB_STEP_SUMMARY" + echo "Pull request builds keep artifact upload non-blocking because GitHub artifact service or DNS failures do not invalidate the build evidence." >> "$GITHUB_STEP_SUMMARY" + echo "Tag and release builds remain blocking because release publication requires uploaded artifacts." >> "$GITHUB_STEP_SUMMARY" build-macos-arm64: name: build / macos / arm64 @@ -358,11 +350,9 @@ jobs: - name: Explain non-blocking macOS arm64 artifact upload failure if: ${{ steps.upload-macos-arm64.outcome == 'failure' }} run: | - { - echo "Artifact upload failed after the macOS arm64 bundle was packaged." - echo "Pull request builds keep artifact upload non-blocking because GitHub artifact service or DNS failures do not invalidate the build evidence." - echo "Tag and release builds remain blocking because release publication requires uploaded artifacts." - } >> "$GITHUB_STEP_SUMMARY" + echo "Artifact upload failed after the macOS arm64 bundle was packaged." >> "$GITHUB_STEP_SUMMARY" + echo "Pull request builds keep artifact upload non-blocking because GitHub artifact service or DNS failures do not invalidate the build evidence." >> "$GITHUB_STEP_SUMMARY" + echo "Tag and release builds remain blocking because release publication requires uploaded artifacts." >> "$GITHUB_STEP_SUMMARY" gate-macos: name: gate / build / macos diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6e743c2ff..d17468129 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,12 +10,6 @@ on: - develop - main -concurrency: - group: >- - ${{ github.workflow }}-${{ github.repository }}-${{ - github.event_name == 'pull_request' && github.event.pull_request.number || github.run_id }} - cancel-in-progress: ${{ github.event_name == 'pull_request' }} - permissions: contents: read diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 000000000..27c5b540f --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,39 @@ +name: codeql + +on: + push: + branches: + - develop + - main + workflow_dispatch: + +permissions: + actions: read + contents: read + +env: + GIT_CONFIG_COUNT: "1" + GIT_CONFIG_KEY_0: init.defaultBranch + GIT_CONFIG_VALUE_0: develop + +jobs: + analyze: + name: codeql + runs-on: ubuntu-latest + permissions: + actions: read + contents: read + security-events: write + strategy: + fail-fast: false + matrix: + language: + - javascript-typescript + - python + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + - uses: github/codeql-action/init@99df26d4f13ea111d4ec1a7dddef6063f76b97e9 # v4.37.0 + with: + languages: ${{ matrix.language }} + - uses: github/codeql-action/autobuild@99df26d4f13ea111d4ec1a7dddef6063f76b97e9 # v4.37.0 + - uses: github/codeql-action/analyze@99df26d4f13ea111d4ec1a7dddef6063f76b97e9 # v4.37.0 diff --git a/.github/workflows/ossf-scorecard.yml b/.github/workflows/ossf-scorecard.yml index 8f5b1bc25..2a4b6eaa9 100644 --- a/.github/workflows/ossf-scorecard.yml +++ b/.github/workflows/ossf-scorecard.yml @@ -9,10 +9,6 @@ on: - develop - main -concurrency: - group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event_name }}-${{ github.ref }} - cancel-in-progress: false - permissions: read-all jobs: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index aa69a973c..34583b414 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1,6 +1,10 @@ name: release on: + pull_request: + branches: + - develop + - main push: branches: - develop @@ -9,10 +13,6 @@ on: - "v*" workflow_dispatch: -concurrency: - group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event_name }}-${{ github.ref }} - cancel-in-progress: false - permissions: contents: read diff --git a/.github/workflows/sbom.yml b/.github/workflows/sbom.yml index df77ed859..38700f773 100644 --- a/.github/workflows/sbom.yml +++ b/.github/workflows/sbom.yml @@ -15,12 +15,6 @@ on: types: - published -concurrency: - group: >- - ${{ github.workflow }}-${{ github.repository }}-${{ - github.event_name == 'pull_request' && github.event.pull_request.number || github.run_id }} - cancel-in-progress: ${{ github.event_name == 'pull_request' }} - permissions: contents: read diff --git a/.github/workflows/secret-scan-gate.yml b/.github/workflows/secret-scan-gate.yml new file mode 100644 index 000000000..88f72b419 --- /dev/null +++ b/.github/workflows/secret-scan-gate.yml @@ -0,0 +1,29 @@ +name: secret-scan-gate + +on: + pull_request: + branches: + - develop + - main + push: + branches: + - develop + - main + +permissions: + contents: read + +env: + GIT_CONFIG_COUNT: "1" + GIT_CONFIG_KEY_0: init.defaultBranch + GIT_CONFIG_VALUE_0: develop + +jobs: + secret-scan: + name: secret-scan-gate + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + - name: Scan for common hardcoded secrets + run: | + ! git grep -nE '(g[h]p_|g[h]o_|A[K]IA[0-9A-Z]{16}|A[I]za[0-9A-Za-z\-_]{35}|BEGIN (R[S]A|E[C]|OPENS[S]H|P[G]P) PRIVATE KEY)' -- . ':(exclude)package-lock.json' ':(exclude)node_modules/**' diff --git a/.github/workflows/security-audit.yml b/.github/workflows/security-audit.yml index 07754a782..f6737f1f6 100644 --- a/.github/workflows/security-audit.yml +++ b/.github/workflows/security-audit.yml @@ -1,15 +1,14 @@ -name: security-backstop +name: security-audit on: + pull_request: + branches: + - develop + - main push: branches: - develop - main - workflow_dispatch: - -concurrency: - group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event_name }}-${{ github.ref }} - cancel-in-progress: false permissions: contents: read @@ -20,12 +19,9 @@ env: GIT_CONFIG_VALUE_0: develop jobs: - security-backstop: - name: security-backstop + audit: + name: security-audit runs-on: ubuntu-latest - permissions: - contents: read - security-events: write steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: @@ -53,9 +49,6 @@ jobs: run: uv sync --project services/analysis-engine --group dev --frozen - name: Audit Python dependencies run: uv run --project services/analysis-engine --with pip-audit==2.8.0 pip-audit --local --strict - - name: Run Bandit - working-directory: services/analysis-engine - run: uv run bandit -c pyproject.toml -r src - name: Install stable Rust toolchain run: rustup toolchain install stable --profile minimal - name: Install cargo-audit @@ -63,35 +56,3 @@ jobs: - name: Audit Rust dependencies working-directory: apps/desktop/src-tauri run: cargo +stable audit - - name: Scan for common hardcoded secrets - run: | - ! git grep -nE '(g[h]p_|g[h]o_|A[K]IA[0-9A-Z]{16}|A[I]za[0-9A-Za-z\-_]{35}|BEGIN (R[S]A|E[C]|OPENS[S]H|P[G]P) PRIVATE KEY)' -- . ':(exclude)package-lock.json' ':(exclude)node_modules/**' - - name: Run Trivy filesystem scan summary - uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0 - with: - scan-type: fs - scan-ref: . - version: v0.71.2 - format: table - severity: CRITICAL,HIGH,MEDIUM - exit-code: "0" - skip-dirs: services/analysis-engine/.venv - trivyignores: ./.trivyignore - - name: Run Trivy filesystem scan - uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0 - with: - scan-type: fs - scan-ref: . - version: v0.71.2 - format: sarif - output: trivy-results.sarif - severity: CRITICAL,HIGH,MEDIUM - limit-severities-for-sarif: true - exit-code: "1" - skip-dirs: services/analysis-engine/.venv - trivyignores: ./.trivyignore - - name: Upload Trivy scan results to GitHub Security tab - if: always() - uses: github/codeql-action/upload-sarif@99df26d4f13ea111d4ec1a7dddef6063f76b97e9 # v4.37.0 - with: - sarif_file: trivy-results.sarif diff --git a/.github/workflows/trivy.yml b/.github/workflows/trivy.yml new file mode 100644 index 000000000..d79ec32e1 --- /dev/null +++ b/.github/workflows/trivy.yml @@ -0,0 +1,54 @@ +name: trivy + +on: + push: + branches: + - develop + - main + +permissions: + contents: read + +env: + GIT_CONFIG_COUNT: "1" + GIT_CONFIG_KEY_0: init.defaultBranch + GIT_CONFIG_VALUE_0: develop + +jobs: + trivy-fs-scan: + name: trivy-fs-scan + runs-on: ubuntu-latest + permissions: + contents: read + security-events: write + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + - name: Run Trivy filesystem scan summary + uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0; SHA pinning retained as supply-chain attack mitigation, do not replace with tag. + with: + scan-type: fs + scan-ref: . + version: v0.71.2 + format: table + severity: CRITICAL,HIGH,MEDIUM + exit-code: '0' + skip-dirs: 'services/analysis-engine/.venv' + trivyignores: ./.trivyignore + - name: Run Trivy filesystem scan + uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0; SHA pinning retained as supply-chain attack mitigation, do not replace with tag. + with: + scan-type: fs + scan-ref: . + version: v0.71.2 + format: sarif + output: trivy-results.sarif + severity: CRITICAL,HIGH,MEDIUM + limit-severities-for-sarif: true + exit-code: '1' + skip-dirs: 'services/analysis-engine/.venv' + trivyignores: ./.trivyignore + - name: Upload Trivy scan results to GitHub Security tab + uses: github/codeql-action/upload-sarif@99df26d4f13ea111d4ec1a7dddef6063f76b97e9 # v4.37.0 peeled commit; SHA pinning retained as supply-chain attack mitigation. + if: always() + with: + sarif_file: trivy-results.sarif diff --git a/CHANGELOG.md b/CHANGELOG.md index 34331fb86..75765a186 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,11 +10,13 @@ ### Changed -- Consolidated Bandit, dependency audits, supplemental secret checks, and Trivy into one trusted-branch security backstop, delegated CodeQL to GitHub default setup, and removed duplicate local PR security and release-preflight runs. - Pinned npm `10.9.9` as the approved lockfile generator, activated it through Node-bundled Corepack before dependency consumption, and fail closed unless its bundled `tar` is at least `7.5.19`; primary CI still consumes the committed lock only through frozen `npm ci` validation, rejects mutable npm resolution in the lock gate, requires integrity evidence for public-registry lock entries, and preserves generator-sensitive root `@esbuild/*` peer metadata. ### Fixed +- Classify `--job` console handles (`CONIN$`/`CONOUT$`) from the 2021-12-30 console-handles contract rather than the naming-a-file reserved list, fail-close the legacy `CLOCK$` device, keep drive-relative job paths from reaching `os.lstat` or `os.open`, and log only the lexical authority class (never the rejected path). +- Reject Windows UNC/network, device-namespace, NTFS alternate-stream, and reserved Win32 device-alias `--job` file shapes before any filesystem metadata lookup, including mixed-separator UNC/device forms after `/`→`\\` translation and aliases exposed only after Win32 leading/trailing ASCII-space/period, extension, stream-suffix, and case normalization, so a caller-selected local job file cannot silently acquire remote-share, device, or named-stream authority on another host. +- Reject unknown CLI arguments and extra `--status` operands before reading standard input, keep valid whitespace-prefixed inline JSON `--job` payloads on the inline path, reject surrogate-bearing inline job arguments and text-only injected stdin with the stable UTF-8 validation error instead of terminating on an uncaught encoding exception, and fail malformed explicit invocations immediately instead of blocking on unrelated pipes or special files. File-backed `--job` reads now reject symlinks and non-regular paths before opening, request no-follow/close-on-exec/nonblocking descriptor semantics where available, verify the opened descriptor still identifies the preflighted regular file, and enforce the byte bound through that descriptor; where supported, nonblocking acquisition prevents a path swapped to a FIFO/device after preflight from turning `open()` itself into an unbounded wait. - Upgraded the local score PDF parser to `pdfjs-dist` 6.2.108, pinned Undici 7.29.0 across the workspace, and constrained PDF loading to copied in-memory bytes with a same-origin bundled worker and npm-generated lock provenance. ## [0.1.3] - 2026-04-29 diff --git a/apps/desktop/src-tauri/Cargo.lock b/apps/desktop/src-tauri/Cargo.lock index 67b39844c..0fed84b0c 100644 --- a/apps/desktop/src-tauri/Cargo.lock +++ b/apps/desktop/src-tauri/Cargo.lock @@ -3563,9 +3563,9 @@ checksum = "b6c140620e7ffbb22c2dee59cafe6084a59b5ffc27a8859a5f0d494b5d52b6be" [[package]] name = "uuid" -version = "1.25.0" +version = "1.23.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f053576934f05a761a402421fbbe3d425d9366f75f978806a037b3ca481abecc" +checksum = "bf80a72845275afea99e7f2b434723d3bc7e38470fcd1c7ed39a599c73319a53" dependencies = [ "getrandom 0.4.3", "js-sys", diff --git a/docs/architecture/overview.md b/docs/architecture/overview.md index e7e56d311..3cf5261b9 100644 --- a/docs/architecture/overview.md +++ b/docs/architecture/overview.md @@ -41,6 +41,6 @@ GitHub is the source of truth for repository governance, PR review, CI/CD, Code ## CI/CD and release flow -- PRs into `develop` and `main` run repository CI, SBOM, and platform builds alongside organization-required OSV, dependency-review, Trivy, CodeQL/code-quality, Semgrep SAST, Strix, and Noema evidence; consolidated local security backstops run after trusted-branch pushes +- PRs into `develop` and `main` run CI, dependency review, security audit, secret-scan gate, SBOM generation, and CodeQL - release flows publish desktop artifacts plus SBOM evidence to GitHub Releases through a tag-driven draft-before-publish path - branch protection connects stable required checks after bootstrap workflows exist diff --git a/docs/doctoring/cli-job-file-authority.md b/docs/doctoring/cli-job-file-authority.md new file mode 100644 index 000000000..bc44149d4 --- /dev/null +++ b/docs/doctoring/cli-job-file-authority.md @@ -0,0 +1,72 @@ +# CLI job-file path authority evidence + +## Status + +**Active Draft PR evidence.** This record documents the security boundary under review on BandScope PR #811. It is not protected-`develop` shipped truth until the implementation is merged and revalidated on the protected branch. + +## Boundary and threat model + +`bandscope-analysis --job ` is an explicit local-file input mode. Selecting that mode authorizes one bounded read of one regular local job file; it does not grant network-share, device, pipe, directory, symlink, or alternate-data-stream authority. + +A pathname is not merely a string on Windows. Universal Naming Convention (UNC) paths are used to access network resources, while DOS device paths use the `\\?\` or `\\.\` namespace forms. Windows file APIs can also translate ordinary `/` separators to `\` before native path processing. Therefore, sending an arbitrary caller-provided pathname to `os.lstat()` before classifying its namespace can acquire network or device authority even if later checks reject the resulting object (Microsoft, 2025; Microsoft, n.d.-a; Microsoft, n.d.-b). + +The CLI consequently rejects pathname strings whose slash-normalized form begins with two backslashes **before any filesystem metadata lookup**. Windows file APIs translate `/` to `\\` before native path processing, so a homogeneous-separator-only prefix test would miss mixed forms such as `/\\server\\share\\job.json` and `/\\.\\pipe\\...` and would still call `os.lstat()` / `os.open()` (Microsoft, 2025; Microsoft, n.d.-a; Microsoft, n.d.-b). Normalizing `/` to `\\` first catches ordinary UNC forms, mixed-separator UNC forms, extended UNC forms such as `\\?\UNC\server\share` and `/\\?\\UNC\\...`, and device namespace forms such as `\\.\pipe\...` and `/\\.\\pipe\\...`, while making the same explicit-input contract deterministic across hosts. + +NTFS also permits named alternate data streams. Microsoft documents the full stream form as `filename:stream name:stream type` and the common named-data-stream form as `file:stream`; the default stream is the one addressed when no stream-name component is supplied (Microsoft, n.d.-c; Russinovich, 2021). A caller who selected `job.json:secret` would therefore be selecting a different data stream than the ordinary file contents even though the path still names a regular filesystem object. The CLI's job-file contract intentionally authorizes only the ordinary unnamed file stream, so a colon in any post-drive path component is rejected before `os.lstat()`. The drive-designator colon in an absolute form such as `C:\path\job.json` remains distinct because `ntpath.splitdrive()` removes that authority prefix before the alternate-stream test. + +Reserved Win32 device aliases are also classified before metadata lookup. Device-name comparison strips the leading ASCII space that Win32 can normalize away during file/folder creation, then applies the already-established trailing ASCII-space/period, extension, alternate-stream, and case normalization. This prevents forms such as `` NUL``, `` NUL.txt``, `` COM1 .log``, and `` AUX:`` from bypassing the lexical authority boundary merely because a caller prepended an ASCII space (Microsoft, n.d.-b). + +The reserved-name list is not one bucket. Microsoft's *Naming files, paths, and namespaces* page lists `CON`, `PRN`, `AUX`, `NUL`, `COM1`–`COM9`, and `LPT1`–`LPT9`. `CONIN$` and `CONOUT$` are documented separately as console handles (Microsoft, 2021-12-30) and are not naming-a-file reserved filenames. `CLOCK$` is a legacy DOS device that the current reserved-name list no longer carries; the CLI still fail-closes it so a job path cannot acquire that device. Drive-relative forms such as `C:job.json` remain a distinct authority class and must fail before `os.lstat` or `os.open`. Rejection diagnostics log the lexical class only; they do not echo the rejected path. + +## Descriptor-bound local-file validation + +For a pathname that passes the lexical namespace boundary, the CLI uses this sequence: + +1. call `os.lstat()` and require a regular file, rejecting directories, FIFOs, devices, sockets, and symlinks visible at preflight before `open()`; +2. open read-only, requesting close-on-exec, no-follow, and nonblocking descriptor semantics where the host exposes them; +3. call `os.fstat()` on the obtained descriptor and require a regular file whose `(st_dev, st_ino)` identity matches the preflighted file; and +4. read at most `MAX_JSON_FILE_SIZE + 1` bytes through that verified descriptor. + +The nonblocking flag closes a narrower availability race that descriptor revalidation alone cannot close. A local actor can replace a preflighted regular pathname with a FIFO or blocking device between `lstat()` and `open()`. Because `fstat()` executes only after descriptor acquisition, a blocking `open(O_RDONLY)` could otherwise wait indefinitely before the authority check runs. Requesting `O_NONBLOCK` where available prevents FIFO/device acquisition from waiting for a peer, while regular-file reads retain their normal semantics; the subsequent descriptor type and inode/device checks still reject any substituted object. + +Python documents `os.fstat()` as descriptor-based status inspection, `os.lstat()` as a non-following pathname status operation, and `O_NONBLOCK`, `O_NOFOLLOW`, and related flags as platform extensions that may be unavailable when the underlying C library does not define them (Python Software Foundation, 2026). The inode/device identity check is therefore retained even when these flags are available rather than treating one platform-specific flag as the complete authority boundary. + +## TDD evidence contract + +The original regression test landed before the namespace repair. It supplies ordinary UNC, forward-slash UNC, extended UNC, and named-pipe device paths while replacing `os.lstat()` with a sentinel that fails if any filesystem lookup is attempted. Exact-head release preflight on that RED commit failed in harness verification, establishing that the previous implementation reached the filesystem lookup. The production repair then moved namespace rejection ahead of `os.lstat()`. + +A second regression-first cycle covers the `lstat()`-to-`open()` availability race. The test captures the exact descriptor flags used by `_read_bounded_job_file()` while preserving normal regular-file I/O and requires `O_NONBLOCK` whenever the host exposes it. The test-only predecessor head failed on that assertion, proving that close-on-exec/no-follow alone did not prevent a substituted FIFO/device from turning descriptor acquisition into a wait. The production repair adds only the nonblocking descriptor flag; `fstat()` regular-file and identity checks remain unchanged. + +A third regression-first cycle covers Win32 leading-space normalization. The RED test replaces `os.lstat()` with a sentinel and supplies leading-space reserved aliases; therefore any failure to classify the alias lexically is observable as an attempted filesystem lookup. The production repair changes only the reserved-device normalization step by removing leading ASCII spaces before device-name comparison. It does not broadly trim arbitrary leading periods or Unicode whitespace and therefore does not widen the lexical policy beyond the documented Win32 normalization boundary. + +A fourth regression-first cycle covers alternate data streams. Test-only head `6522e50ef1a4a023f4f0efb89f3f0d286d9b334b` supplies `job.json:secret`, `job.json::$DATA`, and an absolute drive path carrying a named stream while replacing `os.lstat()` with a sentinel. The production repair on its successor classifies post-drive colon syntax before filesystem lookup. The RED workflow cycle was queued when the production successor was pushed, so commit order is evidence of test-first construction but the queued predecessor run is not represented as runtime failure evidence. + +A fifth regression-first cycle covers mixed-separator UNC and device-namespace forms. The RED cases `/\server\share\job.json`, `\/server\share\job.json`, `/\.\pipe\bandscope-job`, and `/\?\UNC\server\share\job.json` replace both `os.lstat()` and `os.open()` with sentinels. A homogeneous-separator-only prefix test (`startswith(("\\\\", "//"))`) lets those strings reach filesystem lookup even though `ntpath.normpath()` maps them onto UNC or `\\.\` device paths. The production repair classifies after `/`→`\\` translation and does not change accepted local absolute or relative job paths. + +Commercial merge evidence still requires the final exact head to pass repository CI/release/build-baseline, owned statement and branch coverage, docstring, SAST, security, SBOM/supply-chain, and qualifying independent non-author review gates. Protected-base dependency failures owned by canonical PR #783 are not suppressed or treated as leaf-branch success. + +## Residual boundary + +This lexical rule deliberately does not claim to prove physical storage locality for drive-letter paths. Windows can expose storage through mounts or mappings whose network provenance is controlled outside this process. The CLI boundary prevents caller-selected UNC/device/alternate-stream namespaces from acquiring authority and verifies the selected regular file descriptor; host-level mount policy remains a deployment/endpoint-control responsibility. + +Applying the Win32 alternate-stream exclusion host-independently also means a POSIX filename containing a colon is outside the portable `--job` namespace. That is an intentional portability trade-off: accepted job paths have one meaning across the supported desktop hosts rather than changing authority when a project or automation moves onto Windows. + +`O_NONBLOCK` also does not claim general descriptor-level race freedom across every filesystem or operating system. It prevents the specific blocking-open availability failure where supported. Stronger race-resistant pathname acquisition primitives remain a separate platform-hardening layer when a deployment threat model includes a privileged local actor continuously replacing directory entries. + +## References + +Microsoft. (2024, April 23). *[MS-DFSC]: UNC path*. Microsoft Learn. https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-dfsc/149a3039-98ce-491a-9268-2f5ddef08192 + +Microsoft. (2025, October 22). *File path formats on Windows systems*. Microsoft Learn. https://learn.microsoft.com/en-us/dotnet/standard/io/file-path-formats + +Microsoft. (n.d.-a). *Maximum path length limitation*. Microsoft Learn. Retrieved August 16, 2026, from https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation + +Microsoft. (2021, December 30). *Console handles*. Microsoft Learn. https://learn.microsoft.com/en-us/windows/console/console-handles + +Microsoft. (n.d.-b). *Naming files, paths, and namespaces*. Microsoft Learn. Retrieved August 16, 2026, from https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file + +Microsoft. (n.d.-c). *File streams (local file systems)*. Microsoft Learn. Retrieved August 16, 2026, from https://learn.microsoft.com/en-us/windows/win32/fileio/file-streams + +Python Software Foundation. (2026). *os — Miscellaneous operating system interfaces*. Python 3.14.7 documentation. https://docs.python.org/3/library/os.html + +Russinovich, M. (2021, March 23). *Streams v1.6*. Microsoft Sysinternals. https://learn.microsoft.com/en-us/sysinternals/downloads/streams \ No newline at end of file diff --git a/docs/repository/bootstrap-plan.md b/docs/repository/bootstrap-plan.md index 7aedb1bdd..b16f458a1 100644 --- a/docs/repository/bootstrap-plan.md +++ b/docs/repository/bootstrap-plan.md @@ -31,13 +31,12 @@ After workflows exist, require these stable checks on `main` and `develop`: - `CodeRabbit` - `ci / build-and-test` - `dependency-review` +- `security-audit` +- `CodeQL` - `sbom` +- `release-preflight` - `gate / build / windows` - `gate / build / macos` -- `trivy-fs` -- `Analyze (javascript-typescript)` -- `Analyze (python)` -- organization-required Security Scan, CodeQL/code-quality, SAST Semgrep, Strix, Noema, OpenCode, scheduler, and empty-PR workflows ## Initial README exception diff --git a/docs/security/code-security.md b/docs/security/code-security.md index 472d4d936..f9163b9c4 100644 --- a/docs/security/code-security.md +++ b/docs/security/code-security.md @@ -6,18 +6,12 @@ BandScope treats GitHub Code Security as part of bootstrap governance. ## Required controls -- organization-required CodeQL/code-quality evidence and multi-language SAST on pull requests -- organization-required Trivy filesystem and OSV vulnerability scans -- organization-required dependency review on pull requests -- repository trusted-branch security backstop for npm, Python, and Rust dependencies in scope +- CodeQL or equivalent code scanning workflow +- Trivy filesystem vulnerability scan +- dependency review on pull requests +- security audit workflow for npm, Python, and Rust dependencies in scope - Dependabot alerts and security updates -- secret scanning in GitHub plus a supplemental trusted-branch secret check - -The central Security Scan owns PR OSV, dependency-review, Trivy, and soft -Scorecard evidence. BandScope combines npm, pip, Cargo, Bandit, supplemental -secret, and Trivy checks into one trusted-branch/manual backstop. GitHub default -setup owns CodeQL, while Scorecard remains separate for its restricted publish -permissions. Central workflows own every pull-request security path. +- secret scanning in GitHub plus a supplemental secret-scan gate workflow ## Enforcement diff --git a/docs/security/github-required-checks.md b/docs/security/github-required-checks.md index ce74b1af6..eb62fdae3 100644 --- a/docs/security/github-required-checks.md +++ b/docs/security/github-required-checks.md @@ -8,18 +8,13 @@ These are the merge-gate status checks that should be required on protected bran - `ci / build-and-test` - `dependency-review` +- `security-audit` +- `CodeQL` +- `trivy-fs-scan` - `sbom` +- `release-preflight` - `gate / build / windows` - `gate / build / macos` -- `trivy-fs` -- `coverage-evidence` -- `opencode-review` -- `strix` -- `scan-pr-queue` -- `osv-scan` -- `scorecard` -- `Analyze (javascript-typescript)` -- `Analyze (python)` `gate / build / windows` must cover both Windows `amd64` and Windows `arm64`. `gate / build / macos` must cover both macOS Intel (`amd64`) and macOS `arm64`. @@ -28,28 +23,13 @@ These are the merge-gate status checks that should be required on protected bran - `ci / build-and-test` - `dependency-review` +- `security-audit` +- `CodeQL` +- `trivy-fs-scan` - `sbom` +- `release-preflight` - `gate / build / windows` - `gate / build / macos` -- `trivy-fs` -- `Analyze (javascript-typescript)` -- `Analyze (python)` - -The organization required-workflow rule is the authoritative PR owner for -`osv-scan`, `dependency-review`, `trivy-fs`, Scorecard visibility, Semgrep SAST, -Strix, and Noema. GitHub default setup owns CodeQL. One repository-local -`security-backstop` job combines dependency audits, Bandit, supplemental secret -checks, and Trivy after trusted-branch pushes or manual dispatch. Scorecard stays -separate because its publishing path has stricter permissions and SARIF handling. - -The lists above reflect the live classic required-status contexts verified on -2026-09-04. The active organization ruleset separately requires the central -`close-empty-pr.yml`, `opencode-review.yml`, `pr-review-merge-scheduler.yml`, -`security-scan.yml`, `strix.yml`, `sast-semgrep.yml`, and `noema-review.yml` -workflows on the default branch. Keep these two enforcement mechanisms distinct -when changing local triggers. The retired local `security-audit` and -`release-preflight` PR contexts were removed from classic protection with this -workflow consolidation. ## GitHub settings baseline diff --git a/docs/workflow/github-bootstrap-execution-policy.md b/docs/workflow/github-bootstrap-execution-policy.md index a88f0cddb..736b695aa 100644 --- a/docs/workflow/github-bootstrap-execution-policy.md +++ b/docs/workflow/github-bootstrap-execution-policy.md @@ -38,10 +38,12 @@ The expected sequence is: Bootstrap or setup work is not complete unless GitHub-facing supply-chain controls are both committed and, where permissions allow, enforced: - `.github/dependabot.yml` +- `.github/workflows/dependency-review.yml` - `.github/workflows/security-audit.yml` +- `.github/workflows/codeql.yml` - `.github/workflows/sbom.yml` - `.github/workflows/release.yml` -- branch protection or rulesets for `main` and `develop` that require repository CI, SBOM, platform builds, and the organization-required Security Scan, CodeQL/code-quality, SAST, Strix, and review workflows +- branch protection or rulesets for `main` and `develop` that require `ci / build-and-test`, `dependency-review`, `security-audit`, `CodeQL`, `sbom`, `release-preflight`, `gate / build / windows`, and `gate / build / macos` - PR workflow that still requests CodeRabbit review and records its result when the provider responds cleanly - release retention for the generated SBOM and supplemental inventory diff --git a/scripts/checks/verify_supply_chain.py b/scripts/checks/verify_supply_chain.py index 5b87b8bff..1cd561e5c 100644 --- a/scripts/checks/verify_supply_chain.py +++ b/scripts/checks/verify_supply_chain.py @@ -18,11 +18,13 @@ Path("apps/desktop/src-tauri/Cargo.lock"), Path(".github/dependabot.yml"), # Dependency review runs via the org-level required workflow in - # ContextualWisdomLab/.github; one repo-local security backstop and - # Scorecard stay push/schedule-only while central workflows own PR scans. + # ContextualWisdomLab/.github; repo-local CodeQL and Scorecard stay push-only + # so GitHub/Scorecard can still observe SAST and supply-chain security tabs. Path(".github/workflows/security-audit.yml"), + Path(".github/workflows/codeql.yml"), Path(".github/workflows/sbom.yml"), Path(".github/workflows/release.yml"), + Path(".github/workflows/secret-scan-gate.yml"), Path(".github/workflows/build-baseline.yml"), Path(".github/workflows/ossf-scorecard.yml"), Path(".trivyignore"), @@ -1216,7 +1218,7 @@ def _verify_dependency_review_coverage(missing: list[str]) -> None: def _verify_security_audit_coverage(missing: list[str]) -> None: audit = read_workflow(Path(".github/workflows/security-audit.yml"), "security audit", missing) - for token in ["develop", "main", "push", "bandit", "git grep", "trivy-action"]: + for token in ["develop", "main", "pull_request", "push"]: if audit and token not in audit: missing.append(f"security audit workflow missing trigger token: {token}") audit_run_commands: list[str] = [] @@ -1238,20 +1240,13 @@ def _verify_security_audit_coverage(missing: list[str]) -> None: missing.append(f"security audit workflow missing vulnerability audit token: {token}") -def _verify_bandit_coverage(missing: list[str]) -> None: - bandit = read_workflow(Path(".github/workflows/security-audit.yml"), "bandit", missing) - for token in ["develop", "main", "push", "bandit"]: - if bandit and token not in bandit: - missing.append(f"bandit workflow missing token: {token}") - if bandit and "pull_request:" in bandit: - missing.append( - "bandit workflow must stay push/manual-only; central SAST owns PR scanning" - ) - - def _verify_codeql_coverage(missing: list[str]) -> None: - if Path(".github/workflows/codeql.yml").exists(): - missing.append("repo-local codeql workflow duplicates GitHub default setup") + codeql = read_workflow( + Path(".github/workflows/codeql.yml"), "codeql", missing, optional=True + ) + for token in ["develop", "main", "push", "codeql"]: + if codeql and token not in codeql: + missing.append(f"codeql workflow missing token: {token}") def _verify_release_coverage(missing: list[str]) -> None: @@ -1259,6 +1254,7 @@ def _verify_release_coverage(missing: list[str]) -> None: for token in [ "develop", "main", + "pull_request", "push", "tags:", "release-preflight", @@ -1269,9 +1265,9 @@ def _verify_release_coverage(missing: list[str]) -> None: def _verify_secret_scan_coverage(missing: list[str]) -> None: secret_scan = read_workflow( - Path(".github/workflows/security-audit.yml"), "secret scan", missing + Path(".github/workflows/secret-scan-gate.yml"), "secret scan", missing ) - for token in ["develop", "main", "push", "git grep"]: + for token in ["develop", "main", "pull_request", "push", "secret-scan-gate"]: if secret_scan and token not in secret_scan: missing.append(f"secret scan workflow missing token: {token}") @@ -1356,7 +1352,6 @@ def verify_workflow_coverage() -> list[str]: missing: list[str] = [] _verify_ci_coverage(missing) _verify_sbom_coverage(missing) - _verify_bandit_coverage(missing) _verify_security_audit_coverage(missing) _verify_codeql_coverage(missing) _verify_release_coverage(missing) diff --git a/services/analysis-engine/src/bandscope_analysis/cli.py b/services/analysis-engine/src/bandscope_analysis/cli.py index 6838ee711..b4dc6974b 100644 --- a/services/analysis-engine/src/bandscope_analysis/cli.py +++ b/services/analysis-engine/src/bandscope_analysis/cli.py @@ -4,13 +4,58 @@ import json import logging +import ntpath +import os +import stat import sys from datetime import UTC, datetime from bandscope_analysis.api import get_analysis_status, run_analysis_job, run_analysis_job_updates -from bandscope_analysis.temporal import TemporalAnalyzer +from bandscope_analysis.temporal import TemporalAnalyzer as _TemporalAnalyzer logging.basicConfig(level=logging.INFO, format="%(levelname)s: %(message)s") +logger = logging.getLogger(__name__) + +MAX_JSON_FILE_SIZE = 10 * 1024 * 1024 # 10 MB + +# Microsoft "Naming files, paths, and namespaces" reserved filenames. +# CONIN$/CONOUT$ are not on that list; they are console handles. +_WINDOWS_RESERVED_FILENAMES = frozenset( + { + "CON", + "PRN", + "AUX", + "NUL", + *(f"COM{index}" for index in range(1, 10)), + *(f"LPT{index}" for index in range(1, 10)), + "COM¹", + "COM²", + "COM³", + "LPT¹", + "LPT²", + "LPT³", + } +) + +# Microsoft console-handles (2021-12-30): CONIN$ and CONOUT$ are console +# input/output aliases, not reserved filenames from naming-a-file. +_WINDOWS_CONSOLE_HANDLES = frozenset({"CONIN$", "CONOUT$"}) + +# CLOCK$ is a legacy DOS device that modern naming-a-file no longer lists. +# Keep fail-closed so a job path cannot acquire that device. +_WINDOWS_LEGACY_DEVICE_ALIASES = frozenset({"CLOCK$"}) + +WINDOWS_JOB_PATH_RESERVED_FILENAME = "reserved-filename" +WINDOWS_JOB_PATH_CONSOLE_HANDLE = "console-handle" +WINDOWS_JOB_PATH_LEGACY_DEVICE = "legacy-device" +WINDOWS_JOB_PATH_DRIVE_RELATIVE = "drive-relative" +WINDOWS_JOB_PATH_UNC_OR_DEVICE_NAMESPACE = "unc-or-device-namespace" +WINDOWS_JOB_PATH_ALTERNATE_STREAM = "alternate-stream" + +# Compatibility hook for existing CLI-level tests and downstream monkeypatches. +# The CLI intentionally does not invoke temporal analysis before request validation; +# validated orchestration owns every local-audio file access. +TemporalAnalyzer = _TemporalAnalyzer def failed_cli_response(message: str) -> dict[str, object]: @@ -28,27 +73,228 @@ def failed_cli_response(message: str) -> dict[str, object]: } +def _read_bounded_stdin() -> tuple[str | None, int]: + """Read one bounded UTF-8 stdin payload and return text plus an exit code. + + Standard process stdin exposes ``buffer``; enforce the allocation bound on + raw bytes before UTF-8 decoding. Text-only injected streams are already + decoded outside this boundary, so retain a compatibility path for in-process + callers. Failures are emitted here so callers never need to retain rejected + payload content. + """ + binary_stdin = getattr(sys.stdin, "buffer", None) + if binary_stdin is None: + raw_text = sys.stdin.read(MAX_JSON_FILE_SIZE + 1) + try: + raw_bytes = raw_text.encode("utf-8") + except UnicodeEncodeError: + json.dump(failed_cli_response("Job input must be valid UTF-8"), sys.stdout) + return None, 1 + else: + raw_bytes = binary_stdin.read(MAX_JSON_FILE_SIZE + 1) + if len(raw_bytes) > MAX_JSON_FILE_SIZE: + path = "stdin" + logger.warning("Security: rejected input exceeding maximum size limit: %s", path) + json.dump(failed_cli_response("Job input exceeds maximum size limit"), sys.stdout) + return None, 1 + try: + raw_text = raw_bytes.decode("utf-8") + except UnicodeDecodeError: + json.dump(failed_cli_response("Job input must be valid UTF-8"), sys.stdout) + return None, 1 + return raw_text.strip(), 0 + + +def _normalized_win32_device_token(component: str) -> str: + """Return the Win32 device token after documented space/period normalization.""" + normalized_component = component.lstrip(" ").rstrip(" .") + return normalized_component.split(".", 1)[0].rstrip(" ").split(":", 1)[0].upper() + + +def _path_device_tokens(path: str) -> list[str]: + """Return normalized device tokens for every path component.""" + components = path.replace("\\", "/").split("/") + return [_normalized_win32_device_token(component) for component in components] + + +def _uses_windows_reserved_filename(path: str) -> bool: + """Return whether any component is a naming-a-file reserved filename.""" + return any(token in _WINDOWS_RESERVED_FILENAMES for token in _path_device_tokens(path)) + + +def _uses_windows_console_handle(path: str) -> bool: + """Return whether any component is a CONIN$/CONOUT$ console handle.""" + return any(token in _WINDOWS_CONSOLE_HANDLES for token in _path_device_tokens(path)) + + +def _uses_windows_legacy_device_alias(path: str) -> bool: + """Return whether any component is a fail-closed legacy device such as CLOCK$.""" + return any(token in _WINDOWS_LEGACY_DEVICE_ALIASES for token in _path_device_tokens(path)) + + +def _uses_windows_device_alias(path: str) -> bool: + """Return whether any component normalizes to a reserved, console, or legacy device.""" + return ( + _uses_windows_reserved_filename(path) + or _uses_windows_console_handle(path) + or _uses_windows_legacy_device_alias(path) + ) + + +def _uses_windows_alternate_stream(path: str) -> bool: + """Return whether a post-drive path component carries NTFS stream syntax.""" + _drive, drive_tail = ntpath.splitdrive(path) + return any(":" in component for component in drive_tail.replace("\\", "/").split("/")) + + +def classify_windows_job_path_authority(path: str) -> str | None: + """Return the lexical job-path rejection class, or ``None`` if lookup may proceed. + + Classification is purely lexical and must not call ``os.lstat`` or ``os.open``. + Console handles, including trailing-colon forms such as ``CONOUT$:``, are + reported before the NTFS alternate-stream colon test. Reserved filenames + and the legacy ``CLOCK$`` device follow the same order so a Win32 device + suffix cannot be mislabeled as a named stream. + """ + drive, drive_tail = ntpath.splitdrive(path) + if path.replace("/", "\\").startswith("\\\\"): + return WINDOWS_JOB_PATH_UNC_OR_DEVICE_NAMESPACE + if drive and not drive_tail.startswith(("\\", "/")): + return WINDOWS_JOB_PATH_DRIVE_RELATIVE + if _uses_windows_console_handle(path): + return WINDOWS_JOB_PATH_CONSOLE_HANDLE + if _uses_windows_legacy_device_alias(path): + return WINDOWS_JOB_PATH_LEGACY_DEVICE + if _uses_windows_reserved_filename(path): + return WINDOWS_JOB_PATH_RESERVED_FILENAME + if _uses_windows_alternate_stream(path): + return WINDOWS_JOB_PATH_ALTERNATE_STREAM + return None + + +def _read_bounded_job_file(path: str) -> bytes: + """Read a bounded regular local job file through a verified descriptor. + + UNC/network shapes, device namespaces, drive-relative Win32 paths, NTFS + alternate-stream syntax, naming-a-file reserved filenames, console handles + (``CONIN$`` / ``CONOUT$``, including ``CONOUT$:``), and the legacy ``CLOCK$`` + device are rejected lexically before any filesystem lookup. Slash + translation is applied before the UNC/device-namespace prefix test so + mixed-separator forms such as ``/\\\\server\\\\share`` or ``/\\\\.\\\\pipe\\\\...`` + cannot reach ``lstat`` or ``open``. Drive-relative forms such as + ``C:job.json`` are authority-bearing: Win32 resolves them through a per-drive + current directory rather than from the drive root. The remaining path is + inspected with ``lstat`` before opening so known directories, FIFOs, devices, + sockets, and symbolic links fail before descriptor acquisition. The open also + requests nonblocking mode where available so a path replaced by a FIFO/device + after preflight cannot turn descriptor acquisition into an unbounded wait. + Windows opens additionally request ``O_BINARY`` so descriptor reads preserve + the raw job bytes without text-mode CRLF or 0x1A translation. The obtained + descriptor is then checked with ``fstat`` and must identify the same regular-file + inode observed during preflight. ``O_NOFOLLOW`` and close-on-exec are additionally + requested where the platform exposes them. The byte bound is enforced on the + descriptor-backed stream rather than on a second path lookup. + """ + authority = classify_windows_job_path_authority(path) + if authority is not None: + path = f"class={authority}" + logger.warning("Security: rejected job path authority: %s", path) + raise OSError("job path must use the local regular-file namespace") + + before = os.lstat(path) + if not stat.S_ISREG(before.st_mode): + path = "non-regular" + logger.warning("Security: rejected non-regular job file: %s", path) + raise OSError("job path is not a regular file") + + flags = os.O_RDONLY + flags |= getattr(os, "O_CLOEXEC", 0) + flags |= getattr(os, "O_NOFOLLOW", 0) + flags |= getattr(os, "O_NONBLOCK", 0) + flags |= getattr(os, "O_BINARY", 0) + descriptor = os.open(path, flags) + try: + opened = os.fstat(descriptor) + if not stat.S_ISREG(opened.st_mode): + path = "non-regular" + logger.warning("Security: descriptor yielded non-regular file: %s", path) + raise OSError("opened job path is not a regular file") + if (before.st_dev, before.st_ino) != (opened.st_dev, opened.st_ino): + path = "toctou" + logger.warning("Security: detected potential TOCTOU on job path: %s", path) + raise OSError("job path changed before open") + with os.fdopen(descriptor, "rb", closefd=False) as stream: + return stream.read(MAX_JSON_FILE_SIZE + 1) + finally: + os.close(descriptor) + + def main() -> int: - """Read a job payload from stdin and print a structured job response to stdout.""" - # Read all input from stdin first - input_data = sys.stdin.read().strip() + """Read one explicit argument or bounded stdin job and print its response.""" progress_jsonl = "--progress-jsonl" in sys.argv[1:] cli_args = [arg for arg in sys.argv[1:] if arg != "--progress-jsonl"] + input_data: str | None = None - # Check if there are command line arguments (fallback for manual testing) + # Explicit argument modes own their input source. Resolve them before touching + # stdin so ``--status`` and ``--job`` cannot block on an unrelated open pipe or + # consume data that the caller did not select as the job payload. if cli_args: if cli_args[0] == "--status": + if len(cli_args) != 1: + json.dump( + failed_cli_response("--status does not accept additional arguments"), + sys.stdout, + ) + return 1 json.dump(get_analysis_status(), sys.stdout) return 0 - elif cli_args[0] == "--job" and len(cli_args) > 1: + if cli_args[0] == "--job": + if len(cli_args) != 2: + json.dump( + failed_cli_response("--job requires exactly one JSON payload or file path"), + sys.stdout, + ) + return 1 input_data = cli_args[1] - if not input_data.startswith("{"): + if input_data.lstrip(" \t\r\n").startswith("{"): + try: + input_bytes = input_data.encode("utf-8") + except UnicodeEncodeError: + json.dump(failed_cli_response("Job input must be valid UTF-8"), sys.stdout) + return 1 + if len(input_bytes) > MAX_JSON_FILE_SIZE: + path = "cli_arg" + logger.warning("Security: rejected oversized input: %s", path) + json.dump( + failed_cli_response("Job input exceeds maximum size limit"), sys.stdout + ) + return 1 + else: try: - with open(input_data, "r", encoding="utf-8") as f: - input_data = f.read() + input_bytes = _read_bounded_job_file(input_data) + if len(input_bytes) > MAX_JSON_FILE_SIZE: + path = "oversized-job-file" + logger.warning("Security: rejected oversized file: %s", path) + json.dump( + failed_cli_response("Job file exceeds maximum size limit"), + sys.stdout, + ) + return 1 + input_data = input_bytes.decode("utf-8") + except UnicodeDecodeError: + json.dump(failed_cli_response("Job input must be valid UTF-8"), sys.stdout) + return 1 except Exception: json.dump(failed_cli_response("Failed to read job file"), sys.stdout) return 1 + else: + json.dump(failed_cli_response("Unsupported CLI arguments"), sys.stdout) + return 1 + + if input_data is None: + input_data, stdin_exit_code = _read_bounded_stdin() + if input_data is None: + return stdin_exit_code if not input_data: json.dump(failed_cli_response("Empty input"), sys.stdout) @@ -74,29 +320,6 @@ def main() -> int: return 0 request = payload.get("request") - - # Temporary: Inject temporal analyzer call if it's a local file, just to prove it works - # before full orchestrator integration - if ( - isinstance(request, dict) - and request.get("sourceKind") == "local_audio" - and "localSource" in request - ): - local_source = request["localSource"] - audio_path = local_source.get("sourcePath") - file_name = local_source.get("fileName", "selected audio") - if audio_path: - logging.info("Extracting temporal features from %s...", file_name) - try: - temporal_analyzer = TemporalAnalyzer() - features = temporal_analyzer.analyze(audio_path) - logging.info(f"Extracted BPM: {features['bpm']}") - except Exception: - logging.warning( - "Temporal analysis failed for %s; continuing with safe fallback.", - file_name, - ) - requested_at = datetime.now(UTC).isoformat().replace("+00:00", "Z") if progress_jsonl: for update in run_analysis_job_updates(job_id, request, requested_at): diff --git a/services/analysis-engine/tests/test_cli.py b/services/analysis-engine/tests/test_cli.py index 057ef236b..bbde811c7 100644 --- a/services/analysis-engine/tests/test_cli.py +++ b/services/analysis-engine/tests/test_cli.py @@ -148,6 +148,7 @@ def test_cli_main_reads_stdin_and_writes_stdout(monkeypatch: pytest.MonkeyPatch) ) stdout = io.StringIO() + monkeypatch.setattr(cli.sys, "argv", ["cli.py"]) monkeypatch.setattr(cli.sys, "stdin", stdin) monkeypatch.setattr(cli.sys, "stdout", stdout) @@ -160,6 +161,7 @@ def test_cli_main_handles_non_mapping_payload(monkeypatch: pytest.MonkeyPatch) - stdin = io.StringIO(json.dumps(["demo"])) stdout = io.StringIO() + monkeypatch.setattr(cli.sys, "argv", ["cli.py"]) monkeypatch.setattr(cli.sys, "stdin", stdin) monkeypatch.setattr(cli.sys, "stdout", stdout) @@ -185,6 +187,7 @@ def test_cli_main_rejects_invalid_job_id(monkeypatch: pytest.MonkeyPatch) -> Non ) stdout = io.StringIO() + monkeypatch.setattr(cli.sys, "argv", ["cli.py"]) monkeypatch.setattr(cli.sys, "stdin", stdin) monkeypatch.setattr(cli.sys, "stdout", stdout) @@ -198,6 +201,7 @@ def test_cli_main_handles_malformed_json(monkeypatch: pytest.MonkeyPatch) -> Non stdin = io.StringIO("{") stdout = io.StringIO() + monkeypatch.setattr(cli.sys, "argv", ["cli.py"]) monkeypatch.setattr(cli.sys, "stdin", stdin) monkeypatch.setattr(cli.sys, "stdout", stdout) @@ -228,6 +232,7 @@ def test_cli_module_runs_as_main(monkeypatch: pytest.MonkeyPatch) -> None: ) stdout = io.StringIO() + monkeypatch.setattr(sys, "argv", ["cli.py"]) monkeypatch.setattr(sys, "stdin", stdin) monkeypatch.setattr(sys, "stdout", stdout) @@ -246,6 +251,7 @@ def test_cli_main_empty_input(monkeypatch: pytest.MonkeyPatch) -> None: """Ensure empty input yields an error.""" stdin = io.StringIO("") stdout = io.StringIO() + monkeypatch.setattr(cli.sys, "argv", ["cli.py"]) monkeypatch.setattr(cli.sys, "stdin", stdin) monkeypatch.setattr(cli.sys, "stdout", stdout) assert cli.main() == 0 @@ -487,3 +493,23 @@ def fake_stem_separation(*args: Any, **kwargs: Any) -> dict[str, Any]: ] assert updates[-1]["state"] == "succeeded" assert updates[-1]["progressPercent"] == 100 + + +def test_cli_main_job_arg_rejects_large_file(monkeypatch: pytest.MonkeyPatch, tmp_path) -> None: + """Ensure --job rejects files larger than MAX_JSON_FILE_SIZE.""" + stdin = io.StringIO("") + stdout = io.StringIO() + job_file = tmp_path / "large_job.json" + + # Create a dummy file larger than MAX_JSON_FILE_SIZE + from bandscope_analysis.cli import MAX_JSON_FILE_SIZE + + with open(job_file, "wb") as f: + f.seek(MAX_JSON_FILE_SIZE + 1024) + f.write(b"0") + + monkeypatch.setattr(cli.sys, "argv", ["cli.py", "--job", str(job_file)]) + monkeypatch.setattr(cli.sys, "stdin", stdin) + monkeypatch.setattr(cli.sys, "stdout", stdout) + assert cli.main() == 1 + assert "Job file exceeds maximum size limit" in stdout.getvalue() diff --git a/services/analysis-engine/tests/test_cli_input_bounds.py b/services/analysis-engine/tests/test_cli_input_bounds.py new file mode 100644 index 000000000..1c803d76c --- /dev/null +++ b/services/analysis-engine/tests/test_cli_input_bounds.py @@ -0,0 +1,291 @@ +"""Regression tests for bounded analysis-engine CLI input reads.""" + +from __future__ import annotations + +import io +import json + +import pytest + +from bandscope_analysis import cli + + +class _BoundedReadRequired(io.BytesIO): + """Fail when production attempts an unbounded binary stream read.""" + + def read(self, size: int = -1) -> bytes: + """Read only when the caller supplies the exact bounded-size envelope.""" + if not 0 <= size <= cli.MAX_JSON_FILE_SIZE + 1: + raise AssertionError("CLI stdin read must use the maximum bounded size") + return super().read(size) + + +class _OversizedInput: + """Provide an oversized byte payload without retaining it in the fixture.""" + + def read(self, size: int = -1) -> bytes: + """Return exactly the requested amount so production observes overflow.""" + if not 0 <= size <= cli.MAX_JSON_FILE_SIZE + 1: + raise AssertionError("CLI stdin read must use the maximum bounded size") + return b"x" * size + + +class _ForbiddenRead: + """Fail if an explicit CLI argument unexpectedly consumes standard input.""" + + def read(self, size: int = -1) -> bytes: + """Reject every read because explicit argument modes own their input source.""" + raise AssertionError("explicit CLI arguments must not read stdin") + + +class _BinaryStdin: + """Expose a binary buffer like the standard process stdin wrapper.""" + + def __init__(self, buffer: _BoundedReadRequired | _OversizedInput | _ForbiddenRead) -> None: + """Attach the bounded binary stream used by the CLI.""" + self.buffer = buffer + + +def _stdin_bytes(payload: bytes) -> _BinaryStdin: + """Return process-like stdin backed by explicitly bounded bytes.""" + return _BinaryStdin(_BoundedReadRequired(payload)) + + +def test_cli_stdin_read_uses_explicit_size_bound(monkeypatch: pytest.MonkeyPatch) -> None: + """A normal stdin job must never trigger an unbounded binary ``read()`` call.""" + payload = json.dumps( + { + "jobId": "bounded-stdin", + "request": { + "sourceKind": "demo", + "sourceLabel": "Bounded Input", + "roleFocus": ["bass-guitar"], + }, + } + ).encode("utf-8") + stdout = io.StringIO() + monkeypatch.setattr(cli.sys, "argv", ["cli.py"]) + monkeypatch.setattr(cli.sys, "stdin", _stdin_bytes(payload)) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 0 + assert json.loads(stdout.getvalue())["jobId"] == "bounded-stdin" + + +def test_cli_rejects_oversized_stdin_before_json_parsing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Oversized stdin is rejected after one bounded byte read, before JSON parsing.""" + stdout = io.StringIO() + monkeypatch.setattr(cli.sys, "argv", ["cli.py"]) + monkeypatch.setattr(cli.sys, "stdin", _BinaryStdin(_OversizedInput())) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 1 + response = json.loads(stdout.getvalue()) + assert response["state"] == "failed" + assert response["error"]["message"] == "Job input exceeds maximum size limit" + + +def test_cli_stdin_limit_is_measured_in_utf8_bytes( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Multibyte stdin must not bypass the advertised byte-size boundary.""" + stdout = io.StringIO() + monkeypatch.setattr(cli, "MAX_JSON_FILE_SIZE", 8) + monkeypatch.setattr(cli.sys, "argv", ["cli.py"]) + monkeypatch.setattr(cli.sys, "stdin", _stdin_bytes(("é" * 5).encode("utf-8"))) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 1 + response = json.loads(stdout.getvalue()) + assert response["error"]["message"] == "Job input exceeds maximum size limit" + + +def test_cli_rejects_invalid_utf8_before_json_parsing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Invalid UTF-8 stdin must fail with a stable payload-free error.""" + stdout = io.StringIO() + monkeypatch.setattr(cli.sys, "argv", ["cli.py"]) + monkeypatch.setattr(cli.sys, "stdin", _stdin_bytes(b"\xff")) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 1 + response = json.loads(stdout.getvalue()) + assert response["state"] == "failed" + assert response["error"]["message"] == "Job input must be valid UTF-8" + + +def test_cli_text_only_stdin_rejects_surrogate_before_size_measurement( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Text-only compatibility stdin must translate surrogate encoding failures safely.""" + stdout = io.StringIO() + monkeypatch.setattr(cli.sys, "argv", ["cli.py"]) + monkeypatch.setattr(cli.sys, "stdin", io.StringIO('{"jobId":"' + chr(0xDCFF) + '"}')) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 1 + response = json.loads(stdout.getvalue()) + assert response["state"] == "failed" + assert response["error"]["message"] == "Job input must be valid UTF-8" + + +def test_cli_inline_job_argument_obeys_input_byte_limit( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An inline ``--job`` payload cannot bypass the common JSON byte limit.""" + stdout = io.StringIO() + monkeypatch.setattr(cli, "MAX_JSON_FILE_SIZE", 8) + monkeypatch.setattr(cli.sys, "argv", ["cli.py", "--job", '{"jobId":"é"}']) + monkeypatch.setattr(cli.sys, "stdin", _stdin_bytes(b"")) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 1 + response = json.loads(stdout.getvalue()) + assert response["error"]["message"] == "Job input exceeds maximum size limit" + + +def test_cli_inline_job_rejects_surrogate_before_json_parsing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A surrogate-bearing argv payload must fail with the stable UTF-8 error.""" + surrogate_payload = '{"jobId":"' + chr(0xDCFF) + '"}' + stdout = io.StringIO() + monkeypatch.setattr(cli.sys, "argv", ["cli.py", "--job", surrogate_payload]) + monkeypatch.setattr(cli.sys, "stdin", _BinaryStdin(_ForbiddenRead())) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 1 + response = json.loads(stdout.getvalue()) + assert response["state"] == "failed" + assert response["error"]["message"] == "Job input must be valid UTF-8" + + +def test_cli_job_file_limit_is_measured_in_utf8_bytes( + monkeypatch: pytest.MonkeyPatch, + tmp_path, +) -> None: + """A multibyte job file is rejected by bytes even when character count is small.""" + job_file = tmp_path / "multibyte_job.json" + job_file.write_text("é" * 5, encoding="utf-8") + stdout = io.StringIO() + monkeypatch.setattr(cli, "MAX_JSON_FILE_SIZE", 8) + monkeypatch.setattr(cli.sys, "argv", ["cli.py", "--job", str(job_file)]) + monkeypatch.setattr(cli.sys, "stdin", _BinaryStdin(_ForbiddenRead())) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 1 + response = json.loads(stdout.getvalue()) + assert response["error"]["message"] == "Job file exceeds maximum size limit" + + +def test_cli_status_argument_does_not_consume_stdin(monkeypatch: pytest.MonkeyPatch) -> None: + """The status command must return immediately without waiting for standard input.""" + stdout = io.StringIO() + monkeypatch.setattr(cli.sys, "argv", ["cli.py", "--status"]) + monkeypatch.setattr(cli.sys, "stdin", _BinaryStdin(_ForbiddenRead())) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 0 + assert json.loads(stdout.getvalue())["status"] == "ready" + + +def test_cli_job_argument_does_not_consume_stdin( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An explicit inline job must not block on or consume unrelated standard input.""" + payload = json.dumps( + { + "jobId": "argument-job", + "request": { + "sourceKind": "demo", + "sourceLabel": "Argument Input", + "roleFocus": ["bass-guitar"], + }, + } + ) + stdout = io.StringIO() + monkeypatch.setattr(cli.sys, "argv", ["cli.py", "--job", payload]) + monkeypatch.setattr(cli.sys, "stdin", _BinaryStdin(_ForbiddenRead())) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 0 + assert json.loads(stdout.getvalue())["jobId"] == "argument-job" + + +def test_cli_inline_job_with_leading_json_whitespace_does_not_become_a_file_path( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Leading JSON whitespace must preserve inline ``--job`` dispatch semantics.""" + payload = " \n\t" + json.dumps( + { + "jobId": "whitespace-argument-job", + "request": { + "sourceKind": "demo", + "sourceLabel": "Whitespace Argument Input", + "roleFocus": ["bass-guitar"], + }, + } + ) + stdout = io.StringIO() + monkeypatch.setattr(cli.sys, "argv", ["cli.py", "--job", payload]) + monkeypatch.setattr(cli.sys, "stdin", _BinaryStdin(_ForbiddenRead())) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 0 + assert json.loads(stdout.getvalue())["jobId"] == "whitespace-argument-job" + + +def test_cli_non_json_whitespace_prefix_remains_a_job_file_path( + monkeypatch: pytest.MonkeyPatch, + tmp_path, +) -> None: + """Unicode whitespace outside JSON grammar must not redefine a file operand.""" + job_file_name = "\u00a0{job.json" + job_file = tmp_path / job_file_name + job_file.write_text( + json.dumps( + { + "jobId": "unicode-space-file-job", + "request": { + "sourceKind": "demo", + "sourceLabel": "Unicode Space File Input", + "roleFocus": ["bass-guitar"], + }, + } + ), + encoding="utf-8", + ) + stdout = io.StringIO() + monkeypatch.chdir(tmp_path) + monkeypatch.setattr(cli.sys, "argv", ["cli.py", "--job", job_file_name]) + monkeypatch.setattr(cli.sys, "stdin", _BinaryStdin(_ForbiddenRead())) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 0 + assert json.loads(stdout.getvalue())["jobId"] == "unicode-space-file-job" + + +@pytest.mark.parametrize( + "argv", + [ + ["cli.py", "--job"], + ["cli.py", "--job", "{}", "unexpected-extra-argument"], + ], +) +def test_cli_malformed_job_arguments_fail_without_consuming_stdin( + monkeypatch: pytest.MonkeyPatch, + argv: list[str], +) -> None: + """Malformed explicit ``--job`` usage must fail immediately instead of reading stdin.""" + stdout = io.StringIO() + monkeypatch.setattr(cli.sys, "argv", argv) + monkeypatch.setattr(cli.sys, "stdin", _BinaryStdin(_ForbiddenRead())) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 1 + response = json.loads(stdout.getvalue()) + assert response["state"] == "failed" + assert response["error"]["message"] == "--job requires exactly one JSON payload or file path" diff --git a/services/analysis-engine/tests/test_cli_job_console_handle_authority.py b/services/analysis-engine/tests/test_cli_job_console_handle_authority.py new file mode 100644 index 000000000..c4830932a --- /dev/null +++ b/services/analysis-engine/tests/test_cli_job_console_handle_authority.py @@ -0,0 +1,117 @@ +"""Console-handle and legacy-device classification for CLI job paths.""" + +from __future__ import annotations + +import pytest + +from bandscope_analysis import cli + + +def _forbid_filesystem(monkeypatch: pytest.MonkeyPatch) -> None: + """Fail the test if lexical rejection happens after filesystem contact.""" + + def forbidden_lstat(*_args: object, **_kwargs: object) -> object: + raise AssertionError("unsafe job path reached os.lstat") + + def forbidden_open(*_args: object, **_kwargs: object) -> int: + raise AssertionError("unsafe job path reached os.open") + + monkeypatch.setattr(cli.os, "lstat", forbidden_lstat) + monkeypatch.setattr(cli.os, "open", forbidden_open) + + +@pytest.mark.parametrize( + "path", + [ + r"C:job.json", + r"D:..\job.json", + "C:", + ], +) +def test_drive_relative_jobs_fail_before_lstat_and_open( + monkeypatch: pytest.MonkeyPatch, + path: str, +) -> None: + """Drive-relative jobs must not inherit per-drive current-directory authority.""" + _forbid_filesystem(monkeypatch) + assert cli.classify_windows_job_path_authority(path) == cli.WINDOWS_JOB_PATH_DRIVE_RELATIVE + with pytest.raises(OSError, match="local regular-file namespace"): + cli._read_bounded_job_file(path) + + +@pytest.mark.parametrize( + "path", + [ + "CONIN$", + "conin$", + "CONIN$.txt", + "CONOUT$", + "CONOUT$:", + r"parent\CONIN$", + ], +) +def test_console_handles_are_not_naming_a_file_reserved_names( + monkeypatch: pytest.MonkeyPatch, + path: str, +) -> None: + """CONIN$/CONOUT$ follow console-handles (2021-12-30), not naming-a-file.""" + _forbid_filesystem(monkeypatch) + assert cli.classify_windows_job_path_authority(path) == cli.WINDOWS_JOB_PATH_CONSOLE_HANDLE + assert cli.classify_windows_job_path_authority(path) != cli.WINDOWS_JOB_PATH_RESERVED_FILENAME + with pytest.raises(OSError, match="local regular-file namespace"): + cli._read_bounded_job_file(path) + + +@pytest.mark.parametrize("path", ["CLOCK$", "clock$", "CLOCK$.txt", r"jobs\CLOCK$"]) +def test_clock_dollar_is_fail_closed_legacy_device( + monkeypatch: pytest.MonkeyPatch, + path: str, +) -> None: + """CLOCK$ is not a current reserved filename; reject it as a legacy device.""" + _forbid_filesystem(monkeypatch) + assert cli.classify_windows_job_path_authority(path) == cli.WINDOWS_JOB_PATH_LEGACY_DEVICE + assert cli.classify_windows_job_path_authority(path) != cli.WINDOWS_JOB_PATH_RESERVED_FILENAME + with pytest.raises(OSError, match="local regular-file namespace"): + cli._read_bounded_job_file(path) + + +def test_con_remains_a_naming_a_file_reserved_filename(monkeypatch: pytest.MonkeyPatch) -> None: + """CON stays on the naming-a-file reserved list and still fails before lookup.""" + _forbid_filesystem(monkeypatch) + assert cli.classify_windows_job_path_authority("CON") == cli.WINDOWS_JOB_PATH_RESERVED_FILENAME + with pytest.raises(OSError, match="local regular-file namespace"): + cli._read_bounded_job_file("CON") + + +@pytest.mark.parametrize( + ("path", "expected"), + [ + ("CON", True), + ("PRN", True), + ("CONIN$", True), + ("CONOUT$:", True), + ("CLOCK$", True), + (r"jobs\CLOCK$.txt", True), + ("job.json", False), + (r"C:\jobs\ready.json", False), + ], +) +def test_device_alias_union_covers_reserved_console_and_legacy(path: str, expected: bool) -> None: + """The union helper stays true for every Win32 device class and false for regular files.""" + assert cli._uses_windows_device_alias(path) is expected + + +def test_rejected_authority_logs_class_without_the_path( + monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, +) -> None: + """Diagnostics name the lexical class and never echo the rejected job path.""" + _forbid_filesystem(monkeypatch) + path = r"C:secret-job.json" + with caplog.at_level("WARNING", logger="bandscope_analysis.cli"): + with pytest.raises(OSError, match="local regular-file namespace"): + cli._read_bounded_job_file(path) + messages = " ".join(record.getMessage() for record in caplog.records) + assert "class=drive-relative" in messages + assert path not in messages + assert "secret-job" not in messages diff --git a/services/analysis-engine/tests/test_cli_job_file_authority.py b/services/analysis-engine/tests/test_cli_job_file_authority.py new file mode 100644 index 000000000..c0a3782e1 --- /dev/null +++ b/services/analysis-engine/tests/test_cli_job_file_authority.py @@ -0,0 +1,179 @@ +"""Regression tests for CLI job-file authority boundaries.""" + +from __future__ import annotations + +import io +import json +import os +import pathlib +import stat + +import pytest + +from bandscope_analysis import cli + + +def test_cli_rejects_non_regular_job_path_before_open( + monkeypatch: pytest.MonkeyPatch, + tmp_path, +) -> None: + """A non-regular ``--job`` path must fail before any descriptor open.""" + stdout = io.StringIO() + open_called = False + original_os_open = cli.os.open + + def tracking_os_open(path: str, flags: int, mode: int = 0o777) -> int: + """Record descriptor opens while preserving the underlying behavior.""" + nonlocal open_called + open_called = True + return original_os_open(path, flags, mode) + + monkeypatch.setattr(cli.os, "open", tracking_os_open) + monkeypatch.setattr(cli.sys, "argv", ["cli.py", "--job", str(tmp_path)]) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 1 + assert open_called is False + response = json.loads(stdout.getvalue()) + assert response["state"] == "failed" + assert response["error"]["message"] == "Failed to read job file" + + +def test_cli_rejects_symlink_job_path_without_following_target( + monkeypatch: pytest.MonkeyPatch, + tmp_path, +) -> None: + """A ``--job`` symlink must not gain authority to its regular-file target.""" + target = tmp_path / "target.json" + target.write_text('{"jobId":"job","request":{}}', encoding="utf-8") + link = tmp_path / "job.json" + try: + link.symlink_to(target) + except OSError as error: + pytest.skip(f"symlinks unavailable in this environment: {error}") + + stdout = io.StringIO() + monkeypatch.setattr(cli.sys, "argv", ["cli.py", "--job", str(link)]) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 1 + response = json.loads(stdout.getvalue()) + assert response["state"] == "failed" + assert response["error"]["message"] == "Failed to read job file" + + +def test_cli_rejects_non_regular_opened_descriptor( + monkeypatch: pytest.MonkeyPatch, + tmp_path, +) -> None: + """Descriptor revalidation must reject a non-regular object after open.""" + path = tmp_path / "job.json" + path.write_text('{"jobId":"job","request":{}}', encoding="utf-8") + non_regular = os.stat_result((stat.S_IFDIR | 0o755, 0, 0, 0, 0, 0, 0, 0, 0, 0)) + + stdout = io.StringIO() + monkeypatch.setattr(cli.os, "fstat", lambda _descriptor: non_regular) + monkeypatch.setattr(cli.sys, "argv", ["cli.py", "--job", str(path)]) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 1 + response = json.loads(stdout.getvalue()) + assert response["state"] == "failed" + assert response["error"]["message"] == "Failed to read job file" + + +def test_cli_rejects_path_replacement_between_metadata_and_open( + monkeypatch: pytest.MonkeyPatch, + tmp_path, +) -> None: + """The opened descriptor must identify the same regular file that was preflighted.""" + original = tmp_path / "original.json" + replacement = tmp_path / "replacement.json" + original.write_text('{"jobId":"original","request":{}}', encoding="utf-8") + replacement.write_text('{"jobId":"replacement","request":{}}', encoding="utf-8") + original_os_open = os.open + + def substituted_open(path: str, flags: int, mode: int = 0o777) -> int: + """Model a local path replacement by opening a different regular inode.""" + if path == str(original): + return original_os_open(str(replacement), flags, mode) + return original_os_open(path, flags, mode) + + stdout = io.StringIO() + monkeypatch.setattr(cli.os, "open", substituted_open) + monkeypatch.setattr(cli.sys, "argv", ["cli.py", "--job", str(original)]) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 1 + response = json.loads(stdout.getvalue()) + assert response["state"] == "failed" + assert response["error"]["message"] == "Failed to read job file" + + +def test_job_file_open_requests_nonblocking_mode_when_supported( + monkeypatch: pytest.MonkeyPatch, + tmp_path, +) -> None: + """A path swap to a FIFO/device must not turn descriptor acquisition into a blocking wait.""" + nonblocking = getattr(os, "O_NONBLOCK", 0) + if not nonblocking: + pytest.skip("O_NONBLOCK is unavailable on this platform") + + path = tmp_path / "job.json" + expected = b'{"jobId":"job","request":{}}' + path.write_bytes(expected) + observed_flags: int | None = None + original_os_open = cli.os.open + + def tracking_os_open(path_value: str, flags: int, mode: int = 0o777) -> int: + """Capture the authority-bearing open flags and preserve normal file I/O.""" + nonlocal observed_flags + observed_flags = flags + return original_os_open(path_value, flags, mode) + + monkeypatch.setattr(cli.os, "open", tracking_os_open) + + assert cli._read_bounded_job_file(str(path)) == expected + assert observed_flags is not None + assert observed_flags & nonblocking == nonblocking + + +def test_job_file_open_requests_binary_mode_when_supported( + monkeypatch: pytest.MonkeyPatch, tmp_path: pathlib.Path +) -> None: + """Ensure O_BINARY is requested where available to prevent silent CRLF translation.""" + monkeypatch.setattr(os, "O_BINARY", 0x8000, raising=False) + file_path = tmp_path / "job.json" + file_path.write_text("{}") + + open_calls = [] + original_open = os.open + + def mock_open(path: str, flags: int, *args: object, **kwargs: object) -> int: + open_calls.append((path, flags)) + return original_open(path, flags, *args, **kwargs) # type: ignore[arg-type,misc,unused-ignore] + + monkeypatch.setattr(os, "open", mock_open) + + cli._read_bounded_job_file(str(file_path)) + + assert len(open_calls) == 1 + _, flags = open_calls[0] + assert flags & 0x8000 == 0x8000 + + +def test_cli_reports_invalid_utf8_for_file_backed_job( + monkeypatch: pytest.MonkeyPatch, + tmp_path: pathlib.Path, +) -> None: + """A non-UTF-8 job file must use the same stable diagnostic as other input modes.""" + path = tmp_path / "job.json" + path.write_bytes(b"\xff") + stdout = io.StringIO() + monkeypatch.setattr(cli.sys, "argv", ["cli.py", "--job", str(path)]) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 1 + response = json.loads(stdout.getvalue()) + assert response["state"] == "failed" + assert response["error"]["message"] == "Job input must be valid UTF-8" diff --git a/services/analysis-engine/tests/test_cli_job_path_authority.py b/services/analysis-engine/tests/test_cli_job_path_authority.py new file mode 100644 index 000000000..412d69968 --- /dev/null +++ b/services/analysis-engine/tests/test_cli_job_path_authority.py @@ -0,0 +1,127 @@ +"""Regression tests for CLI job-file local-path authority.""" + +from __future__ import annotations + +import pytest + +from bandscope_analysis import cli # type: ignore[attr-defined, unused-ignore, import-untyped] + + +@pytest.mark.parametrize( + "path", + [ + r"\\server\share\job.json", + "//server/share/job.json", + r"\\?\UNC\server\share\job.json", + r"\\.\pipe\bandscope-job", + r"/\server\share\job.json", + r"\/server\share\job.json", + r"/\.\pipe\bandscope-job", + r"/\?\UNC\server\share\job.json", + ], +) +def test_remote_or_device_job_paths_fail_before_filesystem_lookup( + monkeypatch: pytest.MonkeyPatch, + path: str, +) -> None: + """UNC/device namespace input must not reach metadata or open system calls.""" + + def forbidden_lstat(*_args: object, **_kwargs: object) -> object: + """Fail if lexical rejection happens after a filesystem lookup.""" + raise AssertionError("unsafe job path reached os.lstat") + + def forbidden_open(*_args: object, **_kwargs: object) -> int: + """Fail if lexical rejection happens after descriptor acquisition.""" + raise AssertionError("unsafe job path reached os.open") + + monkeypatch.setattr(cli.os, "lstat", forbidden_lstat) + monkeypatch.setattr(cli.os, "open", forbidden_open) + + with pytest.raises(OSError, match="local regular-file namespace"): + cli._read_bounded_job_file(path) + + +@pytest.mark.parametrize( + "path", + [ + "C:", + r"C:job.json", + r"D:..\job.json", + ], +) +def test_windows_drive_relative_job_paths_fail_before_filesystem_lookup( + monkeypatch: pytest.MonkeyPatch, + path: str, +) -> None: + """Drive-relative Win32 input must not inherit per-drive current-directory authority.""" + + def forbidden_lstat(*_args: object, **_kwargs: object) -> object: + """Prove lexical rejection happens before any filesystem metadata lookup.""" + raise AssertionError("drive-relative job path reached os.lstat") + + monkeypatch.setattr(cli.os, "lstat", forbidden_lstat) + + with pytest.raises(OSError, match="local regular-file namespace"): + cli._read_bounded_job_file(path) + + +@pytest.mark.parametrize( + "path", + [ + "job.json:secret", + "job.json::$DATA", + r"C:\tmp\job.json:secret", + ], +) +def test_windows_alternate_stream_job_paths_fail_before_filesystem_lookup( + monkeypatch: pytest.MonkeyPatch, + path: str, +) -> None: + """NTFS alternate-stream syntax must stay outside the regular-file job namespace.""" + + def forbidden_lstat(*_args: object, **_kwargs: object) -> object: + """Fail if alternate-stream syntax reaches the filesystem boundary.""" + raise AssertionError("alternate stream job path reached os.lstat") + + monkeypatch.setattr(cli.os, "lstat", forbidden_lstat) + + with pytest.raises(OSError): + cli._read_bounded_job_file(path) + + +@pytest.mark.parametrize( + "path", + [ + "NUL", + "nul.txt", + "NUL:", + "NUL ", + "NUL .txt", + "CON", + "CONIN$", + "CONOUT$:", + "CLOCK$", + "PRN.json", + "AUX", + "COM1", + "COM1 .log", + "com9.json", + "LPT1.txt", + "parent/COM¹.log", + r"parent\LPT³.json", + ], +) +def test_windows_device_aliases_fail_before_filesystem_lookup( + monkeypatch: pytest.MonkeyPatch, + path: str, +) -> None: + """Reserved Win32 device aliases must be rejected before path lookup.""" + + def forbidden_lstat(*_args: object, **_kwargs: object) -> object: + """Fail if a reserved device alias reaches the filesystem boundary.""" + raise AssertionError("Windows device alias reached os.lstat") + + monkeypatch.setattr(cli.os, "lstat", forbidden_lstat) + + with pytest.raises(OSError): + cli._read_bounded_job_file(path) diff --git a/services/analysis-engine/tests/test_cli_job_path_leading_space_authority.py b/services/analysis-engine/tests/test_cli_job_path_leading_space_authority.py new file mode 100644 index 000000000..8fb2af143 --- /dev/null +++ b/services/analysis-engine/tests/test_cli_job_path_leading_space_authority.py @@ -0,0 +1,31 @@ +"""Win32 leading-space normalization regressions for CLI job-file authority.""" + +from __future__ import annotations + +import pytest + +from bandscope_analysis import cli # type: ignore[attr-defined, unused-ignore, import-untyped] + + +@pytest.mark.parametrize( + "path", + [ + r"C:\jobs\ NUL", + r"C:\jobs\ NUL.txt", + r"C:\jobs\ COM1 .log", + r"C:\jobs\ AUX:", + ], +) +def test_leading_space_reserved_alias_is_rejected_before_filesystem_lookup( + path: str, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Win32-normalized device aliases must not acquire filesystem authority.""" + + def forbidden_lstat(*_args: object, **_kwargs: object) -> object: + raise AssertionError("reserved Win32 alias reached filesystem metadata lookup") + + monkeypatch.setattr(cli.os, "lstat", forbidden_lstat) + + with pytest.raises(OSError, match="local regular-file namespace"): + cli._read_bounded_job_file(path) diff --git a/services/analysis-engine/tests/test_cli_unknown_arguments.py b/services/analysis-engine/tests/test_cli_unknown_arguments.py new file mode 100644 index 000000000..8f3f8e031 --- /dev/null +++ b/services/analysis-engine/tests/test_cli_unknown_arguments.py @@ -0,0 +1,53 @@ +"""Regression tests for fail-closed explicit CLI argument dispatch.""" + +from __future__ import annotations + +import io +import json + +import pytest + +from bandscope_analysis import cli + + +class _ForbiddenRead: + """Reject standard-input reads for malformed explicit argument modes.""" + + def read(self, size: int = -1) -> bytes: + """Raise whenever argument dispatch falls through to standard input.""" + raise AssertionError("malformed explicit CLI arguments must not read stdin") + + +class _BinaryStdin: + """Expose a process-like binary stdin buffer that must remain unread.""" + + def __init__(self) -> None: + """Attach the forbidden read sentinel.""" + self.buffer = _ForbiddenRead() + + +@pytest.mark.parametrize( + ("argv", "message"), + [ + (["cli.py", "--unknown"], "Unsupported CLI arguments"), + ( + ["cli.py", "--status", "unexpected-extra-argument"], + "--status does not accept additional arguments", + ), + ], +) +def test_cli_rejects_other_malformed_explicit_arguments_without_stdin( + monkeypatch: pytest.MonkeyPatch, + argv: list[str], + message: str, +) -> None: + """Every malformed explicit argument form must fail before touching stdin.""" + stdout = io.StringIO() + monkeypatch.setattr(cli.sys, "argv", argv) + monkeypatch.setattr(cli.sys, "stdin", _BinaryStdin()) + monkeypatch.setattr(cli.sys, "stdout", stdout) + + assert cli.main() == 1 + response = json.loads(stdout.getvalue()) + assert response["state"] == "failed" + assert response["error"]["message"] == message diff --git a/services/analysis-engine/tests/test_supply_chain_policy.py b/services/analysis-engine/tests/test_supply_chain_policy.py index 1d8224c5a..ab43df89f 100644 --- a/services/analysis-engine/tests/test_supply_chain_policy.py +++ b/services/analysis-engine/tests/test_supply_chain_policy.py @@ -1235,53 +1235,30 @@ def test_supply_chain_check_accepts_repo_ossf_publish_restrictions( assert not any("ossf scorecard" in violation for violation in violations) -def test_central_governance_workflows_are_consolidated_push_backstops() -> None: - """Ensure central PR governance leaves one local push security backstop.""" +def test_central_governance_workflows_are_push_only_where_local_signals_remain() -> None: + """Ensure central PR governance keeps only repo-local push security signals.""" repo_root = Path(__file__).resolve().parents[3] workflows_dir = repo_root / ".github" / "workflows" assert not (workflows_dir / "dependency-review.yml").exists() - security_backstop = workflows_dir / "security-audit.yml" - assert security_backstop.exists() - workflow = security_backstop.read_text(encoding="utf-8") - assert "pull_request:" not in workflow - for retired_workflow in ("bandit.yml", "codeql.yml", "secret-scan-gate.yml", "trivy.yml"): - assert not (workflows_dir / retired_workflow).exists() + for local_signal in ("codeql.yml", "ossf-scorecard.yml", "trivy.yml"): + workflow = workflows_dir / local_signal + assert workflow.exists(), ( + f"{local_signal} keeps repository-local security-tab/SAST signal " + "while central required workflows handle PR enforcement" + ) + assert "pull_request:" not in workflow.read_text(encoding="utf-8") supply_chain = load_module( "scripts/checks/verify_supply_chain.py", "verify_supply_chain_central" ) required = {path.as_posix() for path in supply_chain.REQUIRED_FILES} assert ".github/workflows/dependency-review.yml" not in required - assert ".github/workflows/codeql.yml" not in required - assert ".github/workflows/security-audit.yml" in required + assert ".github/workflows/codeql.yml" in required assert ".github/workflows/ossf-scorecard.yml" in required -def test_workflow_concurrency_cancels_only_superseded_pr_heads() -> None: - """Cancel same-PR stale heads without cancelling push, release, or schedule work.""" - repo_root = Path(__file__).resolve().parents[3] - workflows_dir = repo_root / ".github" / "workflows" - - for workflow_name in ("build-baseline.yml", "ci.yml", "sbom.yml"): - workflow = (workflows_dir / workflow_name).read_text(encoding="utf-8") - assert "concurrency:" in workflow, workflow_name - assert "github.workflow }}-${{ github.repository }}" in workflow, workflow_name - assert "github.event.pull_request.number" in workflow, workflow_name - assert "cancel-in-progress: ${{ github.event_name == 'pull_request' }}" in workflow - - for workflow_name in ("ossf-scorecard.yml", "release.yml", "security-audit.yml"): - workflow = (workflows_dir / workflow_name).read_text(encoding="utf-8") - assert "concurrency:" in workflow, workflow_name - assert "cancel-in-progress: false" in workflow, workflow_name - assert "contents: read" in workflow or "permissions: read-all" in workflow, ( - workflow_name - ) - - assert "pull_request:" not in (workflows_dir / "release.yml").read_text(encoding="utf-8") - - def test_opencode_review_declares_top_level_token_permissions() -> None: """Ensure OpenCode token posture is delegated to the central required workflow.""" policy = central_required_workflow_policy_text()