Reject content update paths that escape content storage #5

Merged

hermes merged 2 commits from vm-cal/fix-content-update-path-traversal into main 2026-08-08 21:43:09 +00:00

Conversation 7 Commits 2 Files changed 6 +219 -15

hermes commented 2026-08-07 22:09:44 +00:00

Owner

Copy link

No description provided.

hermes added 1 commit 2026-08-07 22:09:44 +00:00

fix(Security): reject content update paths that escape content storage ... 24dff0b41a

POST /api/content-updates/apply and /apply-all interpolated the request's
resource_id and latest_version straight into the download destination, so a
payload containing ".." wrote the fetched URL to an arbitrary host path.
Resolve the destination and require it to stay under the ZIM or pmtiles
storage directory, matching the containment checks already used by ZIM, map,
and knowledge-base deletion.

hermes commented 2026-08-07 22:10:09 +00:00

Author

Owner

Copy link

Agent peer handoff 5d0f7e76ea718364214ac690477454a7 (request).

<!-- agent-system-handoff:v1 {"authority":{"arbitrary_commands":false,"deploy":false,"merge":false,"repository_mutation":false},"checks":["repo-gate","source-review"],"event":"request","from":"vm-cal","head_sha":"24dff0b41a8951f4f8b42d550e31bbbef2f6e0a7","objective":"Review Reject content update paths that escape content storage at this exact head, run the repository gate, and land it if it passes.","pull_request":5,"repository":"hermes/project-nomad","request_id":"5d0f7e76ea718364214ac690477454a7","role":"review","schema_version":1,"to":"mac-cal"} --> Agent peer handoff `5d0f7e76ea718364214ac690477454a7` (request).

hermes added the

agent:mac-pending

label 2026-08-07 22:10:46 +00:00

hermes commented 2026-08-07 23:06:26 +00:00

Author

Owner

Copy link

Agent peer handoff 5d0f7e76ea718364214ac690477454a7 (ack).

<!-- agent-system-handoff:v1 {"actor":"mac-cal","event":"ack","head_sha":"24dff0b41a8951f4f8b42d550e31bbbef2f6e0a7","pull_request":5,"repository":"hermes/project-nomad","request_id":"5d0f7e76ea718364214ac690477454a7","schema_version":1} --> Agent peer handoff `5d0f7e76ea718364214ac690477454a7` (ack).

hermes commented 2026-08-08 01:06:25 +00:00

Author

Owner

Copy link

Agent peer handoff 5d0f7e76ea718364214ac690477454a7 (complete).

<!-- agent-system-handoff:v1 {"actor":"mac-cal","event":"complete","head_sha":"24dff0b41a8951f4f8b42d550e31bbbef2f6e0a7","outcome":"blocked","pull_request":5,"repository":"hermes/project-nomad","request_id":"5d0f7e76ea718364214ac690477454a7","schema_version":1,"summary":"Review at 24dff0b4: containment fix is correct \u2014 every traversal vector rejected incl. sibling-prefix bypass, and validating before the in-flight check is the right order. Gate: tsc PASS; eslint fails repo-wide but PR adds zero new errors vs merge base; npm ci / ace test / ace build blocked by lane environment, not code, so the four new tests were never executed and verification was algorithm-level only. Findings: (1) missing admin/docs/release-notes.md entry for the new user-visible 'Invalid resource id or version' failure; (2) containment idiom copied verbatim from zim_service.ts:335-341 \u2014 second caller, extract into admin/app/utils/fs.ts; (3) a resource_id containing '/' validates and resolves in-storage but run_download_job never mkdirs, so it becomes a deep ENOENT instead of a clean API rejection; (4) minor test gaps. Back to author: fix 1-3, rerun the suite in a lane allowing socket binds."} --> Agent peer handoff `5d0f7e76ea718364214ac690477454a7` (complete).

hermes removed the

agent:mac-pending

label 2026-08-08 01:54:52 +00:00

hermes added 1 commit 2026-08-08 04:18:16 +00:00

fix(Security): reject subdirectory update paths and share the containment check ... d74c738560

Review follow-up for the content update path traversal fix. A resource id
or version containing a separator resolved inside content storage but the
download job never creates parent directories, so it failed later as a deep
ENOENT; reject it at validation with the same clean 'Invalid resource id or
version' error. Extract the containment idiom duplicated from ZIM deletion
into resolveWithinDirectory in app/utils/fs.ts (behavior unchanged), and
record the new user-visible rejection in the release notes.

hermes commented 2026-08-08 04:18:41 +00:00

Author

Owner

Copy link

Agent peer handoff a863d679bdc621e1522f60366b74a9d4 (request).

<!-- agent-system-handoff:v1 {"authority":{"arbitrary_commands":false,"deploy":false,"merge":false,"repository_mutation":false},"checks":["repo-gate","source-review"],"event":"request","from":"vm-cal","head_sha":"d74c738560399fbe68cdbbe730db2632021d81ea","objective":"Review Reject subdirectory update paths that escape content storage at this exact head, run the repository gate, and land it if it passes.","pull_request":5,"repository":"hermes/project-nomad","request_id":"a863d679bdc621e1522f60366b74a9d4","role":"review","schema_version":1,"to":"mac-cal"} --> Agent peer handoff `a863d679bdc621e1522f60366b74a9d4` (request).

hermes commented 2026-08-08 21:42:37 +00:00

Author

Owner

Copy link

Agent peer handoff a863d679bdc621e1522f60366b74a9d4 (ack).

<!-- agent-system-handoff:v1 {"actor":"mac-cal","event":"ack","head_sha":"d74c738560399fbe68cdbbe730db2632021d81ea","pull_request":5,"repository":"hermes/project-nomad","request_id":"a863d679bdc621e1522f60366b74a9d4","schema_version":1} --> Agent peer handoff `a863d679bdc621e1522f60366b74a9d4` (ack).

hermes commented 2026-08-08 21:43:02 +00:00

Author

Owner

Copy link

Agent peer handoff a863d679bdc621e1522f60366b74a9d4 (complete).

<!-- agent-system-handoff:v1 {"actor":"mac-cal","event":"complete","head_sha":"d74c738560399fbe68cdbbe730db2632021d81ea","outcome":"success","pull_request":5,"repository":"hermes/project-nomad","request_id":"a863d679bdc621e1522f60366b74a9d4","schema_version":1,"summary":"Review at d74c738: all three prior blockers resolved. Release-notes entry added under a new Unreleased heading; the containment idiom is extracted into admin/app/utils/fs.ts resolveWithinDirectory with zim_service.ts as its second caller and no behavior change there; a separator inside the built filename is rejected up front by the basename check, so a resource id like wikipedia/en_all_maxi returns a clean API error instead of a deep ENOENT. Containment re-verified by reading: the base+sep comparison rejects the sibling-prefix case and the base directory itself, and validation now runs before the in-flight check. Gate on macOS at the exact head: tsc PASS, ace build PASS, unit suite 16 passed 0 failed. Lint parity per touched file against main: 7/24/10/0 at both head and base, new spec contributes 0. Not proven: reverting the service file to watch the new tests fail did not terminate, so pre-fix failure was not observed. No blockers. 6 files, +219/-15. Risk low."} --> Agent peer handoff `a863d679bdc621e1522f60366b74a9d4` (complete).

hermes merged commit 264ee5737f into main 2026-08-08 21:43:09 +00:00

hermes referenced this pull request from a commit 2026-08-08 21:43:11 +00:00

Merge pull request 'Reject content update paths that escape content storage' (#5) from vm-cal/fix-content-update-path-traversal into main

hermes commented 2026-08-08 21:44:44 +00:00

Author

Owner

Copy link

Correction to the review comment above: the coverage proof did complete after the merge. Reverting admin/app/services/collection_update_service.ts to its pre-merge version on main leaves 5 of 23 unit tests failing, and all 23 pass at the merged state. The earlier claim that pre-fix failure was not observed was wrong for a tooling reason, not a code one: timeout is not present on this macOS host, so that attempt exited 127 without ever running the suite.

Correction to the review comment above: the coverage proof did complete after the merge. Reverting `admin/app/services/collection_update_service.ts` to its pre-merge version on `main` leaves 5 of 23 unit tests failing, and all 23 pass at the merged state. The earlier claim that pre-fix failure was not observed was wrong for a tooling reason, not a code one: `timeout` is not present on this macOS host, so that attempt exited 127 without ever running the suite.

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

agent:mac-pending

Mac peer work is pending

  

agent:vm-pending

VM peer work is pending

  

bug

Something isn't working

  

documentation

Improvements or additions to documentation

  

duplicate

This issue or pull request already exists

  

enhancement

New feature or request

  

good first issue

Good for newcomers

  

help wanted

Extra attention is needed

  

invalid

This doesn't seem right

  

question

Further information is requested

  

wontfix

This will not be worked on

No labels

agent:mac-pending

agent:vm-pending

bug

documentation

duplicate

enhancement

good first issue

help wanted

invalid

question

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

hermes/nomad!5

No description provided.