Skip to content

Sanitize entry/page ids used as export staging paths - #340

Closed
Flerpharos wants to merge 1 commit into
mainfrom
fix/339-export-path-traversal
Closed

Sanitize entry/page ids used as export staging paths#340
Flerpharos wants to merge 1 commit into
mainfrom
fix/339-export-path-traversal

Conversation

@Flerpharos

Copy link
Copy Markdown
Collaborator

Fixes #339

Notebook.export(source="walk") built staging file paths in _walk_tree() (src/labapi/tree/_export.py) directly from API-returned entry and page ids, with no validation:

  • source = entries / entry.id (attachment staging)
  • source = entries / f"{entry.id}.{...}" (text entry staging)
  • source = entries / f"{page.id}.json" (page metadata)

The _writable_name(...) output-name sanitizer was only applied to the files dict key (the final exported name), not to source (the path actually opened for writing). A hostile/compromised id such as ../../outside (or an absolute path) could make source escape the temporary entries staging directory, letting the write overwrite arbitrary process-writable files.

Fix: reuse the existing _writable_name() sanitizer on the id at all three sites before it's used as a staging path component. _writable_name already strips path separators/invalid characters and collapses all-dot names (e.g. ..) to untitled, so a hostile id can no longer produce a path that traverses out of entries. The files dict keys, the on-disk export layout, and the metadata "id" field (which still reports the real, unsanitized id) are unchanged — only the staging filename is affected.

The native backup export path (_backup_tree) was checked too: it builds staging paths from SQLite integer row ids (part["id"], node["id"]), not attacker-controlled strings, so it wasn't affected and wasn't changed.

Added test_walk_tree_sanitizes_hostile_entry_ids in tests/tree/test_export.py, which runs _walk_tree with a page/entry/attachment whose ids contain ../ and asserts every staged file lives directly inside the entries staging directory (no escape) while the export still succeeds and the metadata still reports the real ids.

Test plan

  • uv run pytest --ignore=tests/test_integration.py -q — 431 passed, 1 skipped
  • uv run ruff check . — all checks passed

Notebook.export(source="walk") built staging file paths directly from
API-returned entry and page ids with no validation. A hostile id such
as "../../outside" could escape the temporary entries directory when
the staged file was written, allowing arbitrary process-writable files
to be overwritten.

Reuse the existing _writable_name() sanitizer (already applied to the
exported file/dir names) on the id before it is used as a staging path
component in the three affected spots in _walk_tree(). The reported
metadata still records the real (unsanitized) id; only the on-disk
staging filename changes.

Fixes #339
@Flerpharos

Copy link
Copy Markdown
Collaborator Author

this is overly paranoid? It's not in threat model for LabArchives the API to be giving out hostile ids.

@Flerpharos Flerpharos closed this Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Live export (source=walk) allows path traversal via unsanitized API IDs

1 participant