[FIX]: contain deserialized payload artifacts

[Hinotobi (Hinotoi-agent)]

Summary

This is the focused follow-up requested in #57 (comment).

It keeps the load-time artifact containment hardening only: when deserializing a persisted payload collection, artifact references now have to remain under the collection's artifacts/ directory after normalization and symlink resolution.

What changed

• Adds _resolve_artifact_path() in rampart/payloads/_store.py for deserialized artifact references.
• Rejects persisted artifact paths that are absolute, contain .., or do not start with artifacts/.
• Resolves the final path and rejects symlink escapes outside the collection's artifacts/ directory.
• Adds regression tests for traversal, absolute/non-artifacts paths, and symlink escape handling.

Scope notes

• This does not add Payload.id validation.
• This does not add rampart/core/payload_ids.py.
• This does not change _copy_file_artifact() or OneDriveSurface.upload_async().
• This is framed as defense-in-depth for untrusted/shared collection deserialization, not as a high-severity payload-ID vulnerability.

Test plan

Host-local:

uv run pytest tests/unit/payloads/test_payload_store_security.py -q
uv run pytest tests/unit/payloads/test_store.py tests/unit/payloads/test_payload_store_security.py -q
uv run ruff check rampart/payloads/_store.py tests/unit/payloads/test_payload_store_security.py
uv run ruff format --check rampart/payloads/_store.py tests/unit/payloads/test_payload_store_security.py
uv run ty check rampart/payloads/_store.py tests/unit/payloads/test_payload_store_security.py
uv run pytest -q
python -m compileall -q rampart tests
git diff --check

Results:

• Focused new tests: 5 passed
• Existing payload store + new tests: 18 passed
• Full suite: 617 passed, 5 skipped
• Ruff check/format: passed
• Ty check: passed
• Compileall/diff whitespace check: passed

Container validation:

docker run --rm -e CI=true -v "$PWD":/work -w /work python:3.12-slim bash -lc 'apt-get update -qq && apt-get install -y -qq git cargo >/dev/null && python -m pip install --quiet uv && uv run pytest tests/unit/payloads/test_store.py tests/unit/payloads/test_payload_store_security.py -q && uv run ruff check rampart/payloads/_store.py tests/unit/payloads/test_payload_store_security.py && uv run ty check rampart/payloads/_store.py tests/unit/payloads/test_payload_store_security.py && git diff --check'

Result: passed (18 passed; ruff, ty, and diff whitespace checks passed).

[Hinotobi (Hinotoi-agent)]

Thanks — updated in 261a966 to use Path.relative_to("artifacts") for the lexical artifacts-prefix check.

I kept the explicit absolute/.. rejection and the resolved containment check afterward so the security behavior remains unchanged for traversal and symlink escapes.

Validation:

uv run pytest tests/unit/payloads/test_payload_store_security.py -q
# 5 passed
uv run pytest tests/unit/payloads/test_store.py tests/unit/payloads/test_payload_store_security.py -q
# 18 passed
uv run ruff check rampart/payloads/_store.py tests/unit/payloads/test_payload_store_security.py
uv run ruff format --check rampart/payloads/_store.py tests/unit/payloads/test_payload_store_security.py
uv run ty check rampart/payloads/_store.py tests/unit/payloads/test_payload_store_security.py
python -m compileall -q rampart tests
git diff --check

[Copilot]

Pull request overview

This PR hardens payload collection load-time artifact handling by validating and containing deserialized artifact references so they cannot escape the collection’s artifacts/ directory after normalization and symlink resolution.

Changes:

• Added PayloadStore._resolve_artifact_path() (and helper containment logic) to validate serialized artifact references during deserialization.
• Updated deserialization to use the resolver and fail fast on invalid artifact paths before attempting to read.
• Added regression tests covering traversal, absolute/non-artifacts/ paths, missing artifacts, and symlink escape scenarios.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
rampart/payloads/_store.py	Adds artifact-path resolution/containment checks and routes deserialization through them.
tests/unit/payloads/test_payload_store_security.py	Adds unit regression coverage for invalid serialized artifact paths and symlink escape handling.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[Bashir Partovi (bashirpartovi)]

Hinotobi (@Hinotoi-agent) would you please fix the build? the checks are failing and Copilot also has recommendations

[Hinotobi (Hinotoi-agent)]

Fixed the failing build and remaining Copilot recommendation in 107e8ce:

• added the required Raises/Returns docstring sections for the new helpers, resolving all three Ruff failures;
• normalized non-string deserialized artifact values (null, number, list, object) to the existing ValueError("Invalid artifact path ...") contract with exception chaining;
• added regression coverage for each non-string JSON type.

Validation:

• focused security tests: 12 passed
• combined payload-store tests: 25 passed
• full suite: 624 passed, 5 skipped
• full-repository Ruff check/format, focused ty, compileall, and git diff --check: passed
• Docker Python 3.12: 12 focused tests passed; full Ruff check/format passed
• strict MkDocs build: passed

The prior docs job failed while setup-uv was fetching its version manifest, before dependency installation; the strict local docs build passes and this push retriggers that job.

[Nina Chikanov (nina-msft)]

Thanks for the work Hinotobi (@Hinotoi-agent)! I've enabled auto-merge on this as it seems GitHub Actions is currently in a major outage so jobs may remain queued for a while...I'll check back in on this at the end of the day to see if its still on hold.

[Spencer Schoenberg (spencrr)]

Hinotobi (@Hinotoi-agent) looks like linting failed - can you please fix?

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

tests/unit/payloads/test_payload_store_security.py:23

• The production loader reads JSONL with encoding='utf-8', but this test writes using Path.write_text(...) without specifying encoding. To avoid platform-dependent defaults (and align with the code under test), pass encoding='utf-8' here.

    (collection_dir / "payloads.jsonl").write_text(json.dumps(record) + "\n")

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

rampart/payloads/_store.py:434

• This only verifies that the path exists, so a reference such as artifacts/subdir is accepted and stored as a binary artifact even though Payload.artifact must point to a file. Consumers such as OneDriveSurface.upload_async() call read_bytes() and will then fail with IsADirectoryError. Check is_file() here so deserialization rejects directories and other non-file filesystem entries.

            if not artifact_path.exists():
                msg = f"Missing artifact: {artifact_path}"
                raise FileNotFoundError(msg)