Repository navigation
[pull] master from vastsa:master - #115
Merged
Merged
Conversation
* fix: constant-time comparison for download tokens (security item 5) Replace the set-membership key check in /share/download with hmac.compare_digest over the two valid window tokens; add negative-path tests (wrong key 403, both windows accepted, foreign-code token 403). * fix: single source for attachment headers + guard tests (security item 2) All six Content-Disposition construction sites across the five storage backends now go through build_attachment_headers; the header is what neutralizes stored XSS on the same-origin download path. Guard tests assert every get_file_response uses the builder and no hand-built disposition reappears. * fix: reject internal-network endpoints for storage config (security item 7) s3_endpoint_url/s3_hostname/webdav_url are fetched server-side; with APP_ENV=production the write entry now enforces http(s) schemes and a loopback/private/link-local/metadata host blacklist. Development env stays open for local minio/webdav. Already-stored values are never re-validated, so existing deployments are unaffected. * fix: JSON 404 for handler-raised 404s; negative-path test batch Starlette status-code handlers take precedence over the HTTPException class handler, so registering the theme index as the 404 handler turned every HTTPException(404) app-wide into a 200 HTML page (found by the new negative-path tests). The new not_found_handler serves the theme page only to browsers (Accept: text/html) and JSON 404 to API clients. Negative-path batch: download count exhaustion, missing chunk session 404s, expired presign session deletion, admin update_file uniqueness/ existence. * fix: run container as non-root app user with compat volume chown (security item 3) Entrypoint starts as root only to fix the data-dir ownership (skipped when nothing needs changing or when the deployer sets an explicit user), then drops to uid 10001 via gosu. Verified in-container: uvicorn runs as app (uid 10001), setup and share round-trip work against a root-owned volume. * fix: compare download tokens as bytes (non-ASCII key caused 500) * test: lock changed-only semantics for endpoint config saves Integration regression for the background lesson: a stored internal endpoint must not block unrelated settings saves in production mode; changing to an internal endpoint (URL or bare-hostname form) is 400. * fix: resolve-based SSRF check — deny IP shorthands, hex/decimal IPs, DNS maps Probe found the static blacklist bypassed by glibc shorthands (127.1, 10.1), decimal (2130706433) and hex IP forms, *.localhost, and loopback-mapping DNS services. Both validators now resolve the host and re-check every resolved address; hostname tier extracts the host part correctly for [v6]/host:port/bare-v6 forms. 44 tests. * test: pin 404 handler dual-branch behavior (browser theme page vs API JSON) * fix: write back normalized endpoint values; hostname dev-gate test * test: header-injection negative paths for attachment disposition * fix: S3 merge buffers parts to >=5MiB; missing object 404 upfront - S3 multipart rejects parts <5MiB (EntityTooSmall) except the last; chunk size is client-controlled (commonly 2-4MB) so multi-chunk merges always failed with 500. Merge now buffers chunks into >=5MiB parts (memory bound: 5MB + one chunk). - get_file_response: a 404 head_object now raises StorageError(404) upfront instead of signing a doomed presigned URL and failing mid-stream (aligns with local backend semantics from the M2 behavior unification). * test: S3 backend coverage via in-process moto server (13 cases) mock_aws cannot intercept aioboto3's aiohttp stack, so tests run against a real in-process ThreadedMotoServer endpoint. Covers roundtrips, missing object 404, merge failure modes (missing chunk / hash mismatch -> abort, no leftover object), cleanup scoping, presign shapes, proxy dispatch. Also adds moto[s3,server] to the dev group and CI. * fix: WebDAV missing object 404 upfront; connection errors map to 503 HEAD 404 was silently swallowed (signed a 200 bad stream that failed mid-download, same family as the S3 bug), and HEAD connection errors were also swallowed so the 503 mapping never fired. Both now surface properly; the aiohttp session is reclaimed on pre-stream exception paths. * test: WebDAV backend coverage via in-process WebDAV server (9 cases) Real HTTP (HEAD/GET/PUT/DELETE/MKCOL/PROPFIND) against a minimal aiohttp WebDAV server over a temp dir: roundtrips, missing object 404, connection error 503, delete with empty-parent cleanup, merge failure modes, chunk cleanup scoping. * fix: OneDrive missing object maps to 404 upfront; OpenDAL behavior pinned OneDrive get_file_response translated graph itemNotFound into the outer 503 catch-all; now raises StorageError(404) like local/S3/WebDAV. OpenDAL already mapped missing objects to 404 via its outer catch — pinned with fake-operator tests (SDK not in runtime deps, instances built via __new__). Also tightens the S3 presign URL assertion. * test: admin write-path coverage (batch/single delete, batch update, policy actions) 13 negative-path cases for the previously untested data-modifying admin endpoints: mixed-id aggregation with duplicates and missing records, empty-list rejections, clear_expired_at permanence semantics, policy action boundaries (zero limit 400, unknown action 400, missing 404), and the DoesNotExist->404 mapping pinned for single delete. * test: admin read-path and view-preset CRUD coverage (22 cases) Detail/metadata/preview/admin-download/activities/local-lists+delete/ verify: missing-record 404s, note/tag truncation limits, preview max_chars truncation, activity filtering and the 80-event clamp, local path traversal rejection. Presets: update-vs-create by id, name truncation, filter normalization clamps, 24-preset cap, delete-missing 404. * refactor: move auth primitives to apps/base/auth, kill base->admin dep JWT create/verify, bearer extraction and the share-upload gate move to apps.base.auth (consumed by both surfaces); admin.dependencies keeps admin session gating and service providers, re-exporting the primitives for import-path compatibility. base.views no longer imports from apps.admin — the last cross-app reverse dependency is gone. * refactor: split admin services god-module (ConfigService/LocalFile out) apps/admin/services.py (1697 lines) becomes FileService (~1200 lines) + a compatibility facade re-exporting ConfigService, LocalFileService, LocalFileClass and keyvalue_write_lock from their new homes (config_service.py holds the D5 KeyValue lock; one-way imports, no cycle). test_admin_security patch targets follow ConfigService to its new module. Behavior unchanged; 247 tests green. * fix: size-less uploads no longer crash (upstream seek bug) validate_file_size's size-None branch called UploadFile.seek(0, 2), which raises TypeError (UploadFile.seek takes one arg); the underlying SpooledTemporaryFile.seek is sync, so awaiting it also fails. Use the underlying file object with sync seek(0, SEEK_END). Surface discovered by the mypy pilot run; regression test covers the size-None branch. * chore: bump direct deps (patch/minor) fastapi 0.139.2->0.141.1, pydantic 2.12.5->2.13.5, uvicorn 0.51.0->0.53.0, aiohttp 3.14.2->3.14.3; lockfile regenerated. tortoise-orm 0.x->1.x major deliberately deferred pending API-change review. * ci: gradual mypy adoption via baseline ratchet scripts/mypy_ratchet.py compares mypy output against scripts/mypy-baseline.txt (line numbers stripped for edit stability): new type errors fail CI, fixes are folded back via --regenerate. Baseline starts at 30 known errors; the seek bug found in the pilot was fixed before baselining. mypy joins the dev group and CI. * fix: migrate WebDAV auth off aiohttp.BasicAuth (deprecated in 4.0) Dependency-upgrade probe on aiohttp 3.14.3 surfaced the BasicAuth deprecation; switch to aiohttp.encode_basic_auth() headers so the aiohttp 4.0 upgrade does not break the WebDAV backend. * chore: drop dead code (WebDAV _instance stub, unused LocalFileClass.write) * test: file-validation negative paths (13 cases) Magic-bytes spoofing (text-as-png, exe-as-pdf, mp4/webp box detection), whitelist rule semantics (star/image-wildcard/content mismatch), chunk-0 header validation parity, and end-to-end UploadFile rejection. * test: migration runner coverage (5 cases) Full chain executes 001-007 in filename order and registers; rerun is idempotent; pre-registered entries are skipped (DDL not re-executed); the resulting schema matches deployment; a failing migration propagates and is not registered. Uses a fresh :memory: DB without generate_schemas so the migrations themselves build the schema (the real deployment path). * refactor: split core/storage.py into a package (per-backend modules) core/storage.py (1400+ lines, six backends) becomes core/storage/ with _base.py (data contracts, interface, shared header builder), and local/s3/onedrive/opendal/webdav modules. The package __init__ re-exports the full historical public surface; test patch targets move to the precise backend modules (core.storage.local.data_root etc.) and the attachment guard scans the package. Import-only change; 264 tests green. Storage-layer DI deferred: current tests construct instances against patched settings, so the added surface isn't justified yet. * refactor: TypedDict for IPRateLimit records The rate-limit ledger mixed int and datetime under a loose Union typing, producing 5 mypy noise errors. A TypedDict gives precise per-key types; setdefault replaces the get-then-assign pattern. One mypy baseline error family resolved; 264 tests green. * chore: tighten mypy baseline after IPRateLimit TypedDict * refactor: resolve all 26 baseline mypy errors; precise test assertions Per-error treatment (no bulk script): honest annotations for StoredDownload (Path/bytes/Callable), KeyValue.value (Any for JSONField), dict[str, Any] for form/config dicts; signature split in create_token; str() normalization in config int(); platform/type-ignore only where the checker cannot follow (msvcrt, FastAPI Request injection). Baseline file and ratchet compare now redundant at zero errors but kept as the guard against regressions. Tests: assertions tightened to status_code == 403 with HTTPException raises; _create_share explicit parameters replace **extra passthrough; seek-fix pointer-reset assertion persisted.
Revert gosu/non-root entrypoint so NAS and root-owned volumes keep working. Reject @/userinfo in s3_hostname. Theme-less checkouts no longer crash the HTML 404 handler.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )