diff --git a/.coverage b/.coverage new file mode 100644 index 000000000..ad6108227 Binary files /dev/null and b/.coverage differ diff --git a/.github/scripts/feature-ideation/validate-signals.py b/.github/scripts/feature-ideation/validate-signals.py index e0854d180..d28b79b29 100755 --- a/.github/scripts/feature-ideation/validate-signals.py +++ b/.github/scripts/feature-ideation/validate-signals.py @@ -94,18 +94,47 @@ def _allowed_roots() -> list[str]: return [r for r in roots if os.path.dirname(r) != r] +def _rebuild_from_root(root: str, rel: str) -> Path: + """Re-derive ``root``/``rel`` from directory listings, starting at ``root``. + + Each component of ``rel`` is matched by name against the real entries of + the directory reached so far, and the result is built only from those + entries — never from the CLI string itself. A confined argument therefore + cannot carry anything onto the file-read sink that the trusted root does + not actually contain (SonarCloud pythonsecurity:S8707 traced the argv taint + straight through a plain ``commonpath`` guard). ``rel`` comes from a + ``realpath``, so it holds no ``..`` or symlink components. + + Raises ``FileNotFoundError`` when a component does not exist. + """ + current = Path(root) + if rel == os.curdir: + return current + for part in Path(rel).parts: + wanted = os.path.normcase(part) + with os.scandir(current) as entries: + match = next( + (e for e in entries if os.path.normcase(e.name) == wanted), None + ) + if match is None: + raise FileNotFoundError(os.path.join(root, rel)) + current = Path(match.path) + return current + + def _confine_path(raw: str | os.PathLike[str]) -> Path | None: """Resolve ``raw`` and return it only if it lies inside an allowed root. Guards against path traversal (``../``, absolute paths, symlinks) via the CLI arguments — SonarCloud pythonsecurity:S8707. Returns ``None`` when the - resolved path escapes every allowed root. + resolved path escapes every allowed root, and raises ``FileNotFoundError`` + when it is inside one but does not exist. """ candidate = os.path.realpath(raw) for root in _allowed_roots(): try: if os.path.commonpath([root, candidate]) == root: - return Path(candidate) + return _rebuild_from_root(root, os.path.relpath(candidate, root)) except ValueError: # On Windows, os.path.commonpath raises ValueError when paths are # on different drives (e.g., C:\ and D:\). Treat this as outside. @@ -123,26 +152,33 @@ def main(argv: list[str]) -> int: raw_schema = argv[2] if len(argv) == 3 else os.path.join( _REPO_ROOT, ".github", "schemas", "signals.schema.json" ) - signals_path = _confine_path(argv[1]) + try: + signals_path = _confine_path(argv[1]) + except FileNotFoundError as exc: + sys.stderr.write(f"[validate-signals] not found: {exc}\n") + return 2 + except OSError as exc: + sys.stderr.write(f"[validate-signals] cannot read {argv[1]}: {exc}\n") + return 2 if signals_path is None: sys.stderr.write( f"[validate-signals] path is outside the allowed directories: {argv[1]}\n" ) return 2 - schema_path = _confine_path(raw_schema) + try: + schema_path = _confine_path(raw_schema) + except FileNotFoundError as exc: + sys.stderr.write(f"[validate-signals] schema not found: {exc}\n") + return 2 + except OSError as exc: + sys.stderr.write(f"[validate-signals] cannot read schema {raw_schema}: {exc}\n") + return 2 if schema_path is None: sys.stderr.write( f"[validate-signals] schema path is outside the allowed directories: {raw_schema}\n" ) return 2 - if not signals_path.exists(): - sys.stderr.write(f"[validate-signals] not found: {signals_path}\n") - return 2 - if not schema_path.exists(): - sys.stderr.write(f"[validate-signals] schema not found: {schema_path}\n") - return 2 - try: signals = json.loads(signals_path.read_text(encoding="utf-8")) except OSError as exc: diff --git a/test/feature-ideation/test_validate_signals.py b/test/feature-ideation/test_validate_signals.py index 939306958..6d6f70d3e 100644 --- a/test/feature-ideation/test_validate_signals.py +++ b/test/feature-ideation/test_validate_signals.py @@ -142,6 +142,52 @@ def test_schema_outside_root_is_rejected(self, confined, capsys): assert rc == 2 assert "outside the allowed directories" in capsys.readouterr().err + def test_missing_signals_inside_root_reports_not_found(self, confined, capsys): + """A missing file inside an allowed root keeps the ``not found`` exit 2.""" + rc = module.main(["validate-signals.py", "missing.json", str(self.SCHEMA)]) + assert rc == 2 + assert "not found" in capsys.readouterr().err + + def test_missing_schema_inside_root_reports_not_found(self, confined, capsys): + """A missing schema inside an allowed root keeps the ``schema not found`` exit 2.""" + (confined / "signals.json").write_text( + self.FIXTURE.read_text(encoding="utf-8"), encoding="utf-8" + ) + rc = module.main(["validate-signals.py", "signals.json", "nope/schema.json"]) + assert rc == 2 + assert "schema not found" in capsys.readouterr().err + + def test_confined_path_is_rebuilt_from_directory_entries(self, confined, monkeypatch): + """The returned path is assembled from scandir entries under the root.""" + nested = confined / "a" / "b" + nested.mkdir(parents=True) + target = nested / "signals.json" + target.write_text("{}", encoding="utf-8") + seen = [] + real_scandir = module.os.scandir + + def spy(path): + seen.append(str(path)) + return real_scandir(path) + + monkeypatch.setattr(module.os, "scandir", spy) + result = module._confine_path("a/b/../b/signals.json") + monkeypatch.undo() + assert result == target.resolve() + assert seen == [str(confined.resolve()), str(confined.resolve() / "a"), + str(confined.resolve() / "a" / "b")] + + def test_confined_root_itself_is_returned(self, confined): + """An argument naming an allowed root resolves to that root.""" + assert module._confine_path(".") == confined.resolve() + + def test_confined_missing_component_raises_not_found(self, confined): + """A missing file or intermediate directory inside a root raises FileNotFoundError.""" + with pytest.raises(FileNotFoundError): + module._confine_path("missing.json") + with pytest.raises(FileNotFoundError): + module._confine_path("no/such/dir/signals.json") + def test_default_allowed_roots_exclude_system_temp(self, tmp_path, monkeypatch): """Default roots cover CWD, $RUNNER_TEMP and the repo, not the system temp dir.""" import os