fix(security): validate Postman collection/environment ids, block path traversal

postman:// targets took collection_uid/environment_uid as raw netloc+path with
no format check. requests collapses ".." in a URL path before sending the
request, so fetch_postman_collection/fetch_postman_environment built
`{POSTMAN_API_BASE}/collections/{uid}` (and /environments/{uid}) with an
attacker-controlled string that could traverse to an arbitrary Postman API
endpoint -- authenticated with the caller's own POSTMAN_API_KEY via X-Api-Key.

Confirmed with requests.Request(...).prepare().url:
  'https://api.getpostman.com/collections/x/../../workspaces'
  -> 'https://api.getpostman.com/workspaces'

Also: a successfully-fetched collection was written with the default umask
(world-readable on a shared host) even though it can carry saved auth
headers/tokens, while this codebase already has write_secret_text (0600) for
exactly this kind of data in three other call sites.

Fix:
- validate_postman_uid() (letters/digits/hyphens only, matching real Postman
  uid shape) enforced both at target-parse time (fail fast, clear error) and
  inside fetch_postman_collection/fetch_postman_environment (defense in depth
  for any other caller).
- write_fetched_collection now uses write_secret_text instead of
  Path.write_text.

Verified after rebasing onto upstream main (fde37a14): the 5 test files that
import the changed modules pass (97 tests), ruff format and ruff check are
clean on the touched files, mypy Success: no issues found in 4 source files
(note: unused pyproject override sections for cvss.*, pypdf.*, reportlab.*).
The full suite was not re-run. 6 new tests cover the traversal rejection
(asserting requests.get is never called), an @-authority variant, and the
file-permission fix.
This commit is contained in:
FabioLeitão 2026-08-24 13:43:09 -03:00 • committed by Fabio Leitao
parent fde37a149f
commit 0134b3bcce
No known key found for this signature in database
4 changed files with 93 additions and 4 deletions

View file

@ -20,7 +20,8 @@ from rich.text import Text
from strix.config import load_settings
from strix.telemetry import report_error
from strix.utils.api_spec import detect_spec_format
from strix.utils.api_spec import detect_spec_format, validate_postman_uid
from strix.utils.secret_files import write_secret_text
logger = logging.getLogger(__name__)
@ -1149,6 +1150,10 @@ def infer_target_type(target: str) -> tuple[str, dict[str, str]]: # noqa: PLR09
raise ValueError(
f"Missing Postman collection id in '{target}' (expected postman://<collection-uid>)"
)
# Reject here too, not just inside fetch_postman_collection: a malformed
# id (e.g. containing "..") must never reach the point of being turned
# into an HTTP request against the Postman API with a real API key.
validate_postman_uid(collection_uid, "collection")
details = {
"target_spec": target,
"spec_format": "postman",
@ -1158,6 +1163,7 @@ def infer_target_type(target: str) -> tuple[str, dict[str, str]]: # noqa: PLR09
query = parse_qs(parsed.query)
env_uid = (query.get("env") or query.get("environment") or [""])[0].strip()
if env_uid:
validate_postman_uid(env_uid, "environment")
details["environment_uid"] = env_uid
return "api_spec", details
@ -1508,9 +1514,11 @@ def write_fetched_collection(collection: dict[str, Any], collection_uid: str) ->
spec file from here on and the API key never leaves the host.
"""
staging = Path(tempfile.gettempdir()) / "strix_api_specs" / "fetched"
staging.mkdir(parents=True, exist_ok=True)
path = staging / f"{sanitize_name(collection_uid)}.postman_collection.json"
path.write_text(json.dumps(collection, indent=2), encoding="utf-8")
# A fetched collection can carry auth headers/tokens saved in Postman requests;
# write it 0600 like the other secret-bearing files in this codebase instead of
# the default umask (world-readable on a shared host).
write_secret_text(path, json.dumps(collection, indent=2))
return str(path)

View file

@ -234,6 +234,25 @@ def spec_base_urls(
POSTMAN_API_BASE = "https://api.getpostman.com"
_POSTMAN_FETCH_TIMEOUT = 30
# Postman collection/environment uids are `{ownerId}-{uuid}` (digits, hex, hyphens).
# collection_uid/environment_uid ultimately come from a user-supplied `postman://`
# target (see interface/utils.py) with no upstream format check. requests collapses
# `..` in a URL path before the request is sent, so an unvalidated uid containing
# `../..` turns `GET /collections/{uid}` into a confused-deputy request against an
# arbitrary Postman API endpoint (e.g. `/workspaces`, `/users/me`), authenticated
# with the caller's own POSTMAN_API_KEY. Enforced here too (not just at the target
# parser) so this stays safe regardless of caller.
_POSTMAN_UID_PATTERN = re.compile(r"^[A-Za-z0-9-]+$")
def validate_postman_uid(uid: str, label: str) -> None:
if not _POSTMAN_UID_PATTERN.fullmatch(uid):
raise SpecParseError(
f"Invalid Postman {label} id {uid!r}: expected only letters, digits, "
"and hyphens (a real Postman collection/environment id never contains "
"'/', '.', or other characters).",
)
def _postman_api_json(url: str, api_key: str, label: str) -> dict[str, Any]:
"""GET a Postman API resource and return the parsed JSON payload.
@ -279,6 +298,7 @@ def fetch_postman_collection(collection_uid: str, api_key: str) -> dict[str, Any
wraps the collection under a ``collection`` key, unwrapped here so the result
matches an exported collection file.
"""
validate_postman_uid(collection_uid, "collection")
payload = _postman_api_json(
f"{POSTMAN_API_BASE}/collections/{collection_uid}",
api_key,
@ -296,6 +316,7 @@ def fetch_postman_environment(environment_uid: str, api_key: str) -> dict[str, s
Disabled values are skipped, matching how Postman resolves an environment at
request time.
"""
validate_postman_uid(environment_uid, "environment")
payload = _postman_api_json(
f"{POSTMAN_API_BASE}/environments/{environment_uid}",
api_key,

View file

@ -234,6 +234,32 @@ def test_fetch_postman_missing_key_raises() -> None:
fetch_postman_collection("abc-123", "")
def test_fetch_postman_collection_rejects_path_traversal_uid(
monkeypatch: pytest.MonkeyPatch,
) -> None:
# Security regression: requests collapses ".." in a URL path before sending
# the request, so this used to turn into a confused-deputy GET against an
# arbitrary Postman API endpoint, authenticated with the caller's API key.
# Assert requests.get is never even called -- rejected before any network I/O.
def fail_if_called(*_a: Any, **_k: Any) -> _FakeResponse:
raise AssertionError("requests.get must not be called for an invalid uid")
monkeypatch.setattr(requests, "get", fail_if_called)
with pytest.raises(SpecParseError, match="Invalid Postman collection id"):
fetch_postman_collection("x/../../workspaces", "PMAK-xyz")
def test_fetch_postman_environment_rejects_path_traversal_uid(
monkeypatch: pytest.MonkeyPatch,
) -> None:
def fail_if_called(*_a: Any, **_k: Any) -> _FakeResponse:
raise AssertionError("requests.get must not be called for an invalid uid")
monkeypatch.setattr(requests, "get", fail_if_called)
with pytest.raises(SpecParseError, match="Invalid Postman environment id"):
fetch_postman_environment("../../users/me", "PMAK-xyz")
def test_fetch_postman_404_raises(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setattr(requests, "get", lambda *_a, **_k: _FakeResponse(404, {}))
with pytest.raises(SpecParseError, match="not found"):

View file

@ -11,7 +11,7 @@ import pytest
from strix.core.inputs import build_root_task, build_scope_context
from strix.interface.scan_setup import build_targets_info
from strix.interface.utils import infer_target_type, stage_api_specs
from strix.interface.utils import infer_target_type, stage_api_specs, write_fetched_collection
OPENAPI = {
@ -78,6 +78,27 @@ def test_infer_target_type_rejects_empty_postman_uri() -> None:
infer_target_type("postman://")
def test_infer_target_type_rejects_path_traversal_in_postman_collection_uid() -> None:
# Security regression: requests collapses ".." in a URL path before sending
# the request, so an unvalidated collection_uid turned `GET
# /collections/{uid}` into a confused-deputy request against an arbitrary
# Postman API endpoint, authenticated with the caller's own POSTMAN_API_KEY.
with pytest.raises(ValueError, match="Invalid Postman collection id"):
infer_target_type("postman://x/../../workspaces")
def test_infer_target_type_rejects_path_traversal_in_postman_environment_uid() -> None:
with pytest.raises(ValueError, match="Invalid Postman environment id"):
infer_target_type("postman://coll-uid?env=../../users/me")
def test_infer_target_type_rejects_at_sign_in_postman_uid() -> None:
# Guards against a uid engineered to change the request's authority/host
# component if this ever ends up interpolated somewhere URL-relative.
with pytest.raises(ValueError, match="Invalid Postman collection id"):
infer_target_type("postman://evil.example@trusted.example")
def test_infer_target_type_parses_postman_environment() -> None:
_ttype, details = infer_target_type("postman://coll-uid?env=env-uid")
assert details["collection_uid"] == "coll-uid"
@ -89,6 +110,19 @@ def test_infer_target_type_postman_without_env_omits_key() -> None:
assert "environment_uid" not in details
def test_write_fetched_collection_is_not_world_readable() -> None:
# Security regression: a fetched Postman collection can carry saved auth
# headers/tokens. It used to be written with the default umask (typically
# world-readable on a shared host); it must be 0600 like the codebase's
# other secret-bearing files.
path_str = write_fetched_collection({"info": {"name": "x"}}, "world-readable-test-uid")
try:
mode = Path(path_str).stat().st_mode & 0o777
assert mode == 0o600, f"expected 0600, got {oct(mode)}"
finally:
Path(path_str).unlink(missing_ok=True)
def test_build_targets_info_records_title_and_base_urls(tmp_path: Path) -> None:
(target,) = _resolved_targets(_write_spec(tmp_path))
assert target["details"]["spec_title"] == "Shop API"