fix(api): authenticate /product, bind user_id to principal, clamp doc_path - #2227
Open
Evenss wants to merge 5 commits into
Open
fix(api): authenticate /product, bind user_id to principal, clamp doc_path#2227Evenss wants to merge 5 commits into
Evenss wants to merge 5 commits into
Conversation
Add optional OceanBase / seekdb backends reusing the existing BaseVecDB and BaseGraphDB contracts, without changing any default behavior. - vec_dbs/oceanbase.py: OceanBaseVecDB on top of pyseekdb's Collection API, serving General Memory; require a positive vector_dimension in config. - graph_dbs/oceanbase.py: OceanBaseGraphDB ported from the postgres backend (nodes + edges + JSON + VECTOR) over the MySQL-compatible protocol, with a thread-safe connection pool, atomic multi-step deletes, and identifier whitelisting (table_prefix / search_filter keys). - Register "oceanbase" / "seekdb" aliases in the vec/graph factories and config factories; add GraphDBError; declare the optional "ob-mem" extra. - Add contract tests for both providers.
…ection handling - Updated pyseekdb version constraints in pyproject.toml to restrict to <1.5.0. - Increased default embedding dimension in APIConfig from 768 to 1024. - Improved connection handling in OceanBaseGraphDB and OceanBaseVecDB to ensure better resource management and error handling. - Added validation for table prefix length in OceanBaseGraphDB to prevent identifier overflow. - Enhanced logging for empty password configurations in OceanBaseGraphDB. - Updated tests to reflect changes in search behavior and connection management.
…nd return field projection
… isolation - Default AUTH_ENABLED to true and add MASTER_KEY_HASH env configuration. - Require API key on all /product routes and bind user-scoped parameters to the authenticated principal. - Replace IP-based internal request trust with a constant-time X-Internal-Service secret check. - Confine document enumeration to MEMOS_DOC_ROOT / FILE_LOCAL_PATH to prevent path traversal. - Add security regression tests for auth, user scoping, and document path confinement.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes the four issues reported in #2202, staying within the scope of the issue's Suggested change.
/productis unauthenticated — the router now declaresdependencies=[Depends(verify_api_key)], so every current and future endpoint under it requires a key.AUTH_ENABLEDnow defaults totrue. The two health endpoints parse the same variable independently, so their defaults were flipped too; a startup warning is logged when auth is on butMASTER_KEY_HASHis unset.user_idis client-supplied (IDOR) — newresolve_authorized_user_id()binds a non-privileged principal to its own user ID (an API key'suser_nameis the Product API user ID it may act as). Applied to the 17/productendpoints that accept a caller-supplied user ID; the remaining endpoints have no user ID to bind, so they are authenticated but not further restricted. Master key / internal-service / admin principals keep acting for any user. With auth disabled, the identity is now pinned toMOS_USER_IDinstead of the client-suppliedX-User-Name.doc_pathreads arbitrary files, andNone == Noneinternal bypass —_get_all_documentsconfinesdoc_pathtoMEMOS_DOC_ROOT(falling back toFILE_LOCAL_PATH), re-resolving each matched file so symlinks cannot escape; local library usage with neither variable set is unchanged.is_internal_requestno longer trusts the source IP and requires both the secret and the header to be non-empty, compared withhmac.compare_digest.api_analyzer.pyis a passive adaptation — it calls the endpoint functions in-process and must now pass an explicit internalauth.Related Issue (Required): Fixes #2202
Breaking changes
AUTH_ENABLEDwill start rejecting unauthenticated requests.MASTER_KEY_HASHwas added to bothdocker/.env.examplefiles; noteserver_api.pydoes not mountadmin_router, so the master key is the only bootstrap path on the base server.expires_atwill now always fail to authenticate.lookup_api_keycompares adatetimeagainsttime.time(), and theTypeErroris swallowed into areturn None. It is pre-existing and fails closed, so it is left for a separate fix, but default-on auth makes it reachable.Type of change
How Has This Been Tested?
9 regression cases, one per issue item. The fixtures in
test_server_router.pyandtest_cube_endpoints.pynow supply an admin key, since their pre-existing cases would otherwise all return 401.test_mos_init_invalid_userand the fourtest_mcp_serve.pyimport errors fail identically at the base commit and are unrelated.Checklist