From 47346b930936d34a44b94efc326e6a5fcb75f7fd Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Sun, 13 Sep 2026 13:21:05 +0200 Subject: [PATCH] chore: ambition and target --- AGENTS.md | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/AGENTS.md | 15 ++++++++++++ 2 files changed, 81 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 0e10a030..10443559 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,6 +2,72 @@ This file provides guidance to coding agents (Claude Code, Codex, Cursor, Aider, …) working with this repository. Claude Code reads it via `@AGENTS.md` in `CLAUDE.md`. +# Purpose — what OxiCloud is, and what it is not + +Read this before designing anything. Most "should we…" questions are answered by +the scale target rather than by taste. + +- **Open source, and it stays that way.** MIT (`LICENSE`). This is a constraint on + what you may add, not a footnote: dependencies must be license-compatible — a + GPL/AGPL crate or npm package would force the whole project to relicense, and + that is not on the table. No feature may be gated behind a licence key or an + "enterprise edition", and nothing core may hard-depend on a proprietary service + or SDK. Vendored frontend assets (`frontend/static/vendors/`) carry the same + rule; record the licence when vendoring. +- **Self-hosted**, for an individual or an enterprise. The operator is not an SRE + team: defaults must be safe, failures loud, and nothing may silently depend on a + cloud service. +- **Target scale is up to ~10k users.** Design against that number in both + directions. Do not build million-user machinery (sharding, eventual consistency, + service decomposition) for a load that will never arrive; equally, never ship + anything O(users) per request, or a table scan that is fine at 50 users and + fatal at 10k. +- **Not a mass hoster.** OxiCloud does not claim to serve millions of users on one + deployment, and trade-offs should not pretend otherwise. +- **Decentralised by intent.** Many instances federating beats one large instance — + OpenCloudMesh is one route. Prefer designs that survive "this is one of many + instances" over ones assuming a single authoritative deployment. + +Targets: + +- **Feature ambition: Google Workspace / Office 365.** Breadth of capability is a + goal, not scope creep. +- **Collaboration is the main feature axis.** OxiCloud is not a personal backup + drive that happens to have sharing bolted on — sharing, shared drives, grants, + co-editing (WOPI) and live updates are the product. When choosing what to build + or how to build it, the multi-user case is the primary one, not the case to + generalise to later. A feature that works only for a single owner is unfinished. +- **Customer target: NextCloud users.** Hence the NextCloud-compatible API surface + (`/remote.php`, `/ocs`, `/status.php`) — compatibility is a feature, and breaking + it costs adopters. + +## Design axes + +Four things decide an open design question. **Security and resilience are +absolute** — they are not traded against anything. Performance is measured against +the 10k target. Privacy is a direction with a stated endpoint. + +- **Resilience.** This is a storage product: **no data loss, no data corruption, + ever.** Anything that can silently drop or alter bytes is a top-severity defect, + not a trade-off. In practice that means: a job that skips work must never report + success (pause at a cursor instead — `docs/plan/jobs-handling-recoverable-error.md`); + a read failure is never proof that data is absent; content-addressing and + ref-counting are load-bearing, not decoration; and consistency checks are + discovery-only unless repair is explicitly requested. +- **Security.** Prefer deny-by-default over assert-later; a guarantee enforced by + the type system or the router beats one a reviewer must remember. AuthZ lives in + the service layer, never in handlers. See `src/AGENTS.md` § AuthZ enforcement + points. +- **Performance.** Measure against 10k users, not a dev instance. The hot paths are + listing, thumbnails and auth — a per-row query or an extra round trip there is a + real regression even when it looks harmless. +- **Privacy.** When the backend belongs to a third party (S3, Azure), encryption at + rest is a *should-have*; **end-to-end encryption is the target.** Designs that + assume the server can always read plaintext will have to be undone — the `Vault` + drive kind is reserved for the E2E case. + +Where two conflict, resilience and security win, and the cost is documented. + # Architecture This project is split into two parts: diff --git a/src/AGENTS.md b/src/AGENTS.md index 1a653278..8c8fb483 100644 --- a/src/AGENTS.md +++ b/src/AGENTS.md @@ -15,6 +15,15 @@ Non-obvious rules that trip up new code. Terse on purpose. - Any new endpoint that mints or consumes credentials/tokens must consult one of the `is_*_login_allowed()` helpers, not the raw allowlist. - Any new "policy-disabled" refusal must emit an `audit`-target line before returning — matches `auth.login_rejected`, `magic_link.redemption_rejected` conventions. +## AuthZ enforcement points + +- **The extractors are NOT a choke point.** Four paths authenticate without ever touching `AuthUser` / `CurrentUserId`: `middleware/admin.rs::require_authenticated` (re-parses the JWT from header *or* cookie itself), the three DAV handlers' hand-rolled `extract_user` (`webdav_handler.rs:154`, `caldav_handler.rs:421`, `carddav_handler.rs:173`), `POST /api/auth/refresh` (mounted outside `auth_middleware`, `main.rs:775`), and `GET /api/rt/ws` (self-auths from a raw Bearer and never reads `claims.role`, `rt_ws.rs:240-255`). A rule added to an extractor does not hold until it is added to these too. +- **Never hand-roll principal extraction.** `req.extensions().get::>()` inside a handler is exactly the anti-pattern above — it bypasses every `FromRequestParts` guard. Take the extractor, or call the shared assertion. +- **A method check is not an authorization check.** GETs that mint credentials or write exist: `GET /api/wopi/editor-url` returns a WOPI token usable for `POST /wopi/files/{id}/contents`; `GET /api/s/{token}` writes via `register_shared_link_access`; `GET /api/auth/device/verify` is an oracle on live device codes; `GET /api/batch/download` builds an arbitrary ZIP from a querystring. Never gate on verb alone. +- **An auth helper's `_ =>` arm must DENY.** Two fail-open gates exist and are bugs, not patterns to copy: `require_internal_user` (`middleware/user.rs:64-71`) admits the caller on *any* `get_user_flags` error, and `decide_live_role` (`:169-176`) resurrects the claim role on a transient DB error. The first is the only middleware guarding all three DAV surfaces. +- **Prefer deny-by-default over assert-in-handler.** A restriction enforced inside the extractor covers ~200 call sites with no edits; the same restriction as "handler takes an optional principal and asserts" is one forgotten call away from silently accepting. `OptionalUserId` (`middleware/auth.rs:85-98`) is the cautionary tale — it exists, it is dead code, and nothing ever used it. +- Anonymous-session direction (share links as a principal): `docs/plan/rationalize-publicshare.md`. + ## Storage backend access - **Read blob content through `Arc`.** It's the ONE canonical read abstraction — CDC-manifest-aware (`file.blob_hash` may reference a chunk manifest, not a blob), backend-agnostic (Local/S3/Azure), wrapper-transparent (encryption/retry/cache). Never take `Arc` directly in a service that reads content; you'll silently break on any file ≥ 64 KiB (`CDC_MIN_CHUNK`). Follow `thumbnail_service`, `audio_metadata_service`, `media_metadata_service`, `face_indexing_service`, `search_index::content_index_worker` as reference impls. @@ -30,3 +39,9 @@ Non-obvious rules that trip up new code. Terse on purpose. - **After adding: `cargo run --bin generate-openapi`** to regenerate `resources/gen/openapi.json`, then `git diff resources/gen/openapi.json` — the new path + its request/response schemas must be present. Zero-diff means you missed the registration. - Sanity check for the whole surface: `diff <(grep -oE 'path = "/api[^"]+"' src/interfaces/api/handlers/*.rs | grep -oE '/api[^"]+' | sort -u) <(jq -r '.paths | keys | .[]' resources/gen/openapi.json | sort -u)` — should always be empty. Non-empty diff = drift. - Handlers referenced by the `paths(...)` list MUST be `pub` (module-visible from the paths list). Private `async fn` compiles at the router mount but breaks the paths list with a visibility error — see `get_smtp_info`, `send_smtp_test`, `get_user_profile` for the retrofit. + +### Security / scope in the spec + +- **`security(("bearerAuth" = []))` — the empty array is the SCOPES list**, not decoration. Every route currently declares the same thing, so the spec claims "a session is required" for `GET /api/version` and `PUT /api/admin/users/{id}/role` alike: true, and useless. If a route's gate differs from the default, declare it there. +- OpenAPI has **no field for a minimum role** — OAuth2 has no role concept, so the spec has nowhere to put one. Use a pseudo-scope (`["role:admin"]`) rather than a vendor extension no tooling renders. +- **Declaring is not enforcing.** utoipa's `security` wires nothing, so it drifts from the real gate silently. Any scope worth declaring is worth a test cross-checking it against the actual mount — otherwise the spec becomes a parallel description of the authorization boundary rather than a picture of it.