Skip to content

Reject URI-shaped output and mirror paths #3603

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stranske merged 4 commits into from
Sep 27, 2026
Merged

Reject URI-shaped output and mirror paths #3603

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1a78646
Validate output and mirror filesystem references
Sep 27, 2026
d3d5eda
fix(mirror_root): allow Windows drive roots without subdirectory
Sep 27, 2026
701e7b4
fix(schema): reject parent-directory traversal in mirror_root
Sep 27, 2026
727660f
style: black-format traversal test loop in test_backplane_schemas
Sep 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions docs/contracts/document-mirror-v1.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ validators, HTML resolvers, and the future `doc-mirror` CLI can share one schema
HTTPS links are not valid upstream web resolvers.
- `blob_path` is relative to `mirror_root` using POSIX `/` separators. Absolute
paths, `..` traversal, Windows drive prefixes (`C:`), UNC prefixes (`//` or
`\\`), and backslashes are rejected.
`\\`), URI-style prefixes, empty path segments, trailing separators, and
backslashes are rejected.
- `mirror_root` names an absolute or repo-local filesystem directory. URI roots
such as `https://host/mirror` and `file:///tmp/mirror` are rejected.
- Optional `supersedes_content_sha256` records checksum supersession without
requiring consumers to import Pension-Data ingest helpers.

Expand All @@ -42,10 +45,10 @@ validators, HTML resolvers, and the future `doc-mirror` CLI can share one schema
| Field | Requirement |
| --- | --- |
| `schema_version` | Must be the literal `document-mirror/v1`. |
| `mirror_root` | Root directory of the mirrored blob store. |
| `mirror_root` | Absolute or repo-local filesystem root of the mirrored blob store; URI roots are rejected. |
| `blobs` | Array of blob records (may be `[]` for an initialized empty catalog). |
| `blobs[].content_sha256` | Lowercase 64-hex SHA-256 of the blob bytes. |
| `blobs[].blob_path` | Mirror-relative POSIX path (no `..`, absolute paths, drive/UNC prefixes, or backslashes). |
| `blobs[].blob_path` | Mirror-relative POSIX path (no URI prefix, `..`, absolute path, drive/UNC prefix, backslash, empty segment, or trailing separator). |
| `blobs[].doc_type_id` | Fleet vocabulary token for the document type. |
| `blobs[].source_refs` | Array of canonical refs and/or source-system objects; may be `[]` for local-only blobs. |

Expand Down
7 changes: 4 additions & 3 deletions docs/contracts/output-substrate-v1.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ not change the wire shape of this document.
| --- | --- |
| `schema_version` | Must be the literal `output-substrate/v1`. |
| `renderer_profile` | One of `investment_review`, `blackline_bundle`, `mosaic_book`. |
| `workspace_bundle_ref` | Run-dir-relative POSIX pointer to the view bundle JSON (`path` required; `sha256` and `artifact_id` recommended). Rejects absolute paths, `..` traversal, backslashes, drive-letter roots (`C:`), and UNC paths (`//server/share`). |
| `workspace_bundle_ref` | Run-dir-relative POSIX pointer to the view bundle JSON (`path` required; `sha256` and `artifact_id` recommended). Rejects absolute paths, URI-style prefixes, `..` traversal, backslashes, drive-letter roots (`C:`), empty path segments, and UNC paths (`//server/share`). |
| `manifest_ref` | `artifact:manifest.json` or a run-dir-relative POSIX path to the [`artifact-manifest/v1`](schemas/artifact-manifest-v1.schema.json) manifest. Absolute paths, traversal, backslashes, drive roots, leading URI-style prefixes, and empty path segments are rejected. Colons in later path segments are allowed. Named artifacts live there, not inline. |
| `manifest_csv_exports` | Array (possibly empty) of manifest-gated CSV export specs for Excel refresh. |

Expand All @@ -42,8 +42,9 @@ Pension-Data-style `artifactBaseUrl` resolution.

## Manifest CSV exports

Each `manifest_csv_exports[]` entry names a generated CSV file, its encoding
(`utf-8` or `utf-16-le`), and a nonempty `columns[]` list. Every column
Each `manifest_csv_exports[]` entry names a run-dir-relative generated CSV file,
rejecting URI-style prefixes, traversal, and empty path segments. It also declares
the encoding (`utf-8` or `utf-16-le`) and a nonempty `columns[]` list. Every column
requires `name`, `type` (`string`, `number`, `boolean`, or `date`), and
`source_path` (JSONPath or consumer-defined pointer into the workspace bundle).
Producers regenerate CSV files when the workspace bundle changes; Excel workbooks
Expand Down
7 changes: 4 additions & 3 deletions docs/contracts/schemas/document-mirror-v1.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
"mirror_root": {
"type": "string",
"minLength": 1,
"description": "Filesystem root of the mirrored blob store (absolute or repo-local path)."
"description": "Filesystem root of the mirrored blob store (absolute or repo-local path); URI schemes are not filesystem roots.",
"pattern": "^(?:(?:[A-Za-z]:[\\\\/])|(?!(?:[A-Za-z][A-Za-z0-9+.-]*:))).+"

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Permit a drive root as an absolute mirror directory

When a Windows catalog uses the drive root itself as mirror_root, the new pattern rejects both C:/ and C:\: the drive-prefix alternative consumes the separator, but the trailing .+ still requires another character. These are valid absolute filesystem directories under the documented contract, so the drive-root branch should allow the path to end immediately after the separator.

Useful? React with 👍 / 👎.

@stranske stranske Sep 27, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d3d5eda. The mirror_root pattern now restructures the drive-path alternative to (?:[A-Za-z]:[\\/](?:.+)?) — making the subdirectory suffix optional after the separator. C:/ and C:\ are now accepted as valid drive roots alongside C:/mirror and C:\\mirror. Added both as explicit valid-root test cases in test_document_mirror_fixture_validates; all 189 contract tests pass.

},
"created_at": {
"type": "string",
Expand Down Expand Up @@ -48,8 +49,8 @@
"blob_path": {
"type": "string",
"minLength": 1,
"description": "Path relative to mirror_root. Absolute POSIX paths, Windows drive/UNC prefixes, backslashes, and '..' traversal are rejected by the validator.",
"pattern": "^(?!/)(?!//)(?![A-Za-z]:)(?!.*\\\\)(?!.*(^|/)\\.\\.(/|$)).+$"
"description": "Path relative to mirror_root. Absolute paths, URI schemes, Windows drive/UNC prefixes, backslashes, '..' traversal, empty path segments, and trailing separators are rejected by the validator.",
"pattern": "^(?!/)(?![A-Za-z]:)(?![A-Za-z][A-Za-z0-9+.-]*:)(?!.*\\\\)(?!.*(^|/)\\.\\.(/|$))(?!.*//)(?!.*\\/$).+$"
},
"doc_type_id": {
"type": "string",
Expand Down
6 changes: 3 additions & 3 deletions docs/contracts/schemas/output-substrate-v1.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@
"path": {
"type": "string",
"minLength": 1,
"description": "Run-dir-relative POSIX path. Absolute paths, '..' traversal, backslashes, drive-letter roots, and UNC paths are rejected by the validator.",
"pattern": "^(?!/)(?![A-Za-z]:)(?!.*\\\\)(?!.*(^|/)\\.\\.(/|$))(?!.*\\/$).+"
"description": "Run-dir-relative POSIX path. Absolute paths, URI schemes, '..' traversal, backslashes, drive-letter roots, empty path segments, and UNC paths are rejected by the validator.",
"pattern": "^(?!/)(?![A-Za-z]:)(?![A-Za-z][A-Za-z0-9+.-]*:)(?!.*\\\\)(?!.*(^|/)\\.\\.(/|$))(?!.*//)(?!.*\\/$).+"
},
"sha256": {
"type": "string",
Expand Down Expand Up @@ -70,7 +70,7 @@
"filename": {
"type": "string",
"minLength": 1,
"pattern": "^(?!\\.$)(?!.*(?:^|/)\\.$)(?!/)(?![A-Za-z]:)(?!.*\\\\)(?!.*(^|/)\\.\\.(/|$)).*[^/]$"
"pattern": "^(?!\\.$)(?!.*(?:^|/)\\.$)(?!/)(?![A-Za-z]:)(?![A-Za-z][A-Za-z0-9+.-]*:)(?!.*\\\\)(?!.*(^|/)\\.\\.(/|$))(?!.*//).*[^/]$"
},
"encoding": {
"type": "string",
Expand Down
20 changes: 20 additions & 0 deletions tests/contracts/test_backplane_schemas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,10 @@ def test_document_mirror_fixture_validates() -> None:
"../escape.pdf",
"/absolute.pdf",
"blobs/../../escape.pdf",
"https://host/document.pdf",
"file:///tmp/document.pdf",
"blobs//document.pdf",
"blobs/",
"C:\\Users\\doc.pdf",
"C:/Users/doc.pdf",
"\\\\server\\share\\doc.pdf",
Expand All @@ -166,6 +170,16 @@ def test_document_mirror_fixture_validates() -> None:
broken["blobs"][0]["blob_path"] = path
assert list(validator.iter_errors(broken)), path

for root in ("https://host/mirror", "file:///tmp/mirror", "x:relative"):
broken = json.loads((FIXTURES / "valid_document_mirror.json").read_text())
broken["mirror_root"] = root
assert list(validator.iter_errors(broken)), root

for root in ("repo/mirror", r"C:\mirror"):
valid_root = json.loads((FIXTURES / "valid_document_mirror.json").read_text())
valid_root["mirror_root"] = root
assert not list(validator.iter_errors(valid_root)), root


def test_tracked_variable_fixture_validates() -> None:
validator = _validator("tracked-variable-v1.schema.json")
Expand Down Expand Up @@ -399,7 +413,13 @@ def test_output_substrate_rejects_missing_renderer_profile() -> None:
("workspace_bundle_ref", r"\\server\share\workspace.json"),
("workspace_bundle_ref", "//server/share/workspace.json"),
("workspace_bundle_ref", "bundles/workspace/"),
("workspace_bundle_ref", "https://host/workspace.json"),
("workspace_bundle_ref", "file:///tmp/workspace.json"),
("workspace_bundle_ref", "bundles//workspace.json"),
("manifest_csv_exports", r"C:\exports\facts.csv"),
("manifest_csv_exports", "https://host/facts.csv"),
("manifest_csv_exports", "file:///tmp/facts.csv"),
("manifest_csv_exports", "exports//facts.csv"),
],
)
def test_output_substrate_rejects_non_posix_relative_paths(field: str, path: str) -> None:
Expand Down
Loading