[security] fix(payloads): harden payload artifact paths

[Hinotoi-agent]

Summary

This PR hardens how RAMPART handles payload identifiers and persisted payload artifacts before they are embedded into local artifact paths or OneDrive upload names.

Payload IDs and serialized artifact paths are now constrained to filename-safe, collection-local values. The patch rejects unsafe IDs at payload construction time, validates artifact copy destinations before writing, validates deserialized artifact references before loading, and applies the same payload ID boundary before constructing OneDrive upload paths.

Security issues covered

Issue	Impact	Fix
Payload ID path traversal in artifact filenames	A payload ID containing path separators or traversal segments could influence where binary artifacts are written when a collection is saved.	Add shared payload ID validation and destination containment checks before artifact copies.
Persisted artifact path escape on load	A crafted payloads.jsonl artifact reference could point outside the collection's artifacts/ directory.	Require relative artifacts/... paths, reject .. and absolute paths, and resolve/contain paths under the collection artifacts directory.
OneDrive path-addressing injection via payload ID	A payload ID with path delimiters could be embedded into a Microsoft Graph path-addressed upload name.	Validate the payload ID before constructing the OneDrive upload filename/path.

Before this PR

• Payload.id values were accepted without a filename/path safety boundary.
• Binary artifact filenames were derived directly from payload.id.
• Deserialized artifact paths were joined with the collection directory without first enforcing that they remained under artifacts/.
• OneDrive uploads embedded payload.id into a Graph path-addressed filename.

After this PR

• Payload IDs must be 1-128 characters using only letters, numbers, ., _, and -.
• . and .. are rejected explicitly.
• Artifact copy destinations are resolved and checked to remain inside the target artifacts/ directory.
• Serialized artifact references must be relative artifacts/... paths and must not contain traversal segments.
• OneDrive uploads reject unsafe payload IDs before constructing remote paths.

Why this matters

Payload collections are persisted to disk and may include binary artifacts. If a payload source, generated collection, or persisted JSONL record is treated as data but can influence filesystem paths, it can cross from payload content into host file placement or host file reads.

Enforcing a narrow filename-safe payload ID format and resolving artifact paths under the collection artifact root keeps payload data inside the intended storage boundary.

Attack flow

1. An unsafe payload ID or crafted payloads.jsonl record is introduced.
2. RAMPART saves or loads a payload collection.
3. The unsafe value is used as part of an artifact path or remote upload path.
4. Without containment checks, the value can escape the intended artifact namespace.
5. With this PR, the unsafe value is rejected before the path is used.

Affected code

• rampart/core/types.py
• rampart/core/payload_ids.py
• rampart/payloads/_store.py
• rampart/surfaces/onedrive.py

Root cause

• Payload IDs were used in file and remote path construction without a shared safety contract.
• Serialized artifact paths were trusted as relative collection-local values without validating traversal, absolute paths, or symlink-mediated escapes.
• Local artifact persistence and remote surface upload construction enforced related path boundaries independently, leaving room for drift.

CVSS assessment

Issue: payload artifact path containment bypass

CVSS v3.1: 7.1 High

Vector: CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:N

Rationale: exploitation requires the ability to introduce or load a crafted payload collection or payload definition in the local RAMPART execution context. If reached, the vulnerable path construction can cross the intended artifact boundary and affect confidentiality or integrity of local files addressed by the running process.

Safe reproduction steps

On vulnerable code, a regression test can construct a payload with an ID containing traversal or load a payloads.jsonl record whose artifact field points outside artifacts/.

Examples covered by the tests in this PR:

Payload(id="../outside", content="x", format=PayloadFormat.TEXT)

{"id":"safe","content":"x","format":"text","metadata":{},"artifact":"../outside.pdf"}

Expected vulnerable behavior

Before the fix, unsafe payload IDs or serialized artifact references could reach path construction. Depending on the operation, this could place an artifact outside the expected directory or resolve a loaded artifact reference outside the collection's artifacts/ root.

After the fix, these inputs raise ValueError before file copy, artifact load, or OneDrive path construction proceeds.

Changes in this PR

• Adds rampart.core.payload_ids.validate_payload_id() as the shared payload ID safety boundary.
• Calls the validator from Payload.__post_init__() and OneDrive upload path construction.
• Validates artifact copy destinations with resolved-path containment checks.
• Adds a dedicated artifact deserialization resolver that rejects absolute paths, .., non-artifacts/ paths, and symlink escapes.
• Adds regression coverage for unsafe IDs, local artifact copy behavior, artifact traversal, symlink escape handling, and OneDrive upload validation.

Files changed

Category	Files	What changed
Core validation	rampart/core/payload_ids.py, rampart/core/types.py	Adds and applies shared payload ID validation.
Payload persistence	rampart/payloads/_store.py	Adds artifact destination and deserialization containment checks.
OneDrive surface	rampart/surfaces/onedrive.py	Validates payload IDs before Graph path-addressed upload names are built.
Regression tests	tests/unit/payloads/test_payload_store_security.py, tests/unit/surfaces/test_onedrive_security.py	Covers rejected unsafe inputs and allowed safe artifact persistence.

Maintainer impact

This is intentionally narrow. Existing payload IDs that already use simple filename-safe IDs continue to work. Payload IDs containing path separators, Graph path delimiters, control characters, empty strings, or traversal-only segments are now rejected early with ValueError.

The persisted artifact format remains artifacts/<filename> for valid existing collections.

Fix rationale

The safest boundary is to prevent payload IDs from being path-like at all, then still perform resolved-path containment checks at the filesystem boundary. This keeps the API contract simple while preserving defense in depth for deserialized data that may come from disk.

Type of change

• Security fix
• Tests
• Documentation update
• Refactor only

Test plan

Commands run locally:

uv run pytest tests/unit/payloads/test_payload_store_security.py tests/unit/surfaces/test_onedrive_security.py -q
uv run pytest -q
uv run ruff check rampart/core/payload_ids.py rampart/core/types.py rampart/payloads/_store.py rampart/surfaces/onedrive.py tests/unit/payloads/test_payload_store_security.py tests/unit/surfaces/test_onedrive_security.py
uv run ruff format --check rampart/core/payload_ids.py rampart/core/types.py rampart/payloads/_store.py rampart/surfaces/onedrive.py tests/unit/payloads/test_payload_store_security.py tests/unit/surfaces/test_onedrive_security.py
uv run pyright rampart/core/payload_ids.py rampart/core/types.py rampart/payloads/_store.py rampart/surfaces/onedrive.py tests/unit/payloads/test_payload_store_security.py tests/unit/surfaces/test_onedrive_security.py
python -m compileall -q rampart tests
git diff --check

Results:

• Focused security tests: 14 passed
• Full test suite: 462 passed
• Ruff check: passed
• Ruff format check: passed
• Pyright on touched files/tests: 0 errors, 0 warnings, 0 informations
• Compileall: passed
• Diff whitespace check: passed

Disclosure notes

This PR is limited to hardening payload ID and artifact path handling in the local payload store and OneDrive upload path construction. It does not claim to change unrelated payload generation, model behavior, or broader surface authentication semantics.

[Hinotoi-agent]

@microsoft-github-policy-service agree

[bashirpartovi]

Thanks for this @Hinotoi-agent and for the care that went into it, shared validator, thorough tests, everything green. After reviewing it against how RAMPART actually constructs and uses payloads, I'm going to close this PR as-is, but there's a piece of it I'd genuinely like to see come back as a focused follow-up, so I've written up the reasoning and a suggested scope below.

The central change, validating Payload.id for filename/path safety at construction time, guards a path that isn't reachable the way the description implies. Payload.id is either framework-generated (uuid.uuid4().hex[:12] in the generators) or set explicitly by the test author in their own code; it's never derived from model or agent output. The adversarial, model-controlled field is content, and content is never used to build a filesystem or Graph path. So "payload-ID path traversal" and "OneDrive path injection via payload ID" effectively require an author to hand themselves a malicious ID in their own test code, or to already have write access to the local .rampart/payloads/ tree which puts the real severity well below the 7.1 High in the description. It also doesn't justify enforcing a filename charset on Payload, a core shared type whose value space we'd rather not couple to OneDrive/Graph delimiters or filesystem naming rules.

That said, the load-side hardening is a real improvement and I'd like to take it, when deserializing a persisted collection, rejecting artifact references that are absolute, contain .., fall outside artifacts/, or resolve through a symlink outside the collection. A collection that's committed, shared between teammates, or downloaded is a plausible way an untrusted artifact path reaches disk, so containing that boundary on load is genuinely worthwhile.

Would you be up for opening a new PR with just that? A scope that keeps it easy to accept:

• Keep the load-time containment (_resolve_artifact_path plus the resolve() / is_relative_to check) in _store.py.
• Drop the Payload.__post_init__ change and rampart/core/payload_ids.py, along with the redundant re-checks in _copy_file_artifact and OneDriveSurface.upload_async.
• Make the symlink test cross-platform - pytest.skip on OSError from symlink_to, since CI is Linux-only and Windows contributors won't have symlink privileges by default.
• Frame it as defense-in-depth for untrusted-collection deserialization rather than a high-severity ID vulnerability.

Thanks again, the deserialization instinct is the right one, and I'll happily review the narrower version.

[Hinotoi-agent]

Thanks for the detailed review and suggested scope. I opened the focused follow-up here: #106

It keeps only the load-time artifact containment path in _store.py, drops the payload-ID/OneDrive pieces, adds cross-platform symlink handling in the regression test, and frames the change as defense-in-depth for untrusted collection deserialization.