Skip to content

fix(api): confine absolute-path asset URIs to the asset directory#3117

Merged
vpetersson merged 2 commits into
masterfrom
fix/asset-uri-path-confinement
Jul 6, 2026
Merged

fix(api): confine absolute-path asset URIs to the asset directory#3117
vpetersson merged 2 commits into
masterfrom
fix/asset-uri-path-confinement

Conversation

@vpetersson

Copy link
Copy Markdown
Contributor

Summary

The asset-create API accepts an absolute-path (/-prefixed) uri after only an existence check, then moves it into the asset store and serves it back — a pre-auth arbitrary file read (and destructive move) on a default install.

  • validate_uri (api/serializers/__init__.py) only ran path.isfile(uri) for a /-prefixed URI, with no confinement to the asset directory.
  • prepare_asset then renames that file into <assetdir>/<uuid> (source removed), and AssetContentViewMixin.get reads it back as base64.
  • Because auth_backend defaults to '', @authorized no-ops, so any unauthenticated LAN client could:
    1. POST /api/v2/assets {"uri": "/data/.anthias/anthias.conf", "mimetype": "image", ...} → the file is moved into the asset store.
    2. GET /api/v1/assets/<id>/content → base64 of anthias.conf, which contains django_secret_key (enough to forge signed sessions and the internal-auth token). Targeting anthias.db discloses operator password hashes; either target is also destroyed by the move.

Fix

Confine /-prefixed URIs to settings['assetdir'] in the shared validate_uri, applying realpath to both sides so a symlink staged inside the asset dir can't resolve back out. The trailing separator prevents a sibling-prefix bypass (<assetdir>_evil).

The only legitimate absolute-path flow is the two-step file_asset upload, which stages files as <assetdir>/<uuid>.tmp — inside the asset dir, so it is unaffected. The HTML create path already rejects absolute paths via validate_url; this gap was API-only.

Tests

Adds test_create_asset_rejects_absolute_uri_outside_assetdir (parametrized across v1/v1.1/v1.2/v2): a real file placed outside the asset dir is rejected before the rename, no row is persisted, and the source file is left intact. Verified the confinement accepts a genuine in-assetdir path and blocks /etc/passwd, the sibling .anthias/anthias.conf, .. traversal, and the _evil sibling-prefix case.

🤖 Generated with Claude Code

The asset-create serializers accept a `/`-prefixed `uri` after only an
`isfile()` check, then `rename` it into the asset store and serve it
back verbatim via the content endpoint. With auth disabled (the default
install), any LAN client could POST `uri=/data/.anthias/anthias.conf`
to read `django_secret_key` (or the SQLite DB's password hashes) and,
because `rename` moves the source, corrupt the device.

Confine `/`-prefixed URIs to `settings['assetdir']` in the shared
`validate_uri`, resolving symlinks on both sides so a staged symlink
can't escape. The only legitimate absolute-path flow — the two-step
`file_asset` upload that stages files as `<assetdir>/<uuid>.tmp` —
stays within the asset dir and is unaffected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vpetersson vpetersson requested a review from a team as a code owner July 6, 2026 18:27
@vpetersson vpetersson requested a review from Copilot July 6, 2026 18:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a security flaw in the asset-create API where absolute-path (/-prefixed) uri values could reference arbitrary readable files on the host, which were then moved into the asset store and retrievable via the content endpoint.

Changes:

  • Added _is_within_assetdir() and applied it in validate_uri() to confine absolute-path URIs to settings['assetdir'] using realpath + trailing-separator prefix checks.
  • Added a regression test ensuring asset creation rejects absolute-path URIs that resolve outside the asset directory and that no destructive rename occurs.

Reviewed changes

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

File Description
src/anthias_server/api/tests/test_assets.py Adds regression test for rejecting absolute-path URIs outside the asset directory and ensuring no file move / DB persistence occurs.
src/anthias_server/api/serializers/__init__.py Introduces _is_within_assetdir() and updates validate_uri() to reject absolute-path URIs outside the configured asset directory.
Comments suppressed due to low confidence (1)

src/anthias_server/api/serializers/init.py:60

  • validate_uri raises a generic Exception for invalid user input. In these API views, that can bubble up as a 500 instead of a structured 400 validation response; with the new assetdir confinement, more invalid requests will hit this path. Prefer raising DRF ValidationError keyed to the uri field so serializer.is_valid() returns False and the views respond with HTTP 400.
def validate_uri(uri: str) -> None:
    if uri.startswith('/'):
        if not _is_within_assetdir(uri) or not path.isfile(uri):
            raise Exception('Invalid file path. Failed to add asset.')
    else:
        if not validate_url(uri):
            raise Exception('Invalid URL. Failed to add asset.')


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/anthias_server/api/tests/test_assets.py Outdated
Address review: raise DRF ValidationError keyed on `uri` from
`validate_uri` so `serializer.is_valid()` catches it and the create
view returns a 400 rather than a 500 (`validate_uri` only runs inside
the create serializers' `prepare_asset`). The regression test now
asserts the exact 400 and the `uri` error key, so it can't pass on an
unrelated server error.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@sonarqubecloud

sonarqubecloud Bot commented Jul 6, 2026

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

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

@vpetersson vpetersson merged commit b6d797e into master Jul 6, 2026
10 checks passed
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.

2 participants