diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b6f7e784..75765a186 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ ### 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 @@ -74,4 +77,4 @@ - `ChordsFeature` (코드 분석) 화면에서 각 파트(Role)의 `transpositionPlan`(이조/조옮김 계획)을 표시하는 기능을 추가했습니다. - `RangesFeature` (음역대 분석) 화면에서 겹침 경고(Overlap warning) 외에 해당 파트의 채보(Transcription) 가능 노드 수를 요약하여 보여주는 기능을 추가했습니다. -- 신규 UI 요소에 대한 단위 테스트를 추가했습니다 (`apps/desktop/src/features/chords/index.test.tsx`, `apps/desktop/src/features/ranges/index.test.tsx`). \ No newline at end of file +- 신규 UI 요소에 대한 단위 테스트를 추가했습니다 (`apps/desktop/src/features/chords/index.test.tsx`, `apps/desktop/src/features/ranges/index.test.tsx`). 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/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