mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-27 13:40:37 +00:00
31 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Xinmin Zeng
|
d48ea2a9a3
|
refactor: remove unused proxy policy and private constants (#7554)
* refactor: remove unused proxy policy and private constants * refactor(sandbox): share provisioner runtime mount path |
||
|
Hyeonsang Cho
|
66508d7522
|
fix(auth): map malformed OIDC provider bodies to OIDCError (#7258)
* fix(auth): map malformed OIDC provider bodies to OIDCError The SSO login route answers OIDCError with 502 and the callback with an sso_failed redirect, but oidc.py raised OIDCError only for transport and HTTP status errors. A provider answering 200 with an HTML page or any non-object JSON raised JSONDecodeError, KeyError or AttributeError instead, so both routes failed with a 500. A malformed UserInfo body also aborted the login, although UserInfo is best-effort. Decode discovery, token, JWKS and UserInfo bodies through one helper that requires a JSON object, require string endpoints and issuer, reject a non-list JWKS keys field, skip non-object JWK entries, and wrap a malformed ID token header as OIDCValidationError. Build discovery metadata before caching it so a rejected document is not served from the cache. * docs(changelog): reference #7258 for the OIDC malformed body fix * fix(auth): report a falsy non-string OIDC issuer as wrong-typed Check the issuer's type before its presence so 0, false and [] get the non-string message instead of "missing the issuer field"; an absent or empty issuer still reports missing. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Huang-404-Q
|
c0ba948825
|
fix(auth): treat a signed token with wrong claims as malformed (#7266)
* fix(auth): treat a signed token with wrong claims as malformed
decode_token caught jwt.PyJWTError but not pydantic.ValidationError, so a
correctly signed token without sub/exp raised out of the function. The
OIDC state cookie shares jwt_secret and has exactly that shape, and the
OAuth start endpoint is public, so an unauthenticated caller could turn
it into a 500 (or a broken WebSocket handshake) instead of a 401.
Catch ValidationError and return TokenError.MALFORMED.
The regression test signs an OIDC-state-shaped payload and asserts
MALFORMED; it raises on main.
* fix(auth): treat a signed OIDC state cookie with bad claims as unusable
_verify_state_signed had the same latent shape as decode_token: it caught
only jwt.PyJWTError, so a state cookie signed with jwt_secret but missing
required claims raised pydantic.ValidationError straight into the public
/api/v1/auth/callback/{provider} endpoint instead of returning None and
letting the handler answer 403.
Also pin the decode_token precedence a reviewer raised: an expired token
that is missing sub reports EXPIRED because jwt.decode raises before the
payload model runs.
* docs: reduce inherited backend guidance size
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
jswSpace
|
62a5b6cc26
|
fix(auth): preserve OIDC authorization endpoint query parameters (#6831)
* fix(auth): preserve OIDC authorization endpoint query parameters * docs(auth): keep updated guidance within size budget * docs(auth): clarify OIDC endpoint query configuration --------- Co-authored-by: jswSpace <198684035+jswSpace@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Nan Gao
|
db7340d2fa
|
fix(security): add DEER_FLOW_CREDENTIALS_KEY and wire the channel credentials cipher (#6611)
* fix(security): add DEER_FLOW_CREDENTIALS_KEY for at-rest credential encryption
There was no at-rest encryption key. ChannelCredentialCipher had no
production caller: the channel service, the scheduled-task notification
outbox and the channel-connections router all built
ChannelConnectionRepository without a cipher, so store_credentials refused
(RuntimeError) and get_credentials always returned None.
- New harness module deerflow.config.credentials_key: env-only
DEER_FLOW_CREDENTIALS_KEY (comma-separated Fernet keys; first encrypts,
all decrypt via MultiFernet), CredentialsCipher with encrypt_text /
decrypt_text / rotate_text, a fernet:v2: prefix with legacy fernet:v1:
values still readable, and get_credentials_cipher() as the process-wide
entry point. Malformed keys are refused without echoing key material.
Without the variable a single instance generates
{base_dir}/.credentials_key. Never derived from AUTH_JWT_SECRET.
- New deerflow.config.secret_file.read_or_create_secret_file: O_EXCL
create (0600) plus read-back with a bounded settle window, shared by
.credentials_key, .jwt_secret (fixes two replicas cold-starting on a
shared volume keeping different JWT secrets) and the managed-model key.
- ChannelCredentialCipher now builds on CredentialsCipher (same
constructors); the Gateway loads the cipher off the event loop at
startup when channel_connections is enabled and passes it to every
ChannelConnectionRepository. Undecryptable values stay "missing + warn".
- Startup gate: a malformed key stops startup in any topology; a declared
multi-instance deployment with channel_connections enabled and no key
is refused. Plain multi-worker shares the generated file and is allowed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* feat(deploy): generate and inject DEER_FLOW_CREDENTIALS_KEY in Helm, compose and deploy.sh
- Helm: the app Secret generates a Fernet key (randBytes 32 translated to
the urlsafe alphabet) and preserves it across upgrades via lookup like
AUTH_JWT_SECRET; the gateway reads it through an optional secretKeyRef
so a user-managed existingAppSecret without it still boots (the Gateway
gate refuses only multi-instance deployments that store credentials).
README, NOTES and values document backup, rotation and existingAppSecret.
- scripts/deploy.sh resolves the key shell -> .env -> persisted file ->
generated, persisting it as $DEER_FLOW_HOME/.credentials_key (0600,
noclobber/O_EXCL with read-back): the same file the Gateway generates in
its runtime home, so either side's key is the one both keep.
- Both compose files pass DEER_FLOW_CREDENTIALS_KEY=${...:-} to the gateway.
- .env.example entry with a generation hint; README and CHANGELOG notes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(security): make abandoned secret-file replacement single-winner
A rename is last-writer-wins, so replicas that restart together and all
find the same abandoned empty secret file could each replace it and keep
a different value. The process that exclusively creates
<name>.replacing now owns the replacement, re-checks the file is still
empty, and publishes by renaming the claim over it; peers wait for that
value, bounded so a crashed replacer cannot block startup forever.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* docs(changelog): link DEER_FLOW_CREDENTIALS_KEY entries to #6611
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(security): name the setting that turns off a credentials-key consumer
The multi-instance refusal ended with "or disable
channel_connections.enabled=true"; it now says "or set
channel_connections.enabled=false".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(security): never expose a secret file before its value is written
O_EXCL published the name before its content, so a creator that stalled
mid-write left an empty file a peer replaced after the settle window; the
resumed creator then returned a value nobody persisted, or deleted the
peer's valid file when its own write failed. Creators now write and sync
a private temporary file and hard-link it to the name, which fails if
the name exists; a failed creator only removes its own temporary file.
Filesystems without hard links publish through the same single-winner
<name>.replacing claim that replaces abandoned empty files, so creation
and replacement are serialized there.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(models): recover an empty managed-model key when no catalog exists
An interrupted first save could leave an empty key file; every later save
then passed empty bytes to Fernet and the endpoint returned 503 until the
file was removed by hand. An empty key with no catalog encrypted nothing,
so it now goes through the secret-file recovery path; next to an existing
catalog it is still refused.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* fix(docker): pass the dotenv DEER_FLOW_CREDENTIALS_KEY through make docker-start
scripts/docker.sh runs Compose from docker/ without --env-file, so the dev
gateway's DEER_FLOW_CREDENTIALS_KEY=${DEER_FLOW_CREDENTIALS_KEY:-} entry
interpolated to empty and overrode the key the checkout .env supplied via
env_file. Export it from the dotenv like AUTH_TRUSTED_PROXIES.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
hataa
|
47d8ff13b2
|
feat(authz): account disable/enable across every authentication surface (#4063 gap 3) (#6566)
* feat(authz): account disable/enable across every authentication surface (#4063 gap 3) Vertical slice 1 of the user-lifecycle work: an operator can suspend an account and every authentication surface rejects it, while the row — credentials, role, OAuth linkage — is retained so re-enabling restores exactly what was suspended. - users.disabled (NOT NULL, server default 0) via migration 0036_user_disabled; User/UserResponse carry and expose it. - Enforcement at all four surfaces: password login (no credential comparison happens), JWT session resolution (401 ACCOUNT_DISABLED — sessions die at the next request), PAT resolution (the generic 401, no oracle), OAuth provisioning (403 with a contact-admin message; the link is kept so re-enable restores access). - SQLiteUserRepository.set_disabled: single-column, serialized write (same idiom as update_system_role) with the last-ACTIVE-admin guard — disabled admins cannot authenticate, so they cannot restore management access; re-enabling is always allowed. Field-scoping completed both ways: the credential writer mirrors and never writes disabled, so a stale password-change snapshot cannot re-enable a suspended account. - PATCH /api/v1/admin/users/{id} becomes a partial account update (system_role and/or disabled), each field routed to its field-scoped writer; empty bodies 422. Tests 11->18: route (disable/re-enable, last-active-admin 409, empty 422, combined update), real-repo (field isolation both directions, disabled-admin-not-active, guard inside the serialized write, re-enable always), login rejection with the CORRECT password (only the disabled gate can reject) plus resolver-visible flag. Mutations verified red: guard removed (M19), login gate removed (M20 — after hardening the test with a real hash so the gate is load-bearing). * fix(authz): portable boolean default, alternate-authenticator suspension, active-admin role guard Review round 1 on #6566 — three P1s: - [PG default] sa.text("0") is a raw integer expression PostgreSQL rejects as a BOOLEAN default, breaking startup migration and fresh create_all bootstraps alike. Both the migration and the ORM model now use sa.false()/false() — dialect-rendered, portable. - [alternate authenticators] WebSockets bypass AuthMiddleware, so a disabled user's still-valid cookie could open a NEW browser/stream connection and drive the retained session; the standalone LangGraph authenticate callback accepted the same suspended JWT. Both now route through a shared post-lookup validator (validate_resolved_session_user in deps.py — token_version + disabled in one place) that the HTTP dependency also uses, so a lifecycle change lands everywhere at once. - [active-admin role guard] update_system_role counted every admin row, so 'disable B, then demote self' — or a combined {system_role, disabled} self-update — could strand the deployment with zero administrators able to authenticate or re-enable anyone. The demotion guard now counts ACTIVE admins only (disabled admins cannot authenticate), under the same serialization boundary. Tests 18->22: shared-validator verdicts, WS admission after suspension (real token, real repo), LangGraph-callback admission after suspension, disable-then-demote-self + combined self-update both rejected (fake-repo route level and real-repo update_system_role level). Mutations verified red: helper's disabled check removed (M21 — three admission tests fail), active-admin predicate reverted to all-admins (M22 — repo-level strand scenario fails). 366 tests green on the affected surface. * fix(migrations): chain 0036_user_disabled after upstream 0037 The maintainer main-merge brought 0037_project_document_summaries, which branched from 0035 independently of this PR's 0036 — alembic now sees two heads and every engine bootstrap fails. Chain 0036 after 0037 (single head; revision numbers are labels, the graph is what matters). * fix(migrations): renumber user_disabled to 0038 and follow the chain-test convention Upstream landed its own 0036 (run_idempotency_request) with 0037 chained after it while this PR was in review — my 0036_user_disabled collided on the number and my interim chain-fix (0036 revises 0037) left the graph in a state upstream's chain tests reject (heads == [0037] hardcoded). - 0036_user_disabled -> 0038_user_disabled, revises 0037_project_document_summaries: single head 0038. - Chain tests follow the repo convention (each migration PR appends the new head): 0032/0035 gain the DOC_SUMMARIES link and CURRENT_HEAD = 0038; 0037's single-head assertion moves to CURRENT_HEAD with its own-revision link check unchanged. * fix(migrations): pin the remaining chain tests to the 0038_user_disabled head The 0027/0028, 0029 and 0034 chain tests still pinned the single head to 0037_project_document_summaries (and 0034 asserted the head's parent was 0036_run_idempotency_request), so the renumbered lifecycle migration broke two unit-test shards. Follow the chain-test convention: every pinned head moves to 0038_user_disabled and 0034 walks the chain through 0037. * fix(authz): enforce account suspension on the internal-auth dispatch surface Round-3 review finding: the internal-auth principal is synthesized from trusted headers without a users-row lookup, so a suspended user with an active channel binding kept driving the agent, and scheduler-owned runs (their fabricated in-process requests skip AuthMiddleware entirely) kept launching. Two gates now enforce the lifecycle there: - AuthMiddleware rejects owner-bound internal calls whose users row is disabled (401 account_disabled) at the single gate every internal HTTP call passes through. Auth-disabled mode is exempt (no suspendable accounts, and its owner header carries the synthetic identity), and owner ids without a users row stay allowed — a nonexistent account cannot be suspended. - start_run re-asserts it at run admission after the owner is resolved for attribution: the scheduler records the rejection as the occurrence's launch error and no run record is created. Also addresses the remaining round-3 nits: drop the inline token_version check duplicated by validate_resolved_session_user, correct the local_provider comment (login never surfaces ACCOUNT_DISABLED), refresh the stale 0038 docstring header, and restore the deps module caches in the two suspension tests instead of leaking a closed engine. * fix(auth): carry the disabled field on the frontend synthetic users UserResponse now always includes disabled, so the auth-disabled contract e2e (gateway /auth/me deep-equals the frontend synthetic user) failed without the field on AUTH_DISABLED_USER. Add disabled to both synthetic users and declare it optional on the zod schema — absent still parses as not disabled for pre-gap-3 backends. * fix(authz): log when the internal-owner suspension check is skipped The middleware gate's users-lookup failure path silently degraded to allow; keep the fail-open behavior but leave a WARNING with the exception, so a store that cannot be read is observable in production logs instead of invisible. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Nan Gao
|
b99ac916b6
|
fix(auth): count login lockouts in the database across Gateway replicas (#6501)
* fix(auth): count login lockouts in the database across Gateway replicas POST /api/v1/auth/login/local throttled failed logins per client IP in a module-level dict, so with N Gateway replicas behind one load balancer an attacker got N x auth.local.max_login_attempts guesses and a lockout on one replica was invisible to the others. Move the counter behind a LoginThrottleStore (deerflow.persistence. login_throttle): MemoryLoginThrottleStore keeps the historical per-process semantics and 10k tracked-IP bound; SqlLoginThrottleStore shares one login_throttle row per IP through the application database, incrementing and starting the lock in a single dialect-native upsert, applying check decisions with compare-and-set predicates in a separate write transaction, and sweeping served locks plus 24h-idle counters in bounded batches. Both stores pin the existing contract: the lock starts when the count reaches max_login_attempts, the committed duration is honored when the policy shrinks mid-lock, a served sentence is never resurrected, tightening keeps the count, success resets the IP, and the 429 status and message are unchanged. New startup-only selector auth.local.throttle_storage (auto | memory | db, default auto = the database whenever database.backend is sqlite or postgres). The Gateway installs the store once at startup after the persistence engine; deps._validate_login_throttle_storage warns when a multi-process deployment keeps memory. Migration 0034_login_throttle creates the table; config_version 55 -> 56 in config.example.yaml and the Helm chart. Docs: README, CONFIGURATION.md, AUTH_DESIGN.md, config and migrations AGENTS.md, CHANGELOG. * docs(changelog): link the login throttle entry to #6501 * fix(auth): re-check expiry on the row the login-throttle sweep deletes The bounded cleanup in SqlLoginThrottleStore.record_failure deleted `WHERE ip IN (SELECT ip ... WHERE <served or stale> LIMIT 200)`. On PostgreSQL READ COMMITTED the expiry predicate applied only inside the snapshot-bound subquery: when the DELETE had to wait on a row lock held by a concurrent check() that was extending the same lock under a raised lockout_seconds, PostgreSQL re-evaluated only the outer WHERE on the updated tuple, IP membership still held for the old candidate, and the now-active lock was deleted; the next attempt from that IP got a clean record and reached password verification (review of #6501, P2). Repeat the served/stale predicate on the DELETE target as well (SqlLoginThrottleStore.sweep_statement), keeping the bounded candidate subquery, so the re-evaluation sees the updated row's expiry. The other writes already carry their full compare-and-set predicate on the target row: _discard and the check() UPDATE both use _matches (ip, fail_count, locked_at, lock_duration_seconds), so a tuple changed while they waited no longer matches and the stale decision is skipped. _discard keeps the equality compare-and-set rather than adding an expiry condition: its early-release branch deletes a lock whose committed sentence has not elapsed under the lowered live policy, which an expiry predicate would wrongly block, and the equality predicate already pins every input of the decision. reset stays unconditional by design. Tests: a dialect-compile test asserts the DELETE's own WHERE carries the expiry predicate for postgresql and sqlite (red on the previous HEAD), and a PostgreSQL-only test (skipped without TEST_POSTGRES_URI) reproduces the update-versus-sweep interleaving with two engines, waiting on pg_stat_activity for the sweep to block before the extender commits. Docs: sql.py concurrency contract and the migrations guide's 0034 bullet. * fix(auth): keep an active login lock's clock and fail closed on explicit db Address the "changes requested" review on #6501. Lock clock (confirmed defect): record_failure re-stamped locked_at = now for every failure on an already-locked row, so a sustained attacker's sentence slid forward with each attempt. Both stores now apply one rule in one atomic decision: new_count = fail_count + 1; below max_attempts the IP counts with no lock (a failure after the operator raised the threshold clears an existing lock); at or over it, a failure during an *active* lock keeps locked_at and the committed lock_duration unchanged, while a never-locked or served row starts anew with now / lockout_seconds (so the failure that reaches a raised threshold starts a fresh lock rather than keeping a stale locked_at). The SQL upsert expresses it as nested case() on the row's own values; the memory store mirrors it. The sweeps in both stores now leave the row being recorded to that decision instead of deleting a served lock or stale counter first, which split the contract (SQL would have produced a fresh (1, NULL, NULL) row). base.py states the rule. Explicit db downgrade: resolve_login_throttle_store raises SystemExit — the failure type of the other startup validations in app.gateway.deps — when throttle_storage='db' resolves to the shared table but no engine is initialised, so langgraph_runtime refuses to start instead of silently running every replica on per-process counters; the lazy bare-app path applies the same rule and installs nothing. 'auto' keeps warning and falling back. 'db' on a memory database still degrades with a warning (there is no shared table to insist on). Non-blocking items: _key() logs a warning when a client key is truncated to 255 characters (never the key itself); the sql.py docstring covers wall-clock locked_at across replicas (run NTP; skew shifts evaluation by that much) and why STALE_COUNTER_SECONDS=24h / SWEEP_BATCH_SIZE=200 were chosen; a store-level blocking-I/O anchor covers the SQL reset path; the _validate_login_throttle_storage warning was verified to render 'throttle_storage=memory' for the StrEnum (the gate test now passes the enum member and asserts that text). reset stays unconditional: a peer failure that commits after a reset leaves fail_count=1, one genuine failure after the cleared history, which a new test documents. Docs: CHANGELOG entry, config.example.yaml, CONFIGURATION.md, AUTH_DESIGN.md and the throttle_storage field description. * fix(auth): re-evaluate a login-throttle check whose compare-and-set missed SqlLoginThrottleStore.check applied its decision (commit the live duration, or clear a served / released lock) with a compare-and-set predicate on the snapshot it had read, but ignored the affected-row count. When a peer changed the row between the read and the write — an admitted wrong password incrementing fail_count on another replica, or a successful login clearing the row — the UPDATE or DELETE matched zero rows, yet check reported the decision as applied: it could return an extended 60s sentence while the row kept its 1s duration, so the next check cleared the lock and let password verification through; a live duration decrease could be dropped the same way; and a row a peer had just cleared was still reported as locked (#6501 review, P2). Every compare-and-set write in check now checks result.rowcount. On a miss the row is re-read and re-evaluated from the top, up to CHECK_CAS_ATTEMPTS (3) times, applying a fresh compare-and-set decision on the new snapshot and never clobbering a reset or replacement: a row that is gone is reported as allowed, a fresh peer lock is reported as locked, and an incremented count gets the extension or decrease committed on it. Exhausting the bound fails closed: nothing is written and the answer is the last snapshot's lock under the live policy, logged at debug. The read stays in its own short transaction and each write in a separate one. The evaluation is now one shared pure function, evaluate_check in base.py (CheckDecision), used by both stores so the contract cannot diverge. The memory store keeps a single pass: its compare-and-set cannot miss because there is no await between the read and the write on the loop thread, which its docstring now states. Tests (red first on df26c2acf) use a session proxy that runs the peer mutation on a second engine right after the snapshot read ends its transaction: the reviewer's raised-duration interleaving (59.5s reported and committed, the increment survives, a later check still locked), the mirrored decrease, early release on the fresh row, a reset landing between read and write (0.0, no row recreated), and the bound-exhausted fail-closed case. * docs(changelog): resolve merge markers left in the Unreleased section * test(migrations): pin 0035_login_throttle as the head in the 0034 watermark test Upstream #6477 added test_migration_0034_run_event_seq_watermark.py asserting 0034 is the single head; this branch chains 0035_login_throttle after it, so the test now pins the current head and walks the chain like its siblings. * perf(auth): one session per login-throttle check, one transaction per failure Two non-blocking performance suggestions from the #6501 review. Single session per check. The router probed the row with store.get() and then called store.check(), which re-read it: a recorded IP cost two connection-pool checkouts and two SELECTs per request on the unauthenticated login path, letting a lockout flood compete with legitimate traffic for Postgres connections. LoginThrottleStore.check now takes the policy as an async callable (LoginThrottlePolicy) and resolves it lazily inside the store: the probe, the policy resolution and the compare-and-set loop share one session, a clean IP returns 0.0 without ever calling the policy (the config.yaml read is still skipped for the common case), and a recorded IP resolves it exactly once, also across compare-and-set retries. The decision is made on a snapshot read after the resolution: the policy read is a yield point, and a decision that needs no write has no compare-and-set to catch a row a peer changed meanwhile — so one session, but still a read after the yield. The memory store takes the same signature and re-reads its record after the await for the same reason. The router passes its asyncio.to_thread policy read as the callable and drops the separate probe; get stays on the protocol for tests and anchors. No short-TTL verdict cache: it would mask a successful login's reset for the TTL window, and the single session already halves the hot path. Sweep in the upsert's transaction, plus an index. record_failure ran the housekeeping DELETE in its own transaction and the upsert in a second one, two commits (two WAL flushes on PostgreSQL) per failed login; the sweep now rides in the upsert's transaction with one commit, the keep exclusion already protecting the row being recorded. The new ix_login_throttle_updated_at (ORM __table_args__ and the unreleased 0035 migration, created after the table and dropped before it, each guarded separately so a table left by an interrupted attempt still gets its index) serves the sweep's stale-counter predicate; the served-lock predicate scans a table the sweep itself keeps small. Tests (red first on a137fbc47): a clean IP never resolves the policy and a recorded IP resolves it exactly once (both stores), the SQL store opens exactly one session per check and commits once per record_failure (counting wrappers), the migration test asserts the index after upgrade, after completing a partial upgrade that lacked it, and gone after downgrade while compare_metadata stays empty; the router-level "mutated during policy resolution" tests now inject their mutation through the policy callable. * docs(changelog): note config_version 57 in the login throttle entry |
||
|
hataa
|
0032579017
|
feat(authz): admin role-assignment API for configured custom roles (#4063 gap 2) (#6382)
* feat(authz): admin role-assignment API for configured custom roles (#4063 gap 2) The RBAC provider accepts arbitrary configured role names (guest, ...), but nothing could assign them to real users: User.system_role was pinned to Literal["admin", "user"] at the API layer while the DB column stayed a plain String(16) for exactly this future, and zhfeng's #3462 audit listed the missing assignment surface as gap 2 — custom roles were declarable in YAML but unreachable for authenticated web users. - User/UserResponse.system_role widens to a non-empty string; whether a role is *assignable* is the admin API's contract, not the model's. - RbacAuthorizationProvider exposes known_roles; the Gateway's new assignable_role_names() unions the built-ins with the provider's configured role names (custom providers: built-ins only). - New admin-only router /api/v1/admin/users: GET lists users with their role; PATCH assigns — role validated against assignable names (an unknown role would silently fall through to default_role on every decision), 404 for missing users, and the last remaining admin cannot be demoted (bootstrap lockout protection). Enforcement is unchanged: the assignment only changes which role the principal carries; the @require_permission route guards and runtime provider decisions remain the single enforcement points. - SQLite repository gains list_users (ordered oldest-first). Tests: router guard/422/404/409/no-op/happy-path, last-admin invariant (both directions), assignable names union, principal carries the configured role; the two old Literal-rejection contract tests updated to the new contract (custom role representable; empty string still rejected). * fix(auth): widen the frontend user schema for custom roles (review-lessons catch) The role-assignment API's review-lessons sweep found the one consumer class the implementation had not enumerated: the FRONTEND parses /me with a strict userSchema whose system_role was z.enum(["admin", "user"]) — a user assigned a provider-configured custom role (guest) would fail userSchema.parse in server.ts and break the auth flow before rendering anything. - types.ts: system_role widens to z.string().min(1), kept in lockstep with the backend model (min_length=1). All existing consumers compare === "admin", so any other value is simply non-admin — no behavior change for admin/user. - server.test.ts: contract tests — custom role parses verbatim; empty string still rejected. - config.example.yaml: the authorization comment now says custom roles become reachable via PATCH /api/v1/admin/users/{id} and that a declared-but-unassigned role grants nothing. * fix(auth): commit the admin_users router, drop the stale type-ignore, document the surface Review catch on #6382: the router module was never staged — app.py and the tests import it, so the pushed branch could not boot (ImportError in create_app). The local tree kept it as an untracked file, which is why every check passed before the push. - Actually commit app/gateway/routers/admin_users.py. - _row_to_user: drop the now-stale '# type: ignore[arg-type]' on system_role — the model field is a plain string now (keep the comma). - README: the authorization paragraph now mentions the admin role-assignment surface (GET/PATCH /api/v1/admin/users, admin-only, last-admin demotion protected, declared-but-unassigned grants nothing). - server.test.ts import-order lint fix from the frontend sweep. * fix(auth): field-scoped role writes close the demotion-undo race Review round 2 on #6382 — three findings, one design: - [P1] update_user copied system_role from the caller's account snapshot, so a password change that read the user as admin could restore a role concurrently revoked by the demotion route (stale-snapshot write-back). Account writes are now field-scoped against each other: update_user is the credential/profile writer (never touches the role, mirrors the row's current role back), update_system_role is the only role writer (single-column, never touches credentials or token_version). - The last-admin lockout check moved inside the role write's serialized transaction: count-then-write across two sessions let two concurrent demotions of the only two admins both pass. Same idiom as create_first_admin — BEGIN IMMEDIATE on SQLite, pg_advisory_xact_lock on a distinct key on Postgres, raise on unknown dialects. - UserNotFoundError from the racing-delete window maps to the route's 404 instead of a 500. Tests 11->12: real-repo regression covering the P1 interleaving (demotion lands -> stale credential snapshot writes -> role stays revoked, credentials applied), field isolation both directions, and the last-admin invariant raised from inside the write with the row intact. Mutation-verified: restoring the role write-back in update_user fails it. * style(auth): format the server auth tests * fix(tests): migrate the personal-MCP demote revocation to update_system_role The role-write field-scoping (66e2cec23) made update_user a credential/profile-only writer, so the demote branch of test_revocation_blocks_discovery... silently no-oped and CI failed on all four shards (assert 2 == 1 — the role never changed). - The demote now goes through update_system_role, the sole role writer. - The fixture gains a second admin: the revocation test demotes its sole synthetic admin, which the new last-admin lockout guard correctly blocks — a second admin keeps the scenario legal without weakening either invariant. --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
creed
|
9eb01b371a
|
feat(config): select an explicit backend dotenv file (#6227)
Signed-off-by: 97three <2212371308@qq.com> |
||
|
yetuge
|
300abb36d2
|
fix(auth): reject a non-ASCII ID token nonce instead of raising (#6115)
* fix(auth): reject a non-ASCII ID token nonce instead of raising The nonce comparison in validate_id_token still used the local secrets.compare_digest on str operands, which raises TypeError when either side holds non-ASCII characters. The nonce claim is provider-controlled text, so such a provider response turned the callback's sso_failed redirect (OIDCValidationError) into an unhandled 500. Route it through the shared constant_time_equals helper #6076 introduced for the same five compare sites, and drop the now-unused local helper. Signed-off-by: yetuge <2219677952@qq.com> * fix(auth): reject a non-string ID token nonce claim like a mismatch A provider returning a truthy non-string nonce claim (e.g. an int) slipped past the missing-claim guard and crashed the comparison helper with AttributeError, which the callback turned into a 500 instead of the sso_failed redirect. Guard the claim type before comparing so every malformed nonce takes the same OIDCValidationError path. Signed-off-by: yetuge <2219677952@qq.com> --------- Signed-off-by: yetuge <2219677952@qq.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Hyeonsang Cho
|
e489bf2c92
|
fix(auth): claim the first admin atomically instead of counting first (#5776)
* fix(auth): claim the first admin atomically instead of counting first POST /api/v1/auth/initialize read the admin count in one session and created the admin in another, with no lock between them. Two requests arriving together with different emails both saw an empty system and both became admins: reproduced over HTTP, two 201s and two admin rows, after which setup-status reports an initialized system. The endpoint is public and unthrottled, and during the pre-setup window anyone who can reach the gateway can claim admin with a single request anyway — that part is the first-boot design. What the race removes is the only signal: without it exactly one request wins and the loser gets 409, so an operator who loses sees "already initialized" and looks into it. With it, both succeed and the operator's setup looks ordinary. The check and the insert now share one transaction in the repository, with writers serialized first — SQLite takes the write lock up front (BEGIN IMMEDIATE) and Postgres a transaction-scoped advisory lock, the idiom the project and channel repositories already use. The same repository serves both backends, and multi-replica deployments make the race genuinely parallel rather than an event-loop interleaving. create_first_admin returns None when the claim is lost, which the route reports as 409; uniqueness conflicts still raise ValueError and stay a 400. The count in the handler is now only a fast path that avoids hashing a password for an already-initialized system. AUTH_DESIGN.md said concurrent initialization was backstopped by a database unique constraint. No such constraint exists — email uniqueness only catches the same address submitted twice — so it now describes the claim that does the work. * docs(changelog): note the atomic first-admin claim (#5776) * fix(auth): refuse a dialect the first-admin claim cannot serialize Review follow-up. The claim's serialization handled sqlite and postgresql and fell through silently for anything else, which would leave create_first_admin as the plain check-then-act it exists to prevent — the one place the abstract method's atomicity contract is enforced. An unknown dialect now raises instead. The engine builds only these two today, so this guards a future backend: it fails loudly at first boot rather than quietly reopening the race. |
||
|
Keswyk Cheung
|
731e8c6eee
|
feat(memory): bulk-load sample for registered users (#4066)
* feat(memory): bulk-load sample for registered users * fix(memory): harden all-users sample loader --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
陈志谦
|
1b2898c367
|
fix(authz): cover create/run/memory/agent routes with permission checks; scope USER.md per user (#4989)
* fix(authz): cover create/run/memory/agent routes with permission checks; scope USER.md per user
Route permissions: the AuthorizationProvider model only applied to routes
carrying @require_permission, so POST /api/threads, /api/threads/search,
POST /api/runs/stream and /api/runs/wait (runs:create), every /api/memory
route and the custom-agent routes ran with authentication only — a
provider configured to deny threads:write/runs:create/... could not
enforce those decisions. Add the missing decorators (threads:write,
threads:read, runs:create, memory:read/write, agents:read/write) and
register the new permission names in authz.Permissions. No owner checks
are added: per-user data scoping stays in the repository layer.
User profile: GET/PUT /api/user-profile read and wrote a single global
{base_dir}/USER.md, so in a multi-user deployment any authenticated user
could overwrite the prompt context injected for everyone else (and read
it). Scope the file to the caller's bucket
({base_dir}/users/{user_id}/USER.md) like user-scoped skills; no other
consumer of the old global path exists in the tree (verified by grep and
the updated tests). test_put_user_profile asserted the wrong effective
user under the autouse conftest user fixture; fixed to test-user-autouse.
* fix(authz): bind positional args in require_permission; migrate legacy USER.md
Review follow-up on #4989 (willem-bd):
- require_permission's wrapper only looked for request in keyword
arguments, so direct positional calls like
create_thread(body, request) made the injected keyword stub collide
with the positional request (TypeError: multiple values). The wrapper
now binds the wrapped signature via inspect.signature().bind() and
honors a positionally-passed request (and thread_id) instead of
assuming kwargs; the test-stub injection only fires when request is
absent everywhere. The two positional callers in
tests/test_threads_router.py pass again (8/8 channel tests).
- migrate_user_isolation.py now claims the legacy global USER.md for
--user-id (default 'default') like the other unowned legacy
artifacts; without it, upgrading installs with an existing profile
would read content: null and later strand the old file beside the
new per-user one. Conflict handling mirrors migrate_memory (rename to
USER.legacy.md).
* style(scripts): keep migrate_user_isolation help within the line budget
* test(authz): give internal-request stubs realistic auth fields
The create_thread permission wrapper added in this PR authenticates
the direct calls in the internal-owner tests; their SimpleNamespace
requests had state.user but no auth_source and no cookies, so
get_current_user_from_request fell through to request.cookies.get and
raised AttributeError (verified introduced by this branch: both tests
pass on the base commit).
The stubs now carry cookies={} and
state.auth_source=AUTH_SOURCE_INTERNAL, which is exactly what
AuthMiddleware stamps on real internal requests, so state.user is
honored without the JWT path. All 80 tests in the file pass.
* fix(pat): keep memory/agent permissions out of PAT scopes by design
The route permissions exist (they guard the memory/agent routers) but PATs
govern the thread/run lifecycle only: _PAT_ROUTE_RULES default-denies those
routers for PAT callers regardless of scopes. The alignment invariant
becomes a subset check plus a pinned exclusion, and pat.py documents that
opening these scopes is a product decision requiring three synchronized
changes.
* test/docs: cover migrate_user_profile; drop the unclaimed USER.md injection wording
Five-scenario test class mirroring the sibling migration steps (move,
conflict-rename, noop, both under dry-run). The GET/PUT descriptions no
longer say USER.md is 'injected into agents' — nothing at this head consumes
it for prompts; the routes are storage/retrieval only. The memory AGENTS.md
migration pointer now enumerates skills/ and the global USER.md.
* docs(paths): align the USER.md layout comment with the injection disclaimer
* test(authz): align effective permission contracts
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
Zeren Wang
|
a58ab484a6
|
feat(projects): Projects MVP Phase 2 — instructions, document shelf, promotion, trash (#5443)
* feat(projects): Projects MVP Phase 2 — instructions, document shelf, promotion, trash Implements docs/superpowers/specs/2026-09-12-projects-mvp-phase2-design.md (issue #5160, tracker #5129) in the slice order of the spec's §16. Slices: - A: ProjectsConfig + write-time 422 UTF-8 byte cap; PROJECT_CONTEXT_KEY admission pinning (both server-owned sets + worker hoist); latest-only request-scoped <project> block via DynamicContextMiddleware wrap_model_call/awrap_model_call (idempotent reassembly, reserved ID prefix + marker + provenance, never persisted); journal audit fingerprints; Instructions tab. - B: ProjectDocumentRow + migration 0023; ProjectDocumentRepository with locked check-and-set; hash-qualified immutable shelf storage with Paths helpers; upload/list/content/delete-to-trash routes; project delete trashes the shelf in-transaction; request-scoped bounded <documents> index with honest count/shown + actionable overflow note; list_project_documents/read_project_document tools registered only on pinned runs; PAT allowlist + drift guards; blocking-IO anchors. - C: shared thread-upload ingestion service (uploads router refactored to parity); POST from-thread with provenance; attach-to-thread with lock-staged copy (archived source allowed); read-only thread-files view with per-group truncation reporting. - D: restore (restored/merged/not_found/no_target/content_missing; no file moves), purge (continuous row lock across unlink/delete/commit, retryable on FS errors), retention sweep (lazy + startup, 24h orphan guard, row-side reconciliation never deletes). - E: Documents tab (shelf + conversation-files browser, provenance, archived banner, content-missing rows), /workspace/trash route, sidebar entry, composer attach handoff, i18n (en-US/zh-CN), e2e mocks + specs. Review hardening folded in (10 rounds, all with tests): - force active shelf content (HTML/XML family) to download; nosniff on artifact + content responses; unified unsandboxed-iframe PDF preview (fixes the pre-existing Chromium sandbox blank in the artifact viewer) - scope document trash to the URL project under the document lock - atomic no-overwrite filename reservation for ALL ingestion (seeded claims + os.link commit with suffix retry; same-name re-upload now unique-names instead of replacing); hidden staging only, no visible placeholders; lease cleanup on setup failure - serialize conversion under the document lock with post-lock active revalidation; drain locked filesystem work on cancellation; preserve bytes when an insert's commit state is uncertain (including trashed rows) - original-integrity checks before serving text or cached conversions; content_missing surfaced in list responses (UI reads the flag, no 409-probe); downloads always serve original bytes - bounded streaming document reads with cached char counts; shelf limits declared in middleware release identity - thread-root confinement for from-thread sources; config fallback rejects fractional/infinite values; composer counts staged attachments; pending attachments persist until submission or removal; in-flight instruction/rename edits survive save refetches; shelf and trash pagination; conversation-file and thread-files pages stay subscribed to refetches Docs: README/README_zh, backend API.md/ARCHITECTURE.md, AGENTS.md contracts, config.example.yaml projects block. Review follow-ups (head b4807477 → this revision): - The trash retention sweep is split so repeated lazy triggers stay bounded: the indexed expiry purge still runs on every trigger (GET /api/trash/documents, POST /api/trash/purge) while the O(all rows + all files) reconciliation is throttled to one run per user per 15 minutes (process-local, per-user window). The startup sweep now runs as a background task instead of blocking gateway readiness, and shutdown awaits it (bounded). - The export scrub (stripInternalMarkers) is fence- and indentation-aware like the render path, so a pasted, fenced <project>/<documents> snippet survives markdown export while real injected blocks (never fenced) are still removed. Fence regexes moved to a dependency-free leaf module to avoid the messages↔streamdown import cycle. - The artifact viewer's PDF iframe no longer carries an added title attribute (the upstream e2e contract locates it via :not([title])), and the upstream artifact-preview spec now pins the new contract: PDFs render unsandboxed, images keep sandbox="". * fix(projects): round-2 review — cancel an overrun trash sweep, restore the PDF frame title - Shutdown cancelled only the shield around the background startup sweep, so an all-users reconciliation that outlived the 5s budget kept walking rows and files while the document repo and DB engine were disposed underneath it. The wait now lives in `_shutdown_startup_trash_sweep`, which cancels the task and drains it before worker exit: the shield keeps the wait bounded, the cancel makes it final (CancelledError lands at the sweep's next await, and `_run_startup_trash_sweep` only catches `Exception`, so nothing swallows it). - The browser-preview iframe lost `title={getFileName(filepath)}` in the previous fix round, leaving the PDF frame without an accessible name while its siblings keep theirs. Restore it (WCAG frame titles), assert it in the DOM test, and anchor the e2e on `iframe[title="report.pdf"]` instead of `iframe:not([title])`. * fix(projects): round-3 review — report the sweep's late finish, not a phantom cancel `Task.cancel()` returns False when the sweep already finished inside the window between the deadline firing and the cancel, so the shutdown log claimed a cancellation that never happened. Branch on that outcome: the warning stays for a real cancel, a late finish is logged at info, and both paths still reap the task before worker exit. * fix(projects): round-4 review — make Empty trash delete what it confirms `POST /api/trash/purge` only ran the retention sweep, and the sweep's candidate selection is age-gated, so a freshly trashed document survived "Empty trash" even though the confirmation promises that every listed document is permanently deleted. With one trashed row the route answered `{"purged": 0}` and left it in place; `GET /api/trash/documents` sweeps expired rows before listing, so the visible rows were normally ineligible for the action by construction. Empty trash now drives `purge_all_trashed`: the caller's trashed rows (`list_all_trashed`, no age filter) each go through the same guarded, row-locked `purge` as the single-document delete — bytes first, then the row, in one transaction — so a row restored mid-flight is skipped instead of force-deleted, and an unlink failure rolls that row back and answers 500 with a retryable message. Retention expiry stays where it was: the sweep's `purge_candidates` is now the only age-gated selection, and the lazy retention sweep still runs on the listing and at startup. Tests: the router suite replaces the retention-gated expectation with the reviewer's repro (fresh row purged, bytes unlinked, shelf and other users' trash untouched, a failing unlink stays retryable and 500); a blocking-I/O anchor drives the new entry point through the offload; the mocked e2e covers the action end to end; a new real-backend spec performs it against the real gateway and re-reads `GET /api/trash/documents`. README, API, ARCHITECTURE and the phase-2 design docs (en+zh) state the age-independent contract. |
||
|
Zeren Wang
|
5951c89b5b
|
feat(projects): project workspaces with scoped chats and thread membership (#5265)
* feat(projects): project workspaces with scoped chats and thread membership
Backend:
- projects table model and migration; fail-closed ProjectRepository with
ownership checks, CRUD/archive/restore/delete router, and atomic thread
move between projects
- threads_meta.project_id column exposed as reserved deerflow_project_id
metadata; project-aware thread create/search with pagination bounds and
membership echoed in create responses
- first-run admission assigns the project only at genuine first run, seeded
at write time and dropped when invalid; serialized against project
deletion and thread assignment
- branch creation inherits the source thread's project membership (an
archived/deleted project degrades the branch to unassigned instead of
failing the request)
Frontend:
- projects data layer, thread move API, and sidebar projects section with
flat/grouped modes, archived-project threads, and stable virtual-list
offsets
- project detail page with project-scoped new chat
(/workspace/chats/new?project=) and paginated thread list
- move-to-project thread menu, new-project dialog, archived-project gates
- project-scoped new chats pre-create the thread with membership before the
first submit or /goal set, so runs never proceed outside the project
- goal-set preparation is fenced against conversation switches: a stale
continuation is dropped instead of saving the goal or launching the
abandoned submission on the newly opened conversation
- project thread lists join thread lifecycle invalidations (stop, pin) so
an open project page never keeps stale titles, recency, or pagination
* fix(chats): keep archive undo toast when the sidebar row unmounts
The archive success toast was fired from per-mutate callbacks passed to
mutation.mutate. React Query drops those handlers when the observer
component unmounts before the mutation settles; archiving the open chat
removes its sidebar row mid-flight, so the undo toast never appeared and
the e2e archive-undo test timed out waiting for it.
Move the success/error handlers to the mutation level (useArchiveThread
options, same pattern as useMoveThreadToProject) where callbacks are
delivered even after the originating row unmounts.
* fix(projects): pin project thread listing contract and exclude archived chats
GET /api/projects/{id}/threads returned the thread store row verbatim
(list[dict], no response_model): user_id/assistant_id leaked, any future
ThreadMetaRow column would auto-leak, and the OpenAPI schema was empty.
Return a narrow ProjectThreadResponse (the exact fields ProjectThread
declares) with the same metadata secret redaction the surrounding thread
endpoints get from _MetadataRedactingResponse.
The listing also ran search() without the archived filter, so a retired
chat rendered as a normal row on the project page while the sidebar hid
it. Search archived=False to mirror the sidebar's archived:false lists;
restore stays on the global Archived tab.
Both regressions pinned by new router tests: wire-shape allowlist and
archived-member exclusion.
* docs(migrations): record the 0019/0020 chain against the bootstrap reservation
The tree now chains 0018 -> 0019_projects -> 0020_threads_meta_project_id,
so migrations/AGENTS.md was stale twice over: the revision index stopped at
0018 and the rolling-forward section still claimed the tree 'deliberately
remains at 0018'.
Document the new head and record the intentional numeric-prefix reuse of
0019: 0019_projects is in-chain while 0019_thread_incarnations stays the
reserved, allowlisted out-of-tree rollout id. The owning rollout revision
must re-parent onto this tree's head when it merges so alembic never sees
two heads off 0018; bootstrap.py now cross-references that note next to
_FORWARD_COMPATIBLE_REVISION.
* fix(chats): invalidate project thread lists on archive/restore
useArchiveThread refreshed the infinite sidebar cache, threads/search and
the per-thread metadata cache but not the project-scoped list
([...PROJECTS_QUERY_KEY, 'threads', id]) this PR adds — the one thread
mutation not wired to that key, after usePinThread, useRenameThread,
useDeleteThread, useMoveThreadToProject and invalidateStoppedThreadCaches.
An archive from a sidebar row while a project page is open therefore left
the archived chat rendered as a normal row until remount (and undo left it
missing). Invalidate the prefix in the mutation-level success handler.
Regression test asserts the project-list prefix is invalidated on success.
* fix(projects): fetch project discovery only in grouped sidebar mode
RecentChatList mounted two useProjects queries per sidebar render, but
knownProjectIds is consumed only by the grouped-mode exclusion filter; in
the default flat mode every page load paid two GET /api/projects?status=
round trips for data nothing read. Gate both queries on grouped mode —
GroupedProjectList fetches the same keys when the toggle is on and
TanStack dedupes the observers.
Also set retry: false on useProject: a deleted or foreign project 404s
deterministically, and the page renders a dedicated not-found state for
it, so the default 1s/2s/4s retry backoff kept deep links in 'loading'
for ~7s before that state appeared. Matches useThreadMetadata /
useThreadTokenUsage.
* fix(threads): fail closed on project-scoped create in memory mode
MemoryThreadMetaStore.create accepted project_id and silently ignored it,
making memory mode the one membership path that fails open: POST
/api/threads with a project id returned 200 and the run started
unassigned, violating the invariant that a run never proceeds outside the
selected project (the SQL store raises ProjectNotAssignableError inside
the insert transaction for the same request).
Raise ProjectNotAssignableError whenever project_id is present so the
router's existing 404 mapping applies, the frontend keeps the composer
text for a retry, and memory mode behaves exactly like SQL mode.
set_project already reports rejection; create now matches it.
Store-level test (raises, nothing persisted, project filter stays empty,
unscoped creates still work) plus a router-level test asserting the 404
and that no row is left behind.
* fix(projects): window the project page thread list
ProjectThreadsSection rendered every loaded page as a plain Link row, so a
long-lived project accumulated unbounded DOM on the page's scroll surface:
each load-more appended another 100 rows and every formatTimeAgo tick
re-rendered the whole list.
Reuse VirtualThreadList (now generic over any row shape with a
thread_id), pointing its scroll parent at this page's ScrollArea viewport
via the shared [data-slot="scroll-area-viewport"] selector used by
/workspace/chats; under the 60-row threshold it falls back to the plain
render, so small projects are unchanged.
* fix(projects): restore row dividers and pin them with a render test
The row class template literal concatenated transition-colors directly
with the conditional border-b token, so non-final rows rendered the
invalid class 'transition-colorsborder-b' and lost both the divider and
the transition. Compose the row classes with cn() and a boolean guard
instead.
The section moved out of page.tsx into a testable component so the row
markup finally has coverage: a DOM test asserts every row except the
final data row carries border-b (index-based, not last: — correct under
virtualization where the last mounted row is not the last data row), and
the untitled fallback plus load-more button render for a partial page.
* fix(projects): validate forward schemas and fence membership reads
|
||
|
hataa
|
c55f242451
|
feat(authz): surface effective route permissions on GET /auth/me (Phase 4, #4063) (#5228)
* feat(authz): surface effective route permissions on GET /auth/me (Phase 4, #4063) GET /api/v1/auth/me now returns the effective route permissions alongside the user identity, so the frontend can hide actions the caller's role cannot perform (RFC #4063 Phase 4). The value reuses the AuthContext that AuthMiddleware already resolves per request (including PAT-scope intersection and internal-caller semantics), so /me adds zero extra provider evaluations; a middleware-less composition falls back to the same resolution _authenticate uses. Credential-creation responses (register/initialize) leave the field None: they are public paths where the middleware does not run, and resolving there would introduce fresh on-loop config loads on those routes. * test(e2e): expect /auth/me permissions in auth-disabled contract PR #5228 adds the effective route permissions to GET /auth/me, so the strict toEqual against the bare AUTH_DISABLED_USER object no longer holds: the received payload carries six extra keys (the permissions array). Extend the expected payload with the full registered permission set in _ALL_PERMISSIONS order — with authorization disabled the gateway grants exactly that static list, so the pin stays deterministic. The runtime frontend is unaffected (auth-disabled SSR never calls /me, and userSchema strips unknown keys); only this contract pin needed the new field. * refactor(authz): public resolve_route_permissions_for_request wrapper Address review nits on the middleware-less fallback: the router reached into the private authz._is_internal_caller, so expose a thin public wrapper pairing resolve_route_permissions with the internal-caller heuristics, and use it from both _authenticate and the /me fallback so the two cannot drift apart. Also drop an unused tmp_path parameter from test_auth_disabled_me_includes_default_admin_permissions (_setup_auth provisions its own tmp directory). No behavior change: the wrapper delegates to the exact pair of calls the fallback made before. |
||
|
Otavio Rodrigues Santana
|
27cb73659d
|
fix(auth): correct OAuth conflict error message + validate multi-worker Postgres claim with real concurrency benchmark (#5026)
* fix(auth): correct OAuth uniqueness error and index parity on Postgres
create_user() caught any IntegrityError on commit and always reported it
as a duplicate email. The email pre-check already rules out a real email
collision in the common case, so any IntegrityError reaching that handler
is actually idx_users_oauth_identity firing instead -- confirmed against
both backends: SQLite reports "UNIQUE constraint failed:
users.oauth_provider, users.oauth_id", Postgres reports a
UniqueViolationError naming the same index. The caller saw "Email already
registered" for an OAuth account conflict, which is wrong and would send
API consumers debugging the wrong field.
Distinguish the two cases via a substring check on the driver error text
(both backends name the oauth columns) and raise an accurate message for
each.
Also add postgresql_where to the same index, alongside the existing
sqlite_where. This is not a correctness fix -- verified empirically that
Postgres already enforces the same practical uniqueness without it
(NULL is never equal to NULL in either backends unique index, so real
duplicate (provider, id) pairs are already rejected and NULL/NULL rows
are already unconstrained). postgresql_where makes the index genuinely
partial on Postgres too, matching the stated intent in the surrounding
comment and keeping the index smaller as the common case (plain-password
accounts, both columns NULL) accumulates.
* test(bench): add multi-process SQLite vs Postgres concurrency benchmark
CONFIGURATION.md documents that multi-worker deployments must use Postgres
because "SQLite silently ignores row-level locks", but nothing in the repo
exercised that claim against real separate worker processes -- the existing
checkpoint benchmarks (scripts/benchmark/checkpoint/) measure single-process
read/write latency, and the existing Postgres tests
(test_pg_schema_integration.py, test_multi_worker_postgres_gate.py) cover
schema placement and config validation, not throughput or lock behavior
under concurrent load.
run_concurrency_bench.py spawns N real OS processes (subprocess.Popen, not
asyncio tasks or threads within one process) against the shared users
table, mixing reads (get_user_by_email) and writes (create_user) at a
configurable ratio, and reports throughput, error counts by exception
type, and p50/p95/p99/max latency per run.
Measured locally (2/4/8/16 workers, 100 ops/worker, 70/30 read/write):
SQLite completed all operations with zero errors at every worker count
(busy_timeout absorbs contention rather than raising), but total
throughput stayed flat around 28-34 ops/s regardless of worker count, and
p99 latency grew from ~400ms at 2 workers to ~5.9s at 16, with a 22s max.
Postgres throughput scaled with worker count (41 to 66 ops/s) and p99
stayed under 500ms at every worker count tested. Raw JSON output from
both runs is available on request; exact numbers will vary by machine and
are not asserted in the test suite.
test_bench_concurrency.py unit-tests the pure aggregation logic
(percentile math, error grouping, crashed-worker handling) the same way
test_bench_checkpoint_channels.py does for the existing benchmarks --
fast, no DB required, not the full multi-process sweep in CI.
* fix(auth): inspect the driver exception for OAuth conflict detection
str(exc) embeds the full failed INSERT statement, whose column list
always names oauth_provider/oauth_id, so a substring check on it
misclassified every commit-time IntegrityError on the users table as
an OAuth conflict (reproduced on SQLite: a duplicate primary key with
a different email raised "OAuth account already linked: None/None").
_is_oauth_identity_violation now inspects exc.orig instead: constraint_name
on Postgres, both violated column names present (not a bare "oauth"
substring) on SQLite.
Also ships the alembic revision idx_users_oauth_identity's postgresql_where
predicate needed: 0001_baseline created it as a full index on Postgres,
and ORM metadata changes only affect fresh create_all databases, never an
already-versioned deployment.
Addresses review feedback from willem-bd.
* fix(bench): run the concurrency benchmark in an isolated schema and derive paths from the checkout
--pg-url accepted an arbitrary database URL while the code pinned
postgres_schema="public" and unconditionally ran DELETE FROM users --
against any non-disposable database that permanently destroyed every
auth account. Each run now generates a unique throwaway schema
(bench_<uuid>), points both the seeder and every worker subprocess at
it via postgres_schema, and drops only that schema (DROP SCHEMA ...
CASCADE) once the full worker-count sweep finishes.
Also stopped hard-coding /opt/deer-flow/backend as the checkout path
and .venv/bin/python3 as the interpreter: BACKEND_DIR is now derived
from Path(__file__), and workers are spawned with sys.executable (the
orchestrator's own interpreter) instead, so the documented
uv run python scripts/benchmark/concurrency/run_concurrency_bench.py
command works from any checkout.
Addresses review feedback from willem-bd.
* fix: shorten oauth-index revision id, repin migration-head assertions, fix bench read/write mix
- 0017_users_oauth_identity_partial_pg (36 chars) exceeded
alembic_version.version_num's VARCHAR(32) limit, which would fail
stamping/upgrading on both fresh and existing Postgres deployments.
Renamed to 0017_oauth_identity_pg_partial (30 chars).
- Repinned every test asserting 0016_subagent_batches as the migration
head (test_persistence_bootstrap[.py|_concurrency.py|_regression.py],
test_migration_0004/0007/0015) to the new 0017 revision id.
- worker.py's `(i % 100) < int(read_ratio * 100)` assumed n_ops >= 100;
at the documented default (50 ops/worker, 0.7 read ratio) it produced
either all-reads or all-writes, never the claimed mixed workload.
Replaced with read_count()/is_read_op(), which distribute an exact
round(n_ops * read_ratio) reads evenly across the sequence via modular
spacing, and added test_bench_worker.py covering the default values
plus small op counts.
* fix(bench): establish a real physical connection before timing ops
async with sf(): pass entered an empty AsyncSession without checking out
a physical connection -- SQLAlchemy stays lazy until the first statement
executes. That pushed connection-establishment cost onto each worker's
first timed operation instead of conn_time_s, and at 16 workers those 16
cold first-ops (1% of a 1600-op sample) could skew the reported p99.
Execute a real `SELECT 1` before starting the timer instead.
Verified with a real end-to-end run (uv sync + sqlite backend, 2
workers/10 ops, 0 errors) plus the full auth/bench/migration-bootstrap
suites (135 tests) and ruff check/format, all clean.
* fix(bench): synchronize workers before timing, fix percentile off-by-one
Two remaining measurement issues from review:
- run_workers() started the wall clock before spawning any worker, so
throughput/wall_time absorbed N processes' staggered Python-startup and
connection-establishment cost, and early workers could run ahead of ones
still starting. Workers now print READY right before their timed loop
and block on stdin for a GO signal; the orchestrator waits for every
READY, then starts the timer and releases all workers together.
- summarize()'s pct() used int(len(latencies) * p) directly as a
zero-based index -- a one-based-rank-as-index bug that put p95 and p99
at the same slot (the max) for any 20-or-fewer-sample run, and for the
documented 100-sample default. Now delegates to
checkpoint_bench_common.percentile(), the already-correct nearest-rank
implementation used elsewhere in the same benchmark family, instead of
a second, broken one.
Verified: 14/14 unit tests pass (2 new pinned-value regression tests for
the percentile bug, using the reviewer's own 20-sample repro), ruff
clean, and a real 2/4-worker SQLite multi-process smoke run completes
with distinct p95/p99/max latencies and no hang.
* fix(bench): absolute SQLite bench path, surface crash diagnostics, exit nonzero on failure; share OAuth index constant + cover Postgres branch
Three more findings from review at 5fd25a7:
- seed_baseline() cleaned an absolute .deer-flow/bench_data path but
handed DatabaseConfig a relative one, which resolves against the
CALLER's CWD -- not BACKEND_DIR. Invoking the documented command from
anywhere other than backend/ silently pointed the seeder and the
(cwd=BACKEND_DIR) workers at two different directories: workers crashed
with 'unable to open database file' while the run still printed a
well-formed summary and exited 0. Both seed_baseline() and worker.py's
make_session_factory() now use the same absolute path.
- Crashed workers' stderr was captured then discarded, and main() always
exited 0 -- an all-crashed sweep was indistinguishable from a real
(uneventful) measurement to anything checking the exit code or
--out. run_workers() now prints each crash immediately and tags it with
the real worker_id (previously always None); summarize() exposes
crashed_worker_errors alongside the existing crashed_workers count;
main() exits 1 via the new summary_indicates_failure() whenever any
sweep crashed or fell short of expected_total_ops.
- idx_users_oauth_identity was hardcoded separately in the ORM Index and
in _is_oauth_identity_violation's Postgres branch, with no test to
catch drift, and that branch had zero non-skipped coverage (its only
guard needs a live Postgres CI never configures). Exported
OAUTH_IDENTITY_INDEX_NAME from user/model.py as the shared source of
truth (migrations intentionally keep their own frozen literal, matching
every other revision in that package) and added stub-exception unit
tests pinning both the asyncpg constraint_name path and the sqlite
message-substring path, positive and negative.
Verified: 107 passed locally (auth + bench-unit suites), ruff clean, and
two real reproductions -- invoking run_concurrency_bench.py from a
scratch directory outside backend/ (the reviewer's exact repro) now
completes 8/8 ops with crashed_workers: 0 instead of crashing, and the
new crashed_worker_errors/exit-code logic is exercised directly by the
new unit tests against the real summarize()/summary_indicates_failure().
* fix(auth): attribute create_user IntegrityErrors to the right constraint
Two coupled review findings on the classification helpers:
P3 (fall-through) -- after ruling out the OAuth-identity index, create_user
raised "Email already registered: {email}" for every remaining
IntegrityError, including the duplicate-primary-key case the new
regression test exercises, whose address is not registered. Added
_is_email_violation() so the email message is used only for an actual
users.email collision that raced past the pre-check; anything else (in
practice a duplicate id) now raises a neutral
"User already exists (constraint: <name>)".
P2 (unreachable asyncpg branch) -- exc.orig is not the asyncpg error.
SQLAlchemy's asyncpg dialect re-raises its own DBAPI IntegrityError
(pgcode/sqlstate only) 'from' the real asyncpg error, so constraint_name
lives on exc.orig.__cause__. getattr(exc.orig, "constraint_name", None)
was always None on Postgres; the helpers only worked there by accident,
matching asyncpg's DETAIL line in the message fallback. Added
_driver_constraint_name() which walks orig then orig.__cause__, and the
stub tests now model that real shape (orig wrapper + __cause__) instead of
a constraint_name that no driver puts on orig directly.
Tests: 76 passed. New coverage for the email-race path, both new helpers
on each backend, the neutral fallback message, and the cause-chain walk.
* fix(bench): match app SQLite PRAGMAs in workers; fail a sweep on any op error
Two review follow-ups:
- worker.py opened its SQLite engine with only connect_args timeout=30.
synchronous and foreign_keys are per-connection PRAGMAs, so workers ran
at SQLite's synchronous=FULL / foreign_keys=OFF while a real Gateway
worker runs synchronous=NORMAL (persistence/engine.py::_enable_sqlite_wal)
-- an extra fsync per commit on the measured 30%-write path, overstating
SQLite's cost in the direction that flatters the "use Postgres"
conclusion. Added a connect listener applying the same four PRAGMAs, with
a test asserting synchronous/foreign_keys/journal_mode on a real worker
connection.
- summary_indicates_failure() only looked at crashes and the completed vs
expected op counts, so a sweep where every op completed but raised
(e.g. writes hitting OperationalError) passed as a clean measurement:
completed_ops == expected, 0 crashes. Added an "errors > 0" clause; the
error breakdown stays in the JSON, only the exit code changes. Test added.
test_bench_concurrency.py + test_bench_worker.py green (20), plus a real
2-worker sqlite smoke run (6/6 ops, 0 errors, exit 0).
* fix(auth): match the real email index name; only claim "exists" for uniqueness
Review follow-ups on the classification helpers:
- email is mapped_column(unique=True, index=True), which SQLAlchemy and
0001_baseline realise as a single UNIQUE INDEX (ix_users_email), not a
named UNIQUE constraint. _is_email_violation compared the driver
constraint name against "users_email_key", which Postgres never emits,
so that arm was dead on Postgres (SQLite matched via the message). Fixed
to ix_users_email.
- the residual IntegrityError fallback raised "User already exists" for
every remaining IntegrityError -- a NOT NULL / CHECK / foreign-key
violation is not a "user already exists" condition and is not part of
create_user's ValueError contract. Added _is_uniqueness_violation
(sqlstate 23505, or the SQLite "UNIQUE/PRIMARY KEY constraint failed"
message); only that raises the "already exists" ValueError, everything
else propagates unchanged.
- documented scripts/benchmark/concurrency/ in backend/AGENTS.md alongside
the other benchmark family.
Tests: 78 auth + 20 bench-unit pass, ruff clean. New coverage for
_is_uniqueness_violation on both backends and for a non-uniqueness
IntegrityError propagating out of create_user.
* fix(bench): don't pre-close worker stdin (breaks communicate); require --pg-url for postgres
* fix(bench): ruff format; time throughput on the op phase, not teardown
- lint-backend: ruff format the files touched in this PR.
- Throughput window (P2): the orchestrator sampled its wall clock after
every worker's communicate() returned, so it also covered each worker's
engine.dispose(), result serialization and stdout transfer. Each worker
now times just its operation phase (GO -> last op) and reports
ops_elapsed_s; summarize() uses max(ops_elapsed_s) over the workers -- all
released by the same GO -- as the throughput window (ops_window_s).
- Exercise migration 0018 (P2): test_user_oauth_partial_index.py goes
through bootstrap create_all(), which builds the partial index from ORM
metadata and never runs 0018.upgrade(). New Postgres-gated
test_migration_0018_oauth_identity_pg_partial.py alembic-upgrades to 0017
(full index), then 0018 (asserts the predicate appears), then downgrades
(asserts the full index is restored) and re-upgrades.
* docs(middlewares): tighten SandboxAudit and Clarification entries in AGENTS.md
PR #5134 grew agents/middlewares/AGENTS.md ~1.8 KB, pushing the effective
AGENTS.md chain for that directory over the 96 KiB hard limit once this
branch also documents scripts/benchmark/concurrency/ in backend/AGENTS.md.
Condense the two longest middleware entries (SandboxAuditMiddleware,
ClarificationMiddleware) without dropping any identifier, example, issue
reference, ordering constraint, or documented gap; chain back to ~96.8 KiB.
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
Ryker_Feng
|
8d8ca506ba
|
feat(artifacts): download run files as zip (#5117)
* feat(artifacts): download run files as zip * fix(artifacts): address archive review feedback * fix(artifacts): gate unavailable archive downloads * fix(artifacts): verify archive availability * fix(artifacts): harden archive consistency * fix(artifacts): reject archive path aliases |
||
|
Sunshine
|
bf740ffa90
|
feat(auth): add personal access tokens for programmatic API access (#5041)
* feat(auth): add personal access tokens for programmatic API access (#4849) Backend-first implementation of the PAT contract from #4849: show-once dfp_ tokens bound to their owning user (AUTH_SOURCE_PAT, is_internal=false), digest-only storage (migration 0017), strict credential precedence (invalid Bearer is a 401, never cookie fallback), CSRF double-submit skipped only for Bearer requests while auth-endpoint origin checks still run, scopes intersecting the authz route permissions, session-auth-only PAT management and password changes, and throttled best-effort last_used_at stamps. * fix(auth): harden PAT scope boundary and schema parity from adversarial review Independent review of the initial draft found: (1) scopes only constrained the threads/runs permission axis while admin routes treated a PAT as its (possibly admin) owner — is_admin_user now rejects PAT callers outright since no scope grants admin capability; (2) the model declared a column UNIQUE constraint while migration 0017 created a named unique index, so downgrade failed on create_all-bootstrapped DBs — both now use the named unique index; (3) auth-disabled mode is an operator override and now stays ahead of the Bearer check so a stray Authorization header cannot 401 an E2E sandbox; plus wiring the previously-unused constants, bounding the last_used_at stamp cache, and four new tests (middleware-level expiry, expires_in_days, admin-capability rejection with session control, and the auth-disabled precedence). * docs(api): document personal access tokens for programmatic API access * fix(auth): close PAT security boundaries from review (default-deny routes, extension admin suppression) P1-1: scope intersection only constrains @require_permission routes, so undecorated mutation routes (DELETE /api/memory, POST /api/agents, Lark credential switching, channel config) accepted a PAT holding a single read scope. AuthMiddleware now enforces a default-deny route policy in auth/pat.py: PAT requests are admitted only to the thread/run lifecycle routes the v1 scopes govern; everything else answers 403 regardless of scopes. Session-cookie callers are unaffected. P1-2: the extension principal resolver projected is_admin/roles from the raw system_role, so an admin-owned PAT passed deerflow_extension_api.require_admin on contributed routes despite the documented no-admin guarantee. The projection is now PAT-aware and suppresses every admin signal for PAT callers, mirroring deps.is_admin_user. Both fixes carry regression tests (route outside policy 403 + session control; production resolver admin suppression), and API.md documents the default-deny boundary. * fix(auth): enforce PAT scopes on stateless run entry and harden decorator Follow-up hardening from an independent audit of the P1 fixes: - POST /api/runs/stream and /api/runs/wait were the only allowlisted run entrypoints without @require_permission, so a threads:read-only PAT could still start runs (same bug class as P1-1, now closed): both now carry @require_permission("runs", "create"). POST /api/threads and POST /api/threads/search gain threads:write / threads:read for the same reason. Authorization-disabled deployments see no change (the permission set resolves to all permissions). - require_permission now binds the wrapped signature to locate a positionally-passed request before injecting the test stub, fixing 'got multiple values for argument' on direct positional unit-test calls. - API.md: the intro PAT example used GET /api/models, which the new default-deny policy 403s — replaced with GET /api/threads; the default-deny route list now spells out method sets. Regression test: threads:read-only PAT is 403 on the decorated stateless entry while a runs:create PAT passes. * fix(auth): address review P2s (empty Authorization header, PAT name trimming, API example) - CSRFMiddleware treats an explicitly empty Authorization header as present (is None), so an invalid credential always reaches AuthMiddleware's uniform 401 instead of a CSRF 403 that varies by method/CSRF state. Regression: empty-header request dies at auth. - PATCreateRequest strips the name and rejects whitespace-only values before token generation; created names are stored trimmed. - API.md intro PAT example now uses the implemented POST /api/threads/search endpoint (GET /api/threads does not exist). - AGENTS.md trimmed back under the guidance soft budget after the upstream merge. * fix(auth): tighten PAT route policy to implemented methods only The allowlist admitted GET /api/threads, a method no router implements. Pre-authorizing a dead method weakens the default-deny boundary: a future GET collection route added without a permission decorator would become PAT-reachable without an explicit policy change. Restrict the rule to POST, fix the stale GET description in API.md's PAT constraints, and document the default-deny boundary accurately in the gateway AGENTS.md guidance (only the threads/runs allowlist is PAT-reachable; every other authenticated route 403s PAT callers). Audited every remaining rule against the mounted routers: all other method+path entries map to real routes. Regression: test_pat_policy_does_not_pre_authorize_unimplemented_methods. * test(auth): guarantee the negative digest test mutates the token token[:-1] + "X" is identical to the original whenever the generated token already ends in X (1/62), making the negative digest assertion fail intermittently. Choose the replacement character based on the existing tail so the mutated token always differs. * fix(auth): require runs:cancel for cancel-then-stream requests stream_existing_run is gated at runs:read so action-less stream joins work with read-only credentials, but its ?action=interrupt|rollback branch cancels the run — a separate permission. A runs:read-only PAT passed both the PAT route policy and the route decorator and could interrupt or roll back an active run, bypassing the runs:cancel scope. Decorators cannot express query-parameter-conditional permissions, so the check lives in require_cancel_permission_when_action(), applied at the top of the handler. Regression drives the real helper through the production middleware: runs:read-only PAT + action is 403, the same token joins action-less, runs:read+cancel passes, session control unaffected. * docs(changelog): add the PAT feature entry * docs(readme): add personal access tokens section Repo documentation-update policy requires user-facing features to update README.md in the same changeset; the PAT feature previously touched only backend/docs/API.md and the gateway AGENTS.md. * fix(auth): require runs:cancel for mutating multitask strategies All five run-creation entrypoints were gated only by runs:create, but RunCreateRequest.multitask_strategy accepts interrupt/rollback and start_run forwards it to create_or_reject, which terminates an already-active run. A runs:create-only PAT could therefore kill an existing run through a create request, bypassing runs:cancel. Decorators cannot express body-parameter-conditional permissions, and per-route checks leave the same hole for the next entrypoint, so the gate lives in start_run itself — the single choke point every run-creation path (HTTP routes and internal launchers) flows through. Regenerate launches pass multitask_strategy="reject" and are unaffected; requests without a stamped auth context (internal/test compositions) skip the gate. The check is the shared authz.require_cancel_permission_if primitive; require_cancel_permission_when_action now delegates to it, so every request dimension that carries cancel capability (query action, body strategy) flows through one gate. Regression drives the real middleware stack: runs:create-only PAT + interrupt/rollback is 403 with the exact detail, reject (explicit and default) stays available, runs:create+cancel passes, session control unaffected; a source anchor pins the gate inside start_run. * fix(runs): keep observer joins from applying creator cancel-on-disconnect sse_consumer's finally block applied the record's on_disconnect=cancel policy on ANY consumer's disconnect. The join surfaces (GET /join and the action-less GET/POST stream join) feed it the existing RunRecord, so anyone with thread read access — including a runs:read-only PAT — could cancel a locally-owned running run simply by closing the SSE connection, without runs:cancel. The policy expresses the creator's intent for their own connection; an observer's disconnect must never be read as that intent. sse_consumer gains apply_on_disconnect (default True). The two join surfaces pass False; the creating endpoints (thread-scoped and stateless create-and-stream) keep the creator semantics unchanged. wait_for_run_completion needs no change: its callers are creator-side or post-explicit-cancel paths only. Regression exercises a real generator close — the same machinery Starlette drives on client disconnect — against the production sse_consumer: creator stream disconnect cancels, observer join disconnect does not; a wiring anchor pins both join call sites and the creator defaults. API.md documents the cancel-capability constraint (this fix plus the action/strategy gates) in PAT Constraints. * test(auth): pin the multitask gate behaviorally; state wait invariant Independent adversarial review of the round-5 fixes found the P1-a regression only mirror-pinned: the source anchor could be satisfied by a comment, and deleting the gate from start_run would not fail the suite. This drives the production start_run directly — a create-only auth context gets 403 with the exact detail for interrupt, and a reject request with no cancel permission at all proceeds past the gate (never a permission 403). Also documents wait_for_run_completion's creator-side invariant (every caller is the creating endpoint or post-explicit-cancel) so a future observer wiring thinks twice before reusing it — the one-caller- away variant of the observer-disconnect P1. * docs(changelog): correct the PAT entry's digest and route-policy description The entry said HMAC digests (the implementation stores SHA-256 digests, as documented in API.md and pinned by the repository tests) and claimed the route policy admits 'implemented stateless endpoints' (it admits the thread/run lifecycle routes, narrowing further by scopes). Also notes the cancel-capability gate now covering action and multitask strategies. * fix(auth): enumerate the PAT runs route policy per implemented subroute The runs subtree rule was a GET|POST /runs(/.*)? wildcard — it pre-authorized every current and future subroute under /runs, including methods the router never implemented (e.g. GET /runs/stream), which is the same latent default-deny weakening the threads collection rule was tightened for: a future route added under /runs would become PAT-reachable without an explicit policy change. The wildcard is replaced with six segment-precise rules covering exactly the 14 implemented method+path combinations; the {run_id} slot necessarily matches any single segment, so the POST-only collection names (stream, wait, regenerate, edit-regenerate) are excluded from the GET run-id rule via negative lookahead — no dead method stays pre-authorized. Behavior for implemented routes is unchanged. test_pat_runs_policy_admits_exactly_the_mounted_routes derives the expected set from the mounted thread_runs router instead of a hand-maintained list: every implemented GET/POST route under /runs must be admitted, routes in this router outside the subtree stay denied, and representative unimplemented neighbors are denied — so adding a route under /runs now fails CI until it is explicitly allowlisted, and a removed route leaves a dead rule visible. API.md's PAT constraints list the enumerated routes and drops a feedback mention that belonged to the stateless /api/runs axis. * docs(migration): add the 0017 renumbering coordination note to 0017 The PR's migration-coordination comment states each migration file carries the note; the file did not. Adds it: numbering was generated against main head 0016 alongside #5078 and #4843; whoever merges first keeps the slot, the others renumber on rebase (revision/down_revision plus the bootstrap head assertions). * fix(auth): pad base62 tokens to a fixed 43-char width int.from_bytes discards leading zero bytes, so the unpadded encoder returned a variable-length body — empty for all-zero input, and shorter than 40 characters for any draw below 62**39 (~1 in 14.5M), leaving test_generate_pat_token_format probabilistically flaky and the token body without stable width (review round 6, P3). _base62 now left-pads with "0" to _base62_width(len(data)) — the exact integer digit count (62^43 > 2^256 > 62^42, so 43 for 32 bytes). The format test asserts the exact fixed width instead of a probabilistic floor, and a new unit test pins the all-zero, leading-zero-byte, and max-value edges deterministically. |
||
|
Daoyuan Li
|
b5cc3a81c3
|
fix(auth): resolve email accounts case-insensitively (#4101)
* fix(auth): handle email addresses case-insensitively Normalize new and changed addresses, use case-insensitive lookup, and keep legacy case-variant rows stable during unrelated updates. * fix(auth): reject legacy case-variant registrations --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
Aari
|
09e25b8a32
|
fix(auth): let deployments close local self-registration (#4311)
* fix(auth): let deployments close local self-registration The OIDC provisioning policy (allowed_email_domains, require_verified_email, auto_create_users) is enforced only in the SSO callback via get_or_provision_oidc_user. POST /api/v1/auth/register creates a local account without consulting any of it, and nothing can turn that path off, so a deployment declaring an email-domain allowlist can still be joined by any address through local registration. Add auth.local.allow_registration (default true, so existing deployments are unchanged) and gate /register on it before the account is created. Report the flag from /setup-status so the login page stops offering a signup entry the Gateway will reject. /initialize is deliberately not gated: it is the bootstrap path, guarded by admin_count == 0, and closing it would leave a fresh install unable to create its first admin. An unreadable config.yaml falls back to the pre-gate default (open) rather than making these two endpoints a hard dependency on the file. * docs(auth): align registration-gate fallback wording with the FileNotFoundError catch |
||
|
Ryker_Feng
|
a028dfd5fb
|
feat(auth): add keep me signed in login option (#4255)
* feat(auth): add keep me signed in login option * fix(auth): address remember-login review feedback |
||
|
Zheng Feng
|
ee8ad1bc67
|
feat(auth): add OIDC SSO support (#3506)
Add provider-agnostic OIDC authentication with Keycloak-compatible configuration, frontend SSO UI support |
||
|
Willem Jiang
|
6d611c2bf6
|
fix(auth): persist auto-generated JWT secret to survive restarts (#2933)
* fix(auth): persist auto-generated JWT secret to survive restarts When AUTH_JWT_SECRET is not set, the auto-generated secret is now written to .deer-flow/.jwt_secret (mode 0600) and reused on subsequent starts. This prevents session invalidation on every restart while still allowing explicit AUTH_JWT_SECRET in .env to take precedence. * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix the lint errors of backend --------- Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
greatmengqi
|
f734e14d8b
|
docs: document auth design and user isolation (#2913)
* docs: document auth design and user isolation * docs: align auth docs with current storage and reset behavior --------- Co-authored-by: greatmengqi <chenmengqi.0376@bytedance.com> |
||
|
Willem Jiang
|
4e4e4f92a0
|
fix(security): harden auth system and fix run journal logic bug (#2593)
* fix(security): harden auth system and fix run journal logic bug
- Fix inverted condition in RunJournal.on_chat_model_start that prevented
first human message capture (not messages → messages)
- Pre-hash passwords with SHA-256 before bcrypt to avoid silent 72-byte
truncation vulnerability
- Move load_dotenv() from module scope into get_auth_config() to prevent
import-time os.environ mutation breaking test isolation
- Return generic ‘Invalid token’ instead of exposing specific error
variants (expired, malformed, invalid_signature) to clients
- Make @require_auth independently enforce 401 instead of silently
passing through when AuthMiddleware is absent
- Rate-limit /setup-status endpoint with per-IP cooldown to mitigate
initialization-state information leak
- Document in-process rate limiter limitation for multi-worker deployments
* fix(security): return 429+Retry-After on setup-status rate limit, bound cooldown dict
Agent-Logs-Url: https://github.com/bytedance/deer-flow/sessions/070d0be8-99a5-46c8-85bb-6b81b5284021
Co-authored-by: WillemJiang <219644+WillemJiang@users.noreply.github.com>
* fix(security): add versioned password hashes with auto-migration on login
The SHA-256 pre-hash change silently broke verification for any existing
bcrypt-only password hashes. Introduce a <N>$ prefix scheme so hashes
are self-describing:
- v2 (current): bcrypt(b64(sha256(password))) with $ prefix
- v1 (legacy): plain bcrypt, prefixed $ or bare (no prefix)
verify_password auto-detects the version and falls back to v1 for older
hashes. LocalAuthProvider.authenticate() now rehashes legacy hashes to v2
on successful login via needs_rehash(), so existing users upgrade
transparently without a dedicated migration step.
* fix(auth): harden verify_password, best-effort rehash, update require_auth docstring, downgrade journal logging
- password.py: wrap bcrypt.checkpw in try/except → return False for malformed/corrupt hashes instead of crashing
- local_provider.py: wrap auto-rehash update_user() in try/except so transient DB errors don't fail valid logins
- authz.py: update require_auth docstring to reflect independent 401 enforcement
- journal.py: downgrade on_chat_model_start from INFO to DEBUG, log only metadata (batch_count, message_counts) instead of full serialized/messages content
Agent-Logs-Url: https://github.com/bytedance/deer-flow/sessions/48c5cf31-a4ab-418a-982a-6343c37bb299
Co-authored-by: WillemJiang <219644+WillemJiang@users.noreply.github.com>
* fix(auth): address code review - narrow ValueError catch, add rehash warning log, rename num_batches
Agent-Logs-Url: https://github.com/bytedance/deer-flow/sessions/48c5cf31-a4ab-418a-982a-6343c37bb299
Co-authored-by: WillemJiang <219644+WillemJiang@users.noreply.github.com>
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
|
||
|
Willem Jiang
|
ac18b9c424 | Apply the code reviewer suggestion of abstractmethod | ||
|
Willem Jiang
|
653b7ae17a | Apply the code reviewer suggestion of abstractmethod | ||
|
foreleven
|
00a90bbd3d | refactor: Remove init_token handling from admin initialization logic and related tests | ||
|
Copilot
|
848ace98cb |
feat: replace auto-admin creation with secure interactive first-boot setup (#2063)
* feat(persistence): add unified persistence layer with event store, token tracking, and feedback (#1930) * feat(persistence): add SQLAlchemy 2.0 async ORM scaffold Introduce a unified database configuration (DatabaseConfig) that controls both the LangGraph checkpointer and the DeerFlow application persistence layer from a single `database:` config section. New modules: - deerflow.config.database_config — Pydantic config with memory/sqlite/postgres backends - deerflow.persistence — async engine lifecycle, DeclarativeBase with to_dict mixin, Alembic skeleton - deerflow.runtime.runs.store — RunStore ABC + MemoryRunStore implementation Gateway integration initializes/tears down the persistence engine in the existing langgraph_runtime() context manager. Legacy checkpointer config is preserved for backward compatibility. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(persistence): add RunEventStore ABC + MemoryRunEventStore Phase 2-A prerequisite for event storage: adds the unified run event stream interface (RunEventStore) with an in-memory implementation, RunEventsConfig, gateway integration, and comprehensive tests (27 cases). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(persistence): add ORM models, repositories, DB/JSONL event stores, RunJournal, and API endpoints Phase 2-B: run persistence + event storage + token tracking. - ORM models: RunRow (with token fields), ThreadMetaRow, RunEventRow - RunRepository implements RunStore ABC via SQLAlchemy ORM - ThreadMetaRepository with owner access control - DbRunEventStore with trace content truncation and cursor pagination - JsonlRunEventStore with per-run files and seq recovery from disk - RunJournal (BaseCallbackHandler) captures LLM/tool/lifecycle events, accumulates token usage by caller type, buffers and flushes to store - RunManager now accepts optional RunStore for persistent backing - Worker creates RunJournal, writes human_message, injects callbacks - Gateway deps use factory functions (RunRepository when DB available) - New endpoints: messages, run messages, run events, token-usage - ThreadCreateRequest gains assistant_id field - 92 tests pass (33 new), zero regressions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(persistence): add user feedback + follow-up run association Phase 2-C: feedback and follow-up tracking. - FeedbackRow ORM model (rating +1/-1, optional message_id, comment) - FeedbackRepository with CRUD, list_by_run/thread, aggregate stats - Feedback API endpoints: create, list, stats, delete - follow_up_to_run_id in RunCreateRequest (explicit or auto-detected from latest successful run on the thread) - Worker writes follow_up_to_run_id into human_message event metadata - Gateway deps: feedback_repo factory + getter - 17 new tests (14 FeedbackRepository + 3 follow-up association) - 109 total tests pass, zero regressions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test+config: comprehensive Phase 2 test coverage + deprecate checkpointer config - config.example.yaml: deprecate standalone checkpointer section, activate unified database:sqlite as default (drives both checkpointer + app data) - New: test_thread_meta_repo.py (14 tests) — full ThreadMetaRepository coverage including check_access owner logic, list_by_owner pagination - Extended test_run_repository.py (+4 tests) — completion preserves fields, list ordering desc, limit, owner_none returns all - Extended test_run_journal.py (+8 tests) — on_chain_error, track_tokens=false, middleware no ai_message, unknown caller tokens, convenience fields, tool_error, non-summarization custom event - Extended test_run_event_store.py (+7 tests) — DB batch seq continuity, make_run_event_store factory (memory/db/jsonl/fallback/unknown) - Extended test_phase2b_integration.py (+4 tests) — create_or_reject persists, follow-up metadata, summarization in history, full DB-backed lifecycle - Fixed DB integration test to use proper fake objects (not MagicMock) for JSON-serializable metadata - 157 total Phase 2 tests pass, zero regressions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * config: move default sqlite_dir to .deer-flow/data Keep SQLite databases alongside other DeerFlow-managed data (threads, memory) under the .deer-flow/ directory instead of a top-level ./data folder. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(persistence): remove UTFJSON, use engine-level json_serializer + datetime.now() - Replace custom UTFJSON type with standard sqlalchemy.JSON in all ORM models. Add json_serializer=json.dumps(ensure_ascii=False) to all create_async_engine calls so non-ASCII text (Chinese etc.) is stored as-is in both SQLite and Postgres. - Change ORM datetime defaults from datetime.now(UTC) to datetime.now(), remove UTC imports. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(gateway): simplify deps.py with getter factory + inline repos - Replace 6 identical getter functions with _require() factory. - Inline 3 _make_*_repo() factories into langgraph_runtime(), call get_session_factory() once instead of 3 times. - Add thread_meta upsert in start_run (services.py). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(docker): add UV_EXTRAS build arg for optional dependencies Support installing optional dependency groups (e.g. postgres) at Docker build time via UV_EXTRAS build arg: UV_EXTRAS=postgres docker compose build Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(journal): fix flush, token tracking, and consolidate tests RunJournal fixes: - _flush_sync: retain events in buffer when no event loop instead of dropping them; worker's finally block flushes via async flush(). - on_llm_end: add tool_calls filter and caller=="lead_agent" guard for ai_message events; mark message IDs for dedup with record_llm_usage. - worker.py: persist completion data (tokens, message count) to RunStore in finally block. Model factory: - Auto-inject stream_usage=True for BaseChatOpenAI subclasses with custom api_base, so usage_metadata is populated in streaming responses. Test consolidation: - Delete test_phase2b_integration.py (redundant with existing tests). - Move DB-backed lifecycle test into test_run_journal.py. - Add tests for stream_usage injection in test_model_factory.py. - Clean up executor/task_tool dead journal references. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(events): widen content type to str|dict in all store backends Allow event content to be a dict (for structured OpenAI-format messages) in addition to plain strings. Dict values are JSON-serialized for the DB backend and deserialized on read; memory and JSONL backends handle dicts natively. Trace truncation now serializes dicts to JSON before measuring. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(events): use metadata flag instead of heuristic for dict content detection Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(converters): add LangChain-to-OpenAI message format converters Pure functions langchain_to_openai_message, langchain_to_openai_completion, langchain_messages_to_openai, and _infer_finish_reason for converting LangChain BaseMessage objects to OpenAI Chat Completions format, used by RunJournal for event storage. 15 unit tests added. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(converters): handle empty list content as null, clean up test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(events): human_message content uses OpenAI user message format Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(events): ai_message uses OpenAI format, add ai_tool_call message event - ai_message content now uses {"role": "assistant", "content": "..."} format - New ai_tool_call message event emitted when lead_agent LLM responds with tool_calls - ai_tool_call uses langchain_to_openai_message converter for consistent format - Both events include finish_reason in metadata ("stop" or "tool_calls") Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(events): add tool_result message event with OpenAI tool message format Cache tool_call_id from on_tool_start keyed by run_id as fallback for on_tool_end, then emit a tool_result message event (role=tool, tool_call_id, content) after each successful tool completion. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(events): summary content uses OpenAI system message format Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(events): replace llm_start/llm_end with llm_request/llm_response in OpenAI format Add on_chat_model_start to capture structured prompt messages as llm_request events. Replace llm_end trace events with llm_response using OpenAI Chat Completions format. Track llm_call_index to pair request/response events. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(events): add record_middleware method for middleware trace events Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(events): add full run sequence integration test for OpenAI content format Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(events): align message events with checkpoint format and add middleware tag injection - Message events (ai_message, ai_tool_call, tool_result, human_message) now use BaseMessage.model_dump() format, matching LangGraph checkpoint values.messages - on_tool_end extracts tool_call_id/name/status from ToolMessage objects - on_tool_error now emits tool_result message events with error status - record_middleware uses middleware:{tag} event_type and middleware category - Summarization custom events use middleware:summarize category - TitleMiddleware injects middleware:title tag via get_config() inheritance - SummarizationMiddleware model bound with middleware:summarize tag - Worker writes human_message using HumanMessage.model_dump() Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(threads): switch search endpoint to threads_meta table and sync title - POST /api/threads/search now queries threads_meta table directly, removing the two-phase Store + Checkpointer scan approach - Add ThreadMetaRepository.search() with metadata/status filters - Add ThreadMetaRepository.update_display_name() for title sync - Worker syncs checkpoint title to threads_meta.display_name on run completion - Map display_name to values.title in search response for API compatibility Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(threads): history endpoint reads messages from event store - POST /api/threads/{thread_id}/history now combines two data sources: checkpointer for checkpoint_id, metadata, title, thread_data; event store for messages (complete history, not truncated by summarization) - Strip internal LangGraph metadata keys from response - Remove full channel_values serialization in favor of selective fields Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: remove duplicate optional-dependencies header in pyproject.toml Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(middleware): pass tagged config to TitleMiddleware ainvoke call Without the config, the middleware:title tag was not injected, causing the LLM response to be recorded as a lead_agent ai_message in run_events. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: resolve merge conflict in .env.example Keep both DATABASE_URL (from persistence-scaffold) and WECOM credentials (from main) after the merge. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(persistence): address review feedback on PR #1851 - Fix naive datetime.now() → datetime.now(UTC) in all ORM models - Fix seq race condition in DbRunEventStore.put() with FOR UPDATE and UNIQUE(thread_id, seq) constraint - Encapsulate _store access in RunManager.update_run_completion() - Deduplicate _store.put() logic in RunManager via _persist_to_store() - Add update_run_completion to RunStore ABC + MemoryRunStore - Wire follow_up_to_run_id through the full create path - Add error recovery to RunJournal._flush_sync() lost-event scenario - Add migration note for search_threads breaking change - Fix test_checkpointer_none_fix mock to set database=None Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: update uv.lock Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(persistence): address 22 review comments from CodeQL, Copilot, and Code Quality Bug fixes: - Sanitize log params to prevent log injection (CodeQL) - Reset threads_meta.status to idle/error when run completes - Attach messages only to latest checkpoint in /history response - Write threads_meta on POST /threads so new threads appear in search Lint fixes: - Remove unused imports (journal.py, migrations/env.py, test_converters.py) - Convert lambda to named function (engine.py, Ruff E731) - Remove unused logger definitions in repos (Ruff F841) - Add logging to JSONL decode errors and empty except blocks - Separate assert side-effects in tests (CodeQL) - Remove unused local variables in tests (Ruff F841) - Fix max_trace_content truncation to use byte length, not char length Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: apply ruff format to persistence and runtime files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Potential fix for pull request finding 'Statement has no effect' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> * refactor(runtime): introduce RunContext to reduce run_agent parameter bloat Extract checkpointer, store, event_store, run_events_config, thread_meta_repo, and follow_up_to_run_id into a frozen RunContext dataclass. Add get_run_context() in deps.py to build the base context from app.state singletons. start_run() uses dataclasses.replace() to enrich per-run fields before passing ctx to run_agent. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(gateway): move sanitize_log_param to app/gateway/utils.py Extract the log-injection sanitizer from routers/threads.py into a shared utils module and rename to sanitize_log_param (public API). Eliminates the reverse service → router import in services.py. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * perf: use SQL aggregation for feedback stats and thread token usage Replace Python-side counting in FeedbackRepository.aggregate_by_run with a single SELECT COUNT/SUM query. Add RunStore.aggregate_tokens_by_thread abstract method with SQL GROUP BY implementation in RunRepository and Python fallback in MemoryRunStore. Simplify the thread_token_usage endpoint to delegate to the new method, eliminating the limit=10000 truncation risk. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: annotate DbRunEventStore.put() as low-frequency path Add docstring clarifying that put() opens a per-call transaction with FOR UPDATE and should only be used for infrequent writes (currently just the initial human_message event). High-throughput callers should use put_batch() instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(threads): fall back to Store search when ThreadMetaRepository is unavailable When database.backend=memory (default) or no SQL session factory is configured, search_threads now queries the LangGraph Store instead of returning 503. Returns empty list if neither Store nor repo is available. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(persistence): introduce ThreadMetaStore ABC for backend-agnostic thread metadata Add ThreadMetaStore abstract base class with create/get/search/update/delete interface. ThreadMetaRepository (SQL) now inherits from it. New MemoryThreadMetaStore wraps LangGraph BaseStore for memory-mode deployments. deps.py now always provides a non-None thread_meta_repo, eliminating all `if thread_meta_repo is not None` guards in services.py, worker.py, and routers/threads.py. search_threads no longer needs a Store fallback branch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(history): read messages from checkpointer instead of RunEventStore The /history endpoint now reads messages directly from the checkpointer's channel_values (the authoritative source) instead of querying RunEventStore.list_messages(). The RunEventStore API is preserved for other consumers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(persistence): address new Copilot review comments - feedback.py: validate thread_id/run_id before deleting feedback - jsonl.py: add path traversal protection with ID validation - run_repo.py: parse `before` to datetime for PostgreSQL compat - thread_meta_repo.py: fix pagination when metadata filter is active - database_config.py: use resolve_path for sqlite_dir consistency Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Implement skill self-evolution and skill_manage flow (#1874) * chore: ignore .worktrees directory * Add skill_manage self-evolution flow * Fix CI regressions for skill_manage * Address PR review feedback for skill evolution * fix(skill-evolution): preserve history on delete * fix(skill-evolution): tighten scanner fallbacks * docs: add skill_manage e2e evidence screenshot * fix(skill-manage): avoid blocking fs ops in session runtime --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> * fix(config): resolve sqlite_dir relative to CWD, not Paths.base_dir resolve_path() resolves relative to Paths.base_dir (.deer-flow), which double-nested the path to .deer-flow/.deer-flow/data/app.db. Use Path.resolve() (CWD-relative) instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Feature/feishu receive file (#1608) * feat(feishu): add channel file materialization hook for inbound messages - Introduce Channel.receive_file(msg, thread_id) as a base method for file materialization; default is no-op. - Implement FeishuChannel.receive_file to download files/images from Feishu messages, save to sandbox, and inject virtual paths into msg.text. - Update ChannelManager to call receive_file for any channel if msg.files is present, enabling downstream model access to user-uploaded files. - No impact on Slack/Telegram or other channels (they inherit the default no-op). * style(backend): format code with ruff for lint compliance - Auto-formatted packages/harness/deerflow/agents/factory.py and tests/test_create_deerflow_agent.py using `ruff format` - Ensured both files conform to project linting standards - Fixes CI lint check failures caused by code style issues * fix(feishu): handle file write operation asynchronously to prevent blocking * fix(feishu): rename GetMessageResourceRequest to _GetMessageResourceRequest and remove redundant code * test(feishu): add tests for receive_file method and placeholder replacement * fix(manager): remove unnecessary type casting for channel retrieval * fix(feishu): update logging messages to reflect resource handling instead of image * fix(feishu): sanitize filename by replacing invalid characters in file uploads * fix(feishu): improve filename sanitization and reorder image key handling in message processing * fix(feishu): add thread lock to prevent filename conflicts during file downloads * fix(test): correct bad merge in test_feishu_parser.py * chore: run ruff and apply formatting cleanup fix(feishu): preserve rich-text attachment order and improve fallback filename handling * fix(docker): restore gateway env vars and fix langgraph empty arg issue (#1915) Two production docker-compose.yaml bugs prevent `make up` from working: 1. Gateway missing DEER_FLOW_CONFIG_PATH and DEER_FLOW_EXTENSIONS_CONFIG_PATH environment overrides. Added in fb2d99f (#1836) but accidentally reverted by ca2fb95 (#1847). Without them, gateway reads host paths from .env via env_file, causing FileNotFoundError inside the container. 2. Langgraph command fails when LANGGRAPH_ALLOW_BLOCKING is unset (default). Empty $${allow_blocking} inserts a bare space between flags, causing ' --no-reload' to be parsed as unexpected extra argument. Fix by building args string first and conditionally appending --allow-blocking. Co-authored-by: cooper <cooperfu@tencent.com> * fix(frontend): resolve invalid HTML nesting and tabnabbing vulnerabilities (#1904) * fix(frontend): resolve invalid HTML nesting and tabnabbing vulnerabilities Fix `<button>` inside `<a>` invalid HTML in artifact components and add missing `noopener,noreferrer` to `window.open` calls to prevent reverse tabnabbing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(frontend): address Copilot review on tabnabbing and double-tab-open Remove redundant parent onClick on web_fetch ChainOfThoughtStep to prevent opening two tabs on link click, and explicitly null out window.opener after window.open() for defensive tabnabbing hardening. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * refactor(persistence): organize entities into per-entity directories Restructure the persistence layer from horizontal "models/ + repositories/" split into vertical entity-aligned directories. Each entity (thread_meta, run, feedback) now owns its ORM model, abstract interface (where applicable), and concrete implementations under a single directory with an aggregating __init__.py for one-line imports. Layout: persistence/thread_meta/{base,model,sql,memory}.py persistence/run/{model,sql}.py persistence/feedback/{model,sql}.py models/__init__.py is kept as a facade so Alembic autogenerate continues to discover all ORM tables via Base.metadata. RunEventRow remains under models/run_event.py because its storage implementation lives in runtime/events/store/db.py and has no matching repository directory. The repositories/ directory is removed entirely. All call sites in gateway/deps.py and tests are updated to import from the new entity packages, e.g.: from deerflow.persistence.thread_meta import ThreadMetaRepository from deerflow.persistence.run import RunRepository from deerflow.persistence.feedback import FeedbackRepository Full test suite passes (1690 passed, 14 skipped). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(gateway): sync thread rename and delete through ThreadMetaStore The POST /threads/{id}/state endpoint previously synced title changes only to the LangGraph Store via _store_upsert. In sqlite mode the search endpoint reads from the ThreadMetaRepository SQL table, so renames never appeared in /threads/search until the next agent run completed (worker.py syncs title from checkpoint to thread_meta in its finally block). Likewise the DELETE /threads/{id} endpoint cleaned up the filesystem, Store, and checkpointer but left the threads_meta row orphaned in sqlite, so deleted threads kept appearing in /threads/search. Fix both endpoints by routing through the ThreadMetaStore abstraction which already has the correct sqlite/memory implementations wired up by deps.py. The rename path now calls update_display_name() and the delete path calls delete() — both work uniformly across backends. Verified end-to-end with curl in gateway mode against sqlite backend. Existing test suite (1690 passed) and focused router/repo tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(gateway): route all thread metadata access through ThreadMetaStore Following the rename/delete bug fix in PR1, migrate the remaining direct LangGraph Store reads/writes in the threads router and services to the ThreadMetaStore abstraction so that the sqlite and memory backends behave identically and the legacy dual-write paths can be removed. Migrated endpoints (threads.py): - create_thread: idempotency check + write now use thread_meta_repo.get/create instead of dual-writing the LangGraph Store and the SQL row. - get_thread: reads from thread_meta_repo.get; the checkpoint-only fallback for legacy threads is preserved. - patch_thread: replaced _store_get/_store_put with thread_meta_repo.update_metadata. - delete_thread_data: dropped the legacy store.adelete; thread_meta_repo.delete already covers it. Removed dead code (services.py): - _upsert_thread_in_store — redundant with the immediately following thread_meta_repo.create() call. - _sync_thread_title_after_run — worker.py's finally block already syncs the title via thread_meta_repo.update_display_name() after each run. Removed dead code (threads.py): - _store_get / _store_put / _store_upsert helpers (no remaining callers). - THREADS_NS constant. - get_store import (router no longer touches the LangGraph Store directly). New abstract method: - ThreadMetaStore.update_metadata(thread_id, metadata) merges metadata into the thread's metadata field. Implemented in both ThreadMetaRepository (SQL, read-modify-write inside one session) and MemoryThreadMetaStore. Three new unit tests cover merge / empty / nonexistent behaviour. Net change: -134 lines. Full test suite: 1693 passed, 14 skipped. Verified end-to-end with curl in gateway mode against sqlite backend (create / patch / get / rename / search / delete). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> Co-authored-by: DanielWalnut <45447813+hetaoBackend@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> Co-authored-by: JilongSun <965640067@qq.com> Co-authored-by: jie <49781832+stan-fu@users.noreply.github.com> Co-authored-by: cooper <cooperfu@tencent.com> Co-authored-by: yangzheli <43645580+yangzheli@users.noreply.github.com> * feat(auth): release-validation pass for 2.0-rc — 12 blockers + simplify follow-ups (#2008) * feat(auth): introduce backend auth module Port RFC-001 authentication core from PR #1728: - JWT token handling (create_access_token, decode_token, TokenPayload) - Password hashing (bcrypt) with verify_password - SQLite UserRepository with base interface - Provider Factory pattern (LocalAuthProvider) - CLI reset_admin tool - Auth-specific errors (AuthErrorCode, TokenError, AuthErrorResponse) Deps: - bcrypt>=4.0.0 - pyjwt>=2.9.0 - email-validator>=2.0.0 - backend/uv.toml pins public PyPI index Tests: 12 pure unit tests (test_auth_config.py, test_auth_errors.py). Scope note: authz.py, test_auth.py, and test_auth_type_system.py are deferred to commit 2 because they depend on middleware and deps wiring that is not yet in place. Commit 1 stays "pure new files only" as the spec mandates. * feat(auth): wire auth end-to-end (middleware + frontend replacement) Backend: - Port auth_middleware, csrf_middleware, langgraph_auth, routers/auth - Port authz decorator (owner_filter_key defaults to 'owner_id') - Merge app.py: register AuthMiddleware + CSRFMiddleware + CORS, add _ensure_admin_user lifespan hook, _migrate_orphaned_threads helper, register auth router - Merge deps.py: add get_local_provider, get_current_user_from_request, get_optional_user_from_request; keep get_current_user as thin str|None adapter for feedback router - langgraph.json: add auth path pointing to langgraph_auth.py:auth - Rename metadata['user_id'] -> metadata['owner_id'] in langgraph_auth (both metadata write and LangGraph filter dict) + test fixtures Frontend: - Delete better-auth library and api catch-all route - Remove better-auth npm dependency and env vars (BETTER_AUTH_SECRET, BETTER_AUTH_GITHUB_*) from env.js - Port frontend/src/core/auth/* (AuthProvider, gateway-config, proxy-policy, server-side getServerSideUser, types) - Port frontend/src/core/api/fetcher.ts - Port (auth)/layout, (auth)/login, (auth)/setup pages - Rewrite workspace/layout.tsx as server component that calls getServerSideUser and wraps in AuthProvider - Port workspace/workspace-content.tsx for the client-side sidebar logic Tests: - Port 5 auth test files (test_auth, test_auth_middleware, test_auth_type_system, test_ensure_admin, test_langgraph_auth) - 176 auth tests PASS After this commit: login/logout/registration flow works, but persistence layer does not yet filter by owner_id. Commit 4 closes that gap. * feat(auth): account settings page + i18n - Port account-settings-page.tsx (change password, change email, logout) - Wire into settings-dialog.tsx as new "account" section with UserIcon, rendered first in the section list - Add i18n keys: - en-US/zh-CN: settings.sections.account ("Account" / "账号") - en-US/zh-CN: button.logout ("Log out" / "退出登录") - types.ts: matching type declarations * feat(auth): enforce owner_id across 2.0-rc persistence layer Add request-scoped contextvar-based owner filtering to threads_meta, runs, run_events, and feedback repositories. Router code is unchanged — isolation is enforced at the storage layer so that any caller that forgets to pass owner_id still gets filtered results, and new routes cannot accidentally leak data. Core infrastructure ------------------- - deerflow/runtime/user_context.py (new): - ContextVar[CurrentUser | None] with default None - runtime_checkable CurrentUser Protocol (structural subtype with .id) - set/reset/get/require helpers - AUTO sentinel + resolve_owner_id(value, method_name) for sentinel three-state resolution: AUTO reads contextvar, explicit str overrides, explicit None bypasses the filter (for migration/CLI) Repository changes ------------------ - ThreadMetaRepository: create/get/search/update_*/delete gain owner_id=AUTO kwarg; read paths filter by owner, writes stamp it, mutations check ownership before applying - RunRepository: put/get/list_by_thread/delete gain owner_id=AUTO kwarg - FeedbackRepository: create/get/list_by_run/list_by_thread/delete gain owner_id=AUTO kwarg - DbRunEventStore: list_messages/list_events/list_messages_by_run/ count_messages/delete_by_thread/delete_by_run gain owner_id=AUTO kwarg. Write paths (put/put_batch) read contextvar softly: when a request-scoped user is available, owner_id is stamped; background worker writes without a user context pass None which is valid (orphan row to be bound by migration) Schema ------ - persistence/models/run_event.py: RunEventRow.owner_id = Mapped[ str | None] = mapped_column(String(64), nullable=True, index=True) - No alembic migration needed: 2.0 ships fresh, Base.metadata.create_all picks up the new column automatically Middleware ---------- - auth_middleware.py: after cookie check, call get_optional_user_from_ request to load the real User, stamp it into request.state.user AND the contextvar via set_current_user, reset in a try/finally. Public paths and unauthenticated requests continue without contextvar, and @require_auth handles the strict 401 path Test infrastructure ------------------- - tests/conftest.py: @pytest.fixture(autouse=True) _auto_user_context sets a default SimpleNamespace(id="test-user-autouse") on every test unless marked @pytest.mark.no_auto_user. Keeps existing 20+ persistence tests passing without modification - pyproject.toml [tool.pytest.ini_options]: register no_auto_user marker so pytest does not emit warnings for opt-out tests - tests/test_user_context.py: 6 tests covering three-state semantics, Protocol duck typing, and require/optional APIs - tests/test_thread_meta_repo.py: one test updated to pass owner_id= None explicitly where it was previously relying on the old default Test results ------------ - test_user_context.py: 6 passed - test_auth*.py + test_langgraph_auth.py + test_ensure_admin.py: 127 - test_run_event_store / test_run_repository / test_thread_meta_repo / test_feedback: 92 passed - Full backend suite: 1905 passed, 2 failed (both @requires_llm flaky integration tests unrelated to auth), 1 skipped * feat(auth): extend orphan migration to 2.0-rc persistence tables _ensure_admin_user now runs a three-step pipeline on every boot: Step 1 (fatal): admin user exists / is created / password is reset Step 2 (non-fatal): LangGraph store orphan threads → admin Step 3 (non-fatal): SQL persistence tables → admin - threads_meta - runs - run_events - feedback Each step is idempotent. The fatal/non-fatal split mirrors PR #1728's original philosophy: admin creation failure blocks startup (the system is unusable without an admin), whereas migration failures log a warning and let the service proceed (a partial migration is recoverable; a missing admin is not). Key helpers ----------- - _iter_store_items(store, namespace, *, page_size=500): async generator that cursor-paginates across LangGraph store pages. Fixes PR #1728's hardcoded limit=1000 bug that would silently lose orphans beyond the first page. - _migrate_orphaned_threads(store, admin_user_id): Rewritten to use _iter_store_items. Returns the migrated count so the caller can log it; raises only on unhandled exceptions. - _migrate_orphan_sql_tables(admin_user_id): Imports the 4 ORM models lazily, grabs the shared session factory, runs one UPDATE per table in a single transaction, commits once. No-op when no persistence backend is configured (in-memory dev). Tests: test_ensure_admin.py (8 passed) * test(auth): port AUTH test plan docs + lint/format pass - Port backend/docs/AUTH_TEST_PLAN.md and AUTH_UPGRADE.md from PR #1728 - Rename metadata.user_id → metadata.owner_id in AUTH_TEST_PLAN.md (4 occurrences from the original PR doc) - ruff auto-fix UP037 in sentinel type annotations: drop quotes around "str | None | _AutoSentinel" now that from __future__ import annotations makes them implicit string forms - ruff format: 2 files (app/gateway/app.py, runtime/user_context.py) Note on test coverage additions: - conftest.py autouse fixture was already added in commit 4 (had to be co-located with the repository changes to keep pre-existing persistence tests passing) - cross-user isolation E2E tests (test_owner_isolation.py) deferred — enforcement is already proven by the 98-test repository suite via the autouse fixture + explicit _AUTO sentinel exercises - New test cases (TC-API-17..20, TC-ATK-13, TC-MIG-01..07) listed in AUTH_TEST_PLAN.md are deferred to a follow-up PR — they are manual-QA test cases rather than pytest code, and the spec-level coverage is already met by test_user_context.py + the 98-test repository suite. Final test results: - Auth suite (test_auth*, test_langgraph_auth, test_ensure_admin, test_user_context): 186 passed - Persistence suite (test_run_event_store, test_run_repository, test_thread_meta_repo, test_feedback): 98 passed - Lint: ruff check + ruff format both clean * test(auth): add cross-user isolation test suite 10 tests exercising the storage-layer owner filter by manually switching the user_context contextvar between two users. Verifies the safety invariant: After a repository write with owner_id=A, a subsequent read with owner_id=B must not return the row, and vice versa. Covers all 4 tables that own user-scoped data: TC-API-17 threads_meta — read, search, update, delete cross-user TC-API-18 runs — get, list_by_thread, delete cross-user TC-API-19 run_events — list_messages, list_events, count_messages, delete_by_thread (CRITICAL: raw conversation content leak vector) TC-API-20 feedback — get, list_by_run, delete cross-user Plus two meta-tests verifying the sentinel pattern itself: - AUTO + unset contextvar raises RuntimeError - explicit owner_id=None bypasses the filter (migration escape hatch) Architecture note ----------------- These tests bypass the HTTP layer by design. The full chain (cookie → middleware → contextvar → repository) is covered piecewise: - test_auth_middleware.py: middleware sets contextvar from cookies - test_owner_isolation.py: repositories enforce isolation when contextvar is set to different users Together they prove the end-to-end safety property without the ceremony of spinning up a full TestClient + in-memory DB for every router endpoint. Tests pass: 231 (full auth + persistence + isolation suite) Lint: clean * refactor(auth): migrate user repository to SQLAlchemy ORM Move the users table into the shared persistence engine so auth matches the pattern of threads_meta, runs, run_events, and feedback — one engine, one session factory, one schema init codepath. New files --------- - persistence/user/__init__.py, persistence/user/model.py: UserRow ORM class with partial unique index on (oauth_provider, oauth_id) - Registered in persistence/models/__init__.py so Base.metadata.create_all() picks it up Modified -------- - auth/repositories/sqlite.py: rewritten as async SQLAlchemy, identical constructor pattern to the other four repositories (def __init__(self, session_factory) + self._sf = session_factory) - auth/config.py: drop users_db_path field — storage is configured through config.database like every other table - deps.py/get_local_provider: construct SQLiteUserRepository with the shared session factory, fail fast if engine is not initialised - tests/test_auth.py: rewrite test_sqlite_round_trip_new_fields to use the shared engine (init_engine + close_engine in a tempdir) - tests/test_auth_type_system.py: add per-test autouse fixture that spins up a scratch engine and resets deps._cached_* singletons * refactor(auth): remove SQL orphan migration (unused in supported scenarios) The _migrate_orphan_sql_tables helper existed to bind NULL owner_id rows in threads_meta, runs, run_events, and feedback to the admin on first boot. But in every supported upgrade path, it's a no-op: 1. Fresh install: create_all builds fresh tables, no legacy rows 2. No-auth → with-auth (no existing persistence DB): persistence tables are created fresh by create_all, no legacy rows 3. No-auth → with-auth (has existing persistence DB from #1930): NOT a supported upgrade path — "有 DB 到有 DB" schema evolution is out of scope; users wipe DB or run manual ALTER So the SQL orphan migration never has anything to do in the supported matrix. Delete the function, simplify _ensure_admin_user from a 3-step pipeline to a 2-step one (admin creation + LangGraph store orphan migration only). LangGraph store orphan migration stays: it serves the real "no-auth → with-auth" upgrade path where a user's existing LangGraph thread metadata has no owner_id field and needs to be stamped with the newly-created admin's id. Tests: 284 passed (auth + persistence + isolation) Lint: clean * security(auth): write initial admin password to 0600 file instead of logs CodeQL py/clear-text-logging-sensitive-data flagged 3 call sites that logged the auto-generated admin password to stdout via logger.info(). Production log aggregators (ELK/Splunk/etc) would have captured those cleartext secrets. Replace with a shared helper that writes to .deer-flow/admin_initial_credentials.txt with mode 0600, and log only the path. New file -------- - app/gateway/auth/credential_file.py: write_initial_credentials() helper. Takes email, password, and a "initial"/"reset" label. Creates .deer-flow/ if missing, writes a header comment plus the email+password, chmods 0o600, returns the absolute Path. Modified -------- - app/gateway/app.py: both _ensure_admin_user paths (fresh creation + needs_setup password reset) now write to file and log the path - app/gateway/auth/reset_admin.py: rewritten to use the shared ORM repo (SQLiteUserRepository with session_factory) and the credential_file helper. The previous implementation was broken after the earlier ORM refactor — it still imported _get_users_conn and constructed SQLiteUserRepository() without a session factory. No tests changed — the three password-log sites are all exercised via existing test_ensure_admin.py which checks that startup succeeds, not that a specific string appears in logs. CodeQL alerts 272, 283, 284: all resolved. * security(auth): strict JWT validation in middleware (fix junk cookie bypass) AUTH_TEST_PLAN test 7.5.8 expects junk cookies to be rejected with 401. The previous middleware behaviour was "presence-only": check that some access_token cookie exists, then pass through. In combination with my Task-12 decision to skip @require_auth decorators on routes, this created a gap where a request with any cookie-shaped string (e.g. access_token=not-a-jwt) would bypass authentication on routes that do not touch the repository (/api/models, /api/mcp/config, /api/memory, /api/skills, …). Fix: middleware now calls get_current_user_from_request() strictly and catches the resulting HTTPException to render a 401 with the proper fine-grained error code (token_invalid, token_expired, user_not_found, …). On success it stamps request.state.user and the contextvar so repository-layer owner filters work downstream. The 4 old "_with_cookie_passes" tests in test_auth_middleware.py were written for the presence-only behaviour; they asserted that a junk cookie would make the handler return 200. They are renamed to "_with_junk_cookie_rejected" and their assertions flipped to 401. The negative path (no cookie → 401 not_authenticated) is unchanged. Verified: no cookie → 401 not_authenticated junk cookie → 401 token_invalid (the fixed bug) expired cookie → 401 token_expired Tests: 284 passed (auth + persistence + isolation) Lint: clean * security(auth): wire @require_permission(owner_check=True) on isolation routes Apply the require_permission decorator to all 28 routes that take a {thread_id} path parameter. Combined with the strict middleware (previous commit), this gives the double-layer protection that AUTH_TEST_PLAN test 7.5.9 documents: Layer 1 (AuthMiddleware): cookie + JWT validation, rejects junk cookies and stamps request.state.user Layer 2 (@require_permission with owner_check=True): per-resource ownership verification via ThreadMetaStore.check_access — returns 404 if a different user owns the thread The decorator's owner_check branch is rewritten to use the SQL thread_meta_repo (the 2.0-rc persistence layer) instead of the LangGraph store path that PR #1728 used (_store_get / get_store in routers/threads.py). The inject_record convenience is dropped — no caller in 2.0 needs the LangGraph blob, and the SQL repo has a different shape. Routes decorated (28 total): - threads.py: delete, patch, get, get-state, post-state, post-history - thread_runs.py: post-runs, post-runs-stream, post-runs-wait, list_runs, get_run, cancel_run, join_run, stream_existing_run, list_thread_messages, list_run_messages, list_run_events, thread_token_usage - feedback.py: create, list, stats, delete - uploads.py: upload (added Request param), list, delete - artifacts.py: get_artifact - suggestions.py: generate (renamed body parameter to avoid conflict with FastAPI Request) Test fixes: - test_suggestions_router.py: bypass the decorator via __wrapped__ (the unit tests cover parsing logic, not auth — no point spinning up a thread_meta_repo just to test JSON unwrapping) - test_auth_middleware.py 4 fake-cookie tests: already updated in the previous commit (745bf432) Tests: 293 passed (auth + persistence + isolation + suggestions) Lint: clean * security(auth): defense-in-depth fixes from release validation pass Eight findings caught while running the AUTH_TEST_PLAN end-to-end against the deployed sg_dev stack. Each is a pre-condition for shipping release/2.0-rc that the previous PRs missed. Backend hardening - routers/auth.py: rate limiter X-Real-IP now requires AUTH_TRUSTED_PROXIES whitelist (CIDR/IP allowlist). Without nginx in front, the previous code honored arbitrary X-Real-IP, letting an attacker rotate the header to fully bypass the per-IP login lockout. - routers/auth.py: 36-entry common-password blocklist via Pydantic field_validator on RegisterRequest + ChangePasswordRequest. The shared _validate_strong_password helper keeps the constraint in one place. - routers/threads.py: ThreadCreateRequest + ThreadPatchRequest strip server-reserved metadata keys (owner_id, user_id) via Pydantic field_validator so a forged value can never round-trip back to other clients reading the same thread. The actual ownership invariant stays on the threads_meta row; this closes the metadata-blob echo gap. - authz.py + thread_meta/sql.py: require_permission gains a require_existing flag plumbed through check_access(require_existing=True). Destructive routes (DELETE/PATCH/state-update/runs/feedback) now treat a missing thread_meta row as 404 instead of "untracked legacy thread, allow", closing the cross-user delete-idempotence gap where any user could successfully DELETE another user's deleted thread. - repositories/sqlite.py + base.py: update_user raises UserNotFoundError on a vanished row instead of silently returning the input. Concurrent delete during password reset can no longer look like a successful update. - runtime/user_context.py: resolve_owner_id() coerces User.id (UUID) to str at the contextvar boundary so SQLAlchemy String(64) columns can bind it. The whole 2.0-rc isolation pipeline was previously broken end-to-end (POST /api/threads → 500 "type 'UUID' is not supported"). - persistence/engine.py: SQLAlchemy listener enables PRAGMA journal_mode=WAL, synchronous=NORMAL, foreign_keys=ON on every new SQLite connection. TC-UPG-06 in the test plan expects WAL; previous code shipped with the default 'delete' journal. - auth_middleware.py: stamp request.state.auth = AuthContext(...) so @require_permission's short-circuit fires; previously every isolation request did a duplicate JWT decode + users SELECT. Also unifies the 401 payload through AuthErrorResponse(...).model_dump(). - app.py: _ensure_admin_user restructure removes the noqa F821 scoping bug where 'password' was referenced outside the branch that defined it. New _announce_credentials helper absorbs the duplicate log block in the fresh-admin and reset-admin branches. * fix(frontend+nginx): rollout CSRF on every state-changing client path The frontend was 100% broken in gateway-pro mode for any user trying to open a specific chat thread. Three cumulative bugs each silently masked the next. LangGraph SDK CSRF gap (api-client.ts) - The Client constructor took only apiUrl, no defaultHeaders, no fetch interceptor. The SDK's internal fetch never sent X-CSRF-Token, so every state-changing /api/langgraph-compat/* call (runs/stream, threads/search, threads/{tid}/history, ...) hit CSRFMiddleware and got 403 before reaching the auth check. UI symptom: empty thread page with no error message; the SPA's hooks swallowed the rejection. - Fix: pass an onRequest hook that injects X-CSRF-Token from the csrf_token cookie per request. Reading the cookie per call (not at construction time) handles login / logout / password-change cookie rotation transparently. The SDK's prepareFetchOptions calls onRequest for both regular requests AND streaming/SSE/reconnect, so the same hook covers runs.stream and runs.joinStream. Raw fetch CSRF gap (7 files) - Audit: 11 frontend fetch sites, only 2 included CSRF (login/setup + account-settings change-password). The other 7 routed through raw fetch() with no header — suggestions, memory, agents, mcp, skills, uploads, and the local thread cleanup hook all 403'd silently. - Fix: enhance fetcher.ts:fetchWithAuth to auto-inject X-CSRF-Token on POST/PUT/DELETE/PATCH from a single shared readCsrfCookie() helper. Convert all 7 raw fetch() callers to fetchWithAuth so the contract is centrally enforced. api-client.ts and fetcher.ts share readCsrfCookie + STATE_CHANGING_METHODS to avoid drift. nginx routing + buffering (nginx.local.conf) - The auth feature shipped without updating the nginx config: per-API explicit location blocks but no /api/v1/auth/, /api/feedback, /api/runs. The frontend's client-side fetches to /api/v1/auth/login/local 404'd from the Next.js side because nginx routed /api/* to the frontend. - Fix: add catch-all `location /api/` that proxies to the gateway. nginx longest-prefix matching keeps the explicit blocks (/api/models, /api/threads regex, /api/langgraph/, ...) winning for their paths. - Fix: disable proxy_buffering + proxy_request_buffering for the frontend `location /` block. Without it, nginx tries to spool large Next.js chunks into /var/lib/nginx/proxy (root-owned) and fails with Permission denied → ERR_INCOMPLETE_CHUNKED_ENCODING → ChunkLoadError. * test(auth): release-validation test infra and new coverage Test fixtures and unit tests added during the validation pass. Router test helpers (NEW: tests/_router_auth_helpers.py) - make_authed_test_app(): builds a FastAPI test app with a stub middleware that stamps request.state.user + request.state.auth and a permissive thread_meta_repo mock. TestClient-based router tests (test_artifacts_router, test_threads_router) use it instead of bare FastAPI() so the new @require_permission(owner_check=True) decorators short-circuit cleanly. - call_unwrapped(): walks the __wrapped__ chain to invoke the underlying handler without going through the authz wrappers. Direct-call tests (test_uploads_router) use it. Typed with ParamSpec so the wrapped signature flows through. Backend test additions - test_auth.py: 7 tests for the new _get_client_ip trust model (no proxy / trusted proxy / untrusted peer / XFF rejection / invalid CIDR / no client). 5 tests for the password blocklist (literal, case-insensitive, strong password accepted, change-password binding, short-password length-check still fires before blocklist). test_update_user_raises_when_row_concurrently_deleted: closes a shipped-without-coverage gap on the new UserNotFoundError contract. - test_thread_meta_repo.py: 4 tests for check_access(require_existing=True) — strict missing-row denial, strict owner match, strict owner mismatch, strict null-owner still allowed (shared rows survive the tightening). - test_ensure_admin.py: 3 tests for _migrate_orphaned_threads / _iter_store_items pagination, covering the TC-UPG-02 upgrade story end-to-end via mock store. Closes the gap where the cursor pagination was untested even though the previous PR rewrote it. - test_threads_router.py: 5 tests for _strip_reserved_metadata (owner_id removal, user_id removal, safe-keys passthrough, empty input, both-stripped). - test_auth_type_system.py: replace "password123" fixtures with Tr0ub4dor3a / AnotherStr0ngPwd! so the new password blocklist doesn't reject the test data. * docs(auth): refresh TC-DOCKER-05 + document Docker validation gap - AUTH_TEST_PLAN.md TC-DOCKER-05: the previous expectation ("admin password visible in docker logs") was stale after the simplify pass that moved credentials to a 0600 file. The grep "Password:" check would have silently failed and given a false sense of coverage. New expectation matches the actual file-based path: 0600 file in DEER_FLOW_HOME, log shows the path (not the secret), reverse-grep asserts no leaked password in container logs. - NEW: docs/AUTH_TEST_DOCKER_GAP.md documents the only un-executed block in the test plan (TC-DOCKER-01..06). Reason: sg_dev validation host has no Docker daemon installed. The doc maps each Docker case to an already-validated bare-metal equivalent (TC-1.1, TC-REENT-01, TC-API-02 etc.) so the gap is auditable, and includes pre-flight reproduction steps for whoever has Docker available. --------- Co-authored-by: greatmengqi <chenmengqi.0376@bytedance.com> * feat: replace auto-admin creation with interactive setup flow On first boot, instead of auto-creating admin@deerflow.dev with a random password written to a credential file, DeerFlow now redirects to /setup where the user creates the admin account interactively. Backend: - Remove auto admin creation from _ensure_admin_user (now only runs orphan thread migration when an admin already exists) - Add POST /api/v1/auth/initialize endpoint (public, only callable when 0 users exist; auto-logs in after creation) - Add /api/v1/auth/initialize to public paths in auth_middleware.py and CSRF exempt paths in csrf_middleware.py - Update test_ensure_admin.py to match new behavior - Add test_initialize_admin.py with 8 tests for the new endpoint Frontend: - Add system_setup_required to AuthResult type - getServerSideUser() checks setup-status when unauthenticated - Auth layout allows system_setup_required (renders children) - Workspace layout redirects system_setup_required → /setup - Login page redirects to /setup when system not initialized - Setup page detects mode via isAuthenticated: unauth = create-admin form (calls /initialize), auth = change-password form (existing) Agent-Logs-Url: https://github.com/bytedance/deer-flow/sessions/9c2471c5-d6e9-4ada-9192-61b56007b8d7 Co-authored-by: foreleven <4785594+foreleven@users.noreply.github.com> * fix: add cleanup flags to useEffect async fetches in setup/login pages Agent-Logs-Url: https://github.com/bytedance/deer-flow/sessions/9c2471c5-d6e9-4ada-9192-61b56007b8d7 Co-authored-by: foreleven <4785594+foreleven@users.noreply.github.com> * fix: address reviewer feedback on /initialize endpoint security and robustness 1. Concurrency/register-blocking: switch setup-status and /initialize to check admin_count (via new count_admin_users()) instead of total user_count — /register can no longer block admin initialization 2. Dedicated error code: add SYSTEM_ALREADY_INITIALIZED to AuthErrorCode and use it in /initialize 409 responses; add to frontend types 3. Init token security: generate a one-time token at startup (logged to stdout) and require it in the /initialize request body — prevents an attacker from claiming admin on an exposed first-boot instance 4. Setup-status fetch timeout: apply SSR_AUTH_TIMEOUT_MS abort-controller pattern to the setup-status fetch in server.ts (same as /auth/me) Backend repo/provider: add count_admin_users() to base, SQLite, and LocalAuthProvider. Tests updated + new token-validation/register-blocking test cases added. Agent-Logs-Url: https://github.com/bytedance/deer-flow/sessions/b9f531fc-8ed3-41db-b416-237f243b45fd Co-authored-by: foreleven <4785594+foreleven@users.noreply.github.com> * fix: address code review nits — move secrets import, add INVALID_INIT_TOKEN error code, fix test assertions Agent-Logs-Url: https://github.com/bytedance/deer-flow/sessions/b9f531fc-8ed3-41db-b416-237f243b45fd Co-authored-by: foreleven <4785594+foreleven@users.noreply.github.com> * refactor: remove init_token generation and validation from admin setup flow * fix: re-apply init_token security for /initialize endpoint Re-adds the one-time init_token requirement to the /initialize endpoint, building on the human's UI improvements in 5eeeb09. This addresses the two remaining unresolved review threads: 1. Dedicated error code (SYSTEM_ALREADY_INITIALIZED + INVALID_INIT_TOKEN) 2. Init token security gate — requires the token logged at startup Changes: - errors.py: re-add INVALID_INIT_TOKEN error code - routers/auth.py: re-add `import secrets`, `init_token` field, token validation with secrets.compare_digest, and token consumption - app.py: re-add token generation/logging and app.state.init_token = None - setup/page.tsx: re-add initToken state + input field (human's UI kept) - types.ts: re-add invalid_init_token error code - test_initialize_admin.py: restore full token test coverage - test_ensure_admin.py: restore init_token assertions Agent-Logs-Url: https://github.com/bytedance/deer-flow/sessions/646fb5c0-ec09-41aa-9fe9-e6f7c32364e8 Co-authored-by: foreleven <4785594+foreleven@users.noreply.github.com> * fix: make init_token optional (403 not 422 on missing), don't consume token on error paths Agent-Logs-Url: https://github.com/bytedance/deer-flow/sessions/646fb5c0-ec09-41aa-9fe9-e6f7c32364e8 Co-authored-by: foreleven <4785594+foreleven@users.noreply.github.com> * refactor: remove redundant skill-related functions and documentation --------- Co-authored-by: rayhpeng <rayhpeng@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> Co-authored-by: DanielWalnut <45447813+hetaoBackend@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> Co-authored-by: JilongSun <965640067@qq.com> Co-authored-by: jie <49781832+stan-fu@users.noreply.github.com> Co-authored-by: cooper <cooperfu@tencent.com> Co-authored-by: yangzheli <43645580+yangzheli@users.noreply.github.com> Co-authored-by: greatmengqi <chenmengqi.0376@gmail.com> Co-authored-by: greatmengqi <chenmengqi.0376@bytedance.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: foreleven <4785594+foreleven@users.noreply.github.com> Co-authored-by: jiangfeng.11 <jiangfeng.11@bytedance.com> |
||
|
greatmengqi
|
94eee95fe0 |
feat(auth): release-validation pass for 2.0-rc — 12 blockers + simplify follow-ups (#2008)
* feat(auth): introduce backend auth module Port RFC-001 authentication core from PR #1728: - JWT token handling (create_access_token, decode_token, TokenPayload) - Password hashing (bcrypt) with verify_password - SQLite UserRepository with base interface - Provider Factory pattern (LocalAuthProvider) - CLI reset_admin tool - Auth-specific errors (AuthErrorCode, TokenError, AuthErrorResponse) Deps: - bcrypt>=4.0.0 - pyjwt>=2.9.0 - email-validator>=2.0.0 - backend/uv.toml pins public PyPI index Tests: 12 pure unit tests (test_auth_config.py, test_auth_errors.py). Scope note: authz.py, test_auth.py, and test_auth_type_system.py are deferred to commit 2 because they depend on middleware and deps wiring that is not yet in place. Commit 1 stays "pure new files only" as the spec mandates. * feat(auth): wire auth end-to-end (middleware + frontend replacement) Backend: - Port auth_middleware, csrf_middleware, langgraph_auth, routers/auth - Port authz decorator (owner_filter_key defaults to 'owner_id') - Merge app.py: register AuthMiddleware + CSRFMiddleware + CORS, add _ensure_admin_user lifespan hook, _migrate_orphaned_threads helper, register auth router - Merge deps.py: add get_local_provider, get_current_user_from_request, get_optional_user_from_request; keep get_current_user as thin str|None adapter for feedback router - langgraph.json: add auth path pointing to langgraph_auth.py:auth - Rename metadata['user_id'] -> metadata['owner_id'] in langgraph_auth (both metadata write and LangGraph filter dict) + test fixtures Frontend: - Delete better-auth library and api catch-all route - Remove better-auth npm dependency and env vars (BETTER_AUTH_SECRET, BETTER_AUTH_GITHUB_*) from env.js - Port frontend/src/core/auth/* (AuthProvider, gateway-config, proxy-policy, server-side getServerSideUser, types) - Port frontend/src/core/api/fetcher.ts - Port (auth)/layout, (auth)/login, (auth)/setup pages - Rewrite workspace/layout.tsx as server component that calls getServerSideUser and wraps in AuthProvider - Port workspace/workspace-content.tsx for the client-side sidebar logic Tests: - Port 5 auth test files (test_auth, test_auth_middleware, test_auth_type_system, test_ensure_admin, test_langgraph_auth) - 176 auth tests PASS After this commit: login/logout/registration flow works, but persistence layer does not yet filter by owner_id. Commit 4 closes that gap. * feat(auth): account settings page + i18n - Port account-settings-page.tsx (change password, change email, logout) - Wire into settings-dialog.tsx as new "account" section with UserIcon, rendered first in the section list - Add i18n keys: - en-US/zh-CN: settings.sections.account ("Account" / "账号") - en-US/zh-CN: button.logout ("Log out" / "退出登录") - types.ts: matching type declarations * feat(auth): enforce owner_id across 2.0-rc persistence layer Add request-scoped contextvar-based owner filtering to threads_meta, runs, run_events, and feedback repositories. Router code is unchanged — isolation is enforced at the storage layer so that any caller that forgets to pass owner_id still gets filtered results, and new routes cannot accidentally leak data. Core infrastructure ------------------- - deerflow/runtime/user_context.py (new): - ContextVar[CurrentUser | None] with default None - runtime_checkable CurrentUser Protocol (structural subtype with .id) - set/reset/get/require helpers - AUTO sentinel + resolve_owner_id(value, method_name) for sentinel three-state resolution: AUTO reads contextvar, explicit str overrides, explicit None bypasses the filter (for migration/CLI) Repository changes ------------------ - ThreadMetaRepository: create/get/search/update_*/delete gain owner_id=AUTO kwarg; read paths filter by owner, writes stamp it, mutations check ownership before applying - RunRepository: put/get/list_by_thread/delete gain owner_id=AUTO kwarg - FeedbackRepository: create/get/list_by_run/list_by_thread/delete gain owner_id=AUTO kwarg - DbRunEventStore: list_messages/list_events/list_messages_by_run/ count_messages/delete_by_thread/delete_by_run gain owner_id=AUTO kwarg. Write paths (put/put_batch) read contextvar softly: when a request-scoped user is available, owner_id is stamped; background worker writes without a user context pass None which is valid (orphan row to be bound by migration) Schema ------ - persistence/models/run_event.py: RunEventRow.owner_id = Mapped[ str | None] = mapped_column(String(64), nullable=True, index=True) - No alembic migration needed: 2.0 ships fresh, Base.metadata.create_all picks up the new column automatically Middleware ---------- - auth_middleware.py: after cookie check, call get_optional_user_from_ request to load the real User, stamp it into request.state.user AND the contextvar via set_current_user, reset in a try/finally. Public paths and unauthenticated requests continue without contextvar, and @require_auth handles the strict 401 path Test infrastructure ------------------- - tests/conftest.py: @pytest.fixture(autouse=True) _auto_user_context sets a default SimpleNamespace(id="test-user-autouse") on every test unless marked @pytest.mark.no_auto_user. Keeps existing 20+ persistence tests passing without modification - pyproject.toml [tool.pytest.ini_options]: register no_auto_user marker so pytest does not emit warnings for opt-out tests - tests/test_user_context.py: 6 tests covering three-state semantics, Protocol duck typing, and require/optional APIs - tests/test_thread_meta_repo.py: one test updated to pass owner_id= None explicitly where it was previously relying on the old default Test results ------------ - test_user_context.py: 6 passed - test_auth*.py + test_langgraph_auth.py + test_ensure_admin.py: 127 - test_run_event_store / test_run_repository / test_thread_meta_repo / test_feedback: 92 passed - Full backend suite: 1905 passed, 2 failed (both @requires_llm flaky integration tests unrelated to auth), 1 skipped * feat(auth): extend orphan migration to 2.0-rc persistence tables _ensure_admin_user now runs a three-step pipeline on every boot: Step 1 (fatal): admin user exists / is created / password is reset Step 2 (non-fatal): LangGraph store orphan threads → admin Step 3 (non-fatal): SQL persistence tables → admin - threads_meta - runs - run_events - feedback Each step is idempotent. The fatal/non-fatal split mirrors PR #1728's original philosophy: admin creation failure blocks startup (the system is unusable without an admin), whereas migration failures log a warning and let the service proceed (a partial migration is recoverable; a missing admin is not). Key helpers ----------- - _iter_store_items(store, namespace, *, page_size=500): async generator that cursor-paginates across LangGraph store pages. Fixes PR #1728's hardcoded limit=1000 bug that would silently lose orphans beyond the first page. - _migrate_orphaned_threads(store, admin_user_id): Rewritten to use _iter_store_items. Returns the migrated count so the caller can log it; raises only on unhandled exceptions. - _migrate_orphan_sql_tables(admin_user_id): Imports the 4 ORM models lazily, grabs the shared session factory, runs one UPDATE per table in a single transaction, commits once. No-op when no persistence backend is configured (in-memory dev). Tests: test_ensure_admin.py (8 passed) * test(auth): port AUTH test plan docs + lint/format pass - Port backend/docs/AUTH_TEST_PLAN.md and AUTH_UPGRADE.md from PR #1728 - Rename metadata.user_id → metadata.owner_id in AUTH_TEST_PLAN.md (4 occurrences from the original PR doc) - ruff auto-fix UP037 in sentinel type annotations: drop quotes around "str | None | _AutoSentinel" now that from __future__ import annotations makes them implicit string forms - ruff format: 2 files (app/gateway/app.py, runtime/user_context.py) Note on test coverage additions: - conftest.py autouse fixture was already added in commit 4 (had to be co-located with the repository changes to keep pre-existing persistence tests passing) - cross-user isolation E2E tests (test_owner_isolation.py) deferred — enforcement is already proven by the 98-test repository suite via the autouse fixture + explicit _AUTO sentinel exercises - New test cases (TC-API-17..20, TC-ATK-13, TC-MIG-01..07) listed in AUTH_TEST_PLAN.md are deferred to a follow-up PR — they are manual-QA test cases rather than pytest code, and the spec-level coverage is already met by test_user_context.py + the 98-test repository suite. Final test results: - Auth suite (test_auth*, test_langgraph_auth, test_ensure_admin, test_user_context): 186 passed - Persistence suite (test_run_event_store, test_run_repository, test_thread_meta_repo, test_feedback): 98 passed - Lint: ruff check + ruff format both clean * test(auth): add cross-user isolation test suite 10 tests exercising the storage-layer owner filter by manually switching the user_context contextvar between two users. Verifies the safety invariant: After a repository write with owner_id=A, a subsequent read with owner_id=B must not return the row, and vice versa. Covers all 4 tables that own user-scoped data: TC-API-17 threads_meta — read, search, update, delete cross-user TC-API-18 runs — get, list_by_thread, delete cross-user TC-API-19 run_events — list_messages, list_events, count_messages, delete_by_thread (CRITICAL: raw conversation content leak vector) TC-API-20 feedback — get, list_by_run, delete cross-user Plus two meta-tests verifying the sentinel pattern itself: - AUTO + unset contextvar raises RuntimeError - explicit owner_id=None bypasses the filter (migration escape hatch) Architecture note ----------------- These tests bypass the HTTP layer by design. The full chain (cookie → middleware → contextvar → repository) is covered piecewise: - test_auth_middleware.py: middleware sets contextvar from cookies - test_owner_isolation.py: repositories enforce isolation when contextvar is set to different users Together they prove the end-to-end safety property without the ceremony of spinning up a full TestClient + in-memory DB for every router endpoint. Tests pass: 231 (full auth + persistence + isolation suite) Lint: clean * refactor(auth): migrate user repository to SQLAlchemy ORM Move the users table into the shared persistence engine so auth matches the pattern of threads_meta, runs, run_events, and feedback — one engine, one session factory, one schema init codepath. New files --------- - persistence/user/__init__.py, persistence/user/model.py: UserRow ORM class with partial unique index on (oauth_provider, oauth_id) - Registered in persistence/models/__init__.py so Base.metadata.create_all() picks it up Modified -------- - auth/repositories/sqlite.py: rewritten as async SQLAlchemy, identical constructor pattern to the other four repositories (def __init__(self, session_factory) + self._sf = session_factory) - auth/config.py: drop users_db_path field — storage is configured through config.database like every other table - deps.py/get_local_provider: construct SQLiteUserRepository with the shared session factory, fail fast if engine is not initialised - tests/test_auth.py: rewrite test_sqlite_round_trip_new_fields to use the shared engine (init_engine + close_engine in a tempdir) - tests/test_auth_type_system.py: add per-test autouse fixture that spins up a scratch engine and resets deps._cached_* singletons * refactor(auth): remove SQL orphan migration (unused in supported scenarios) The _migrate_orphan_sql_tables helper existed to bind NULL owner_id rows in threads_meta, runs, run_events, and feedback to the admin on first boot. But in every supported upgrade path, it's a no-op: 1. Fresh install: create_all builds fresh tables, no legacy rows 2. No-auth → with-auth (no existing persistence DB): persistence tables are created fresh by create_all, no legacy rows 3. No-auth → with-auth (has existing persistence DB from #1930): NOT a supported upgrade path — "有 DB 到有 DB" schema evolution is out of scope; users wipe DB or run manual ALTER So the SQL orphan migration never has anything to do in the supported matrix. Delete the function, simplify _ensure_admin_user from a 3-step pipeline to a 2-step one (admin creation + LangGraph store orphan migration only). LangGraph store orphan migration stays: it serves the real "no-auth → with-auth" upgrade path where a user's existing LangGraph thread metadata has no owner_id field and needs to be stamped with the newly-created admin's id. Tests: 284 passed (auth + persistence + isolation) Lint: clean * security(auth): write initial admin password to 0600 file instead of logs CodeQL py/clear-text-logging-sensitive-data flagged 3 call sites that logged the auto-generated admin password to stdout via logger.info(). Production log aggregators (ELK/Splunk/etc) would have captured those cleartext secrets. Replace with a shared helper that writes to .deer-flow/admin_initial_credentials.txt with mode 0600, and log only the path. New file -------- - app/gateway/auth/credential_file.py: write_initial_credentials() helper. Takes email, password, and a "initial"/"reset" label. Creates .deer-flow/ if missing, writes a header comment plus the email+password, chmods 0o600, returns the absolute Path. Modified -------- - app/gateway/app.py: both _ensure_admin_user paths (fresh creation + needs_setup password reset) now write to file and log the path - app/gateway/auth/reset_admin.py: rewritten to use the shared ORM repo (SQLiteUserRepository with session_factory) and the credential_file helper. The previous implementation was broken after the earlier ORM refactor — it still imported _get_users_conn and constructed SQLiteUserRepository() without a session factory. No tests changed — the three password-log sites are all exercised via existing test_ensure_admin.py which checks that startup succeeds, not that a specific string appears in logs. CodeQL alerts 272, 283, 284: all resolved. * security(auth): strict JWT validation in middleware (fix junk cookie bypass) AUTH_TEST_PLAN test 7.5.8 expects junk cookies to be rejected with 401. The previous middleware behaviour was "presence-only": check that some access_token cookie exists, then pass through. In combination with my Task-12 decision to skip @require_auth decorators on routes, this created a gap where a request with any cookie-shaped string (e.g. access_token=not-a-jwt) would bypass authentication on routes that do not touch the repository (/api/models, /api/mcp/config, /api/memory, /api/skills, …). Fix: middleware now calls get_current_user_from_request() strictly and catches the resulting HTTPException to render a 401 with the proper fine-grained error code (token_invalid, token_expired, user_not_found, …). On success it stamps request.state.user and the contextvar so repository-layer owner filters work downstream. The 4 old "_with_cookie_passes" tests in test_auth_middleware.py were written for the presence-only behaviour; they asserted that a junk cookie would make the handler return 200. They are renamed to "_with_junk_cookie_rejected" and their assertions flipped to 401. The negative path (no cookie → 401 not_authenticated) is unchanged. Verified: no cookie → 401 not_authenticated junk cookie → 401 token_invalid (the fixed bug) expired cookie → 401 token_expired Tests: 284 passed (auth + persistence + isolation) Lint: clean * security(auth): wire @require_permission(owner_check=True) on isolation routes Apply the require_permission decorator to all 28 routes that take a {thread_id} path parameter. Combined with the strict middleware (previous commit), this gives the double-layer protection that AUTH_TEST_PLAN test 7.5.9 documents: Layer 1 (AuthMiddleware): cookie + JWT validation, rejects junk cookies and stamps request.state.user Layer 2 (@require_permission with owner_check=True): per-resource ownership verification via ThreadMetaStore.check_access — returns 404 if a different user owns the thread The decorator's owner_check branch is rewritten to use the SQL thread_meta_repo (the 2.0-rc persistence layer) instead of the LangGraph store path that PR #1728 used (_store_get / get_store in routers/threads.py). The inject_record convenience is dropped — no caller in 2.0 needs the LangGraph blob, and the SQL repo has a different shape. Routes decorated (28 total): - threads.py: delete, patch, get, get-state, post-state, post-history - thread_runs.py: post-runs, post-runs-stream, post-runs-wait, list_runs, get_run, cancel_run, join_run, stream_existing_run, list_thread_messages, list_run_messages, list_run_events, thread_token_usage - feedback.py: create, list, stats, delete - uploads.py: upload (added Request param), list, delete - artifacts.py: get_artifact - suggestions.py: generate (renamed body parameter to avoid conflict with FastAPI Request) Test fixes: - test_suggestions_router.py: bypass the decorator via __wrapped__ (the unit tests cover parsing logic, not auth — no point spinning up a thread_meta_repo just to test JSON unwrapping) - test_auth_middleware.py 4 fake-cookie tests: already updated in the previous commit (745bf432) Tests: 293 passed (auth + persistence + isolation + suggestions) Lint: clean * security(auth): defense-in-depth fixes from release validation pass Eight findings caught while running the AUTH_TEST_PLAN end-to-end against the deployed sg_dev stack. Each is a pre-condition for shipping release/2.0-rc that the previous PRs missed. Backend hardening - routers/auth.py: rate limiter X-Real-IP now requires AUTH_TRUSTED_PROXIES whitelist (CIDR/IP allowlist). Without nginx in front, the previous code honored arbitrary X-Real-IP, letting an attacker rotate the header to fully bypass the per-IP login lockout. - routers/auth.py: 36-entry common-password blocklist via Pydantic field_validator on RegisterRequest + ChangePasswordRequest. The shared _validate_strong_password helper keeps the constraint in one place. - routers/threads.py: ThreadCreateRequest + ThreadPatchRequest strip server-reserved metadata keys (owner_id, user_id) via Pydantic field_validator so a forged value can never round-trip back to other clients reading the same thread. The actual ownership invariant stays on the threads_meta row; this closes the metadata-blob echo gap. - authz.py + thread_meta/sql.py: require_permission gains a require_existing flag plumbed through check_access(require_existing=True). Destructive routes (DELETE/PATCH/state-update/runs/feedback) now treat a missing thread_meta row as 404 instead of "untracked legacy thread, allow", closing the cross-user delete-idempotence gap where any user could successfully DELETE another user's deleted thread. - repositories/sqlite.py + base.py: update_user raises UserNotFoundError on a vanished row instead of silently returning the input. Concurrent delete during password reset can no longer look like a successful update. - runtime/user_context.py: resolve_owner_id() coerces User.id (UUID) to str at the contextvar boundary so SQLAlchemy String(64) columns can bind it. The whole 2.0-rc isolation pipeline was previously broken end-to-end (POST /api/threads → 500 "type 'UUID' is not supported"). - persistence/engine.py: SQLAlchemy listener enables PRAGMA journal_mode=WAL, synchronous=NORMAL, foreign_keys=ON on every new SQLite connection. TC-UPG-06 in the test plan expects WAL; previous code shipped with the default 'delete' journal. - auth_middleware.py: stamp request.state.auth = AuthContext(...) so @require_permission's short-circuit fires; previously every isolation request did a duplicate JWT decode + users SELECT. Also unifies the 401 payload through AuthErrorResponse(...).model_dump(). - app.py: _ensure_admin_user restructure removes the noqa F821 scoping bug where 'password' was referenced outside the branch that defined it. New _announce_credentials helper absorbs the duplicate log block in the fresh-admin and reset-admin branches. * fix(frontend+nginx): rollout CSRF on every state-changing client path The frontend was 100% broken in gateway-pro mode for any user trying to open a specific chat thread. Three cumulative bugs each silently masked the next. LangGraph SDK CSRF gap (api-client.ts) - The Client constructor took only apiUrl, no defaultHeaders, no fetch interceptor. The SDK's internal fetch never sent X-CSRF-Token, so every state-changing /api/langgraph-compat/* call (runs/stream, threads/search, threads/{tid}/history, ...) hit CSRFMiddleware and got 403 before reaching the auth check. UI symptom: empty thread page with no error message; the SPA's hooks swallowed the rejection. - Fix: pass an onRequest hook that injects X-CSRF-Token from the csrf_token cookie per request. Reading the cookie per call (not at construction time) handles login / logout / password-change cookie rotation transparently. The SDK's prepareFetchOptions calls onRequest for both regular requests AND streaming/SSE/reconnect, so the same hook covers runs.stream and runs.joinStream. Raw fetch CSRF gap (7 files) - Audit: 11 frontend fetch sites, only 2 included CSRF (login/setup + account-settings change-password). The other 7 routed through raw fetch() with no header — suggestions, memory, agents, mcp, skills, uploads, and the local thread cleanup hook all 403'd silently. - Fix: enhance fetcher.ts:fetchWithAuth to auto-inject X-CSRF-Token on POST/PUT/DELETE/PATCH from a single shared readCsrfCookie() helper. Convert all 7 raw fetch() callers to fetchWithAuth so the contract is centrally enforced. api-client.ts and fetcher.ts share readCsrfCookie + STATE_CHANGING_METHODS to avoid drift. nginx routing + buffering (nginx.local.conf) - The auth feature shipped without updating the nginx config: per-API explicit location blocks but no /api/v1/auth/, /api/feedback, /api/runs. The frontend's client-side fetches to /api/v1/auth/login/local 404'd from the Next.js side because nginx routed /api/* to the frontend. - Fix: add catch-all `location /api/` that proxies to the gateway. nginx longest-prefix matching keeps the explicit blocks (/api/models, /api/threads regex, /api/langgraph/, ...) winning for their paths. - Fix: disable proxy_buffering + proxy_request_buffering for the frontend `location /` block. Without it, nginx tries to spool large Next.js chunks into /var/lib/nginx/proxy (root-owned) and fails with Permission denied → ERR_INCOMPLETE_CHUNKED_ENCODING → ChunkLoadError. * test(auth): release-validation test infra and new coverage Test fixtures and unit tests added during the validation pass. Router test helpers (NEW: tests/_router_auth_helpers.py) - make_authed_test_app(): builds a FastAPI test app with a stub middleware that stamps request.state.user + request.state.auth and a permissive thread_meta_repo mock. TestClient-based router tests (test_artifacts_router, test_threads_router) use it instead of bare FastAPI() so the new @require_permission(owner_check=True) decorators short-circuit cleanly. - call_unwrapped(): walks the __wrapped__ chain to invoke the underlying handler without going through the authz wrappers. Direct-call tests (test_uploads_router) use it. Typed with ParamSpec so the wrapped signature flows through. Backend test additions - test_auth.py: 7 tests for the new _get_client_ip trust model (no proxy / trusted proxy / untrusted peer / XFF rejection / invalid CIDR / no client). 5 tests for the password blocklist (literal, case-insensitive, strong password accepted, change-password binding, short-password length-check still fires before blocklist). test_update_user_raises_when_row_concurrently_deleted: closes a shipped-without-coverage gap on the new UserNotFoundError contract. - test_thread_meta_repo.py: 4 tests for check_access(require_existing=True) — strict missing-row denial, strict owner match, strict owner mismatch, strict null-owner still allowed (shared rows survive the tightening). - test_ensure_admin.py: 3 tests for _migrate_orphaned_threads / _iter_store_items pagination, covering the TC-UPG-02 upgrade story end-to-end via mock store. Closes the gap where the cursor pagination was untested even though the previous PR rewrote it. - test_threads_router.py: 5 tests for _strip_reserved_metadata (owner_id removal, user_id removal, safe-keys passthrough, empty input, both-stripped). - test_auth_type_system.py: replace "password123" fixtures with Tr0ub4dor3a / AnotherStr0ngPwd! so the new password blocklist doesn't reject the test data. * docs(auth): refresh TC-DOCKER-05 + document Docker validation gap - AUTH_TEST_PLAN.md TC-DOCKER-05: the previous expectation ("admin password visible in docker logs") was stale after the simplify pass that moved credentials to a 0600 file. The grep "Password:" check would have silently failed and given a false sense of coverage. New expectation matches the actual file-based path: 0600 file in DEER_FLOW_HOME, log shows the path (not the secret), reverse-grep asserts no leaked password in container logs. - NEW: docs/AUTH_TEST_DOCKER_GAP.md documents the only un-executed block in the test plan (TC-DOCKER-01..06). Reason: sg_dev validation host has no Docker daemon installed. The doc maps each Docker case to an already-validated bare-metal equivalent (TC-1.1, TC-REENT-01, TC-API-02 etc.) so the gap is auditable, and includes pre-flight reproduction steps for whoever has Docker available. --------- Co-authored-by: greatmengqi <chenmengqi.0376@bytedance.com> |