Conversation
- AppBuilder honors JVSPATIAL_DOCS_DISABLED (1/true/yes/on): unpublishes /docs, /redoc, /openapi.json, /docs/oauth2-redirect (404, no spec leak). - SecurityHeadersMiddleware emits a relaxed CSP on docs paths so Swagger UI / ReDoc render via cdn.jsdelivr.net. App routes keep strict.
…class name (optional) using __entity_name__ = 'Name'
Renames the legacy code-pattern cookbook (formerly SPEC.md, 5284 lines labeled "Language Model Coding Guide") to LLM-CODING-GUIDE.md, since that is its actual purpose. Adds a new SPEC.md that captures the technical contract jvspatial guarantees: identity model, async contract, persistence layer, walker semantics, GraphContext, API surface, auth, configuration, serverless constraints, storage, caching, observability, security boundaries, extension points, error taxonomy, and stability tiers. Every claim cites a source-of-truth file:line. Companions PRD.md, ROADMAP.md, CLAUDE.md, and the docs index land in follow-up commits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds PRD.md to complement SPEC.md (technical contract) with the product layer: positioning, target users (described by role, not by named product), core value propositions, explicit non-goals, constraints, success criteria, stability-tier ratification, and decision boundaries for evaluating proposed changes. Establishes the documentation triangle: PRD answers "why", SPEC answers "what", and the docs/md/ tree answers "how". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Updates docs/md/README.md to: - Reflect current 0.0.8+ state (was stale at 0.0.6 / 2025-03-16) - Index the 8 docs that existed but were not listed (benchmarks, observability, production-deployment, rate-limiting, serverless-mode, stability, security-review, security-operational-notes, api-keys) - Cross-link to PRD/SPEC/CLAUDE/ROADMAP at repo root - Establish authoritative source order when docs disagree Adds ROADMAP.md capturing current focus areas (security hardening, performance, observability, stability contract), known gaps with file:line citations (migration story, multi-DB transactions, per-worker state, count consistency, walker trail in serverless, quantitative budgets, Python 3.8 classifier, experimental JsonDBTransaction status), and out-of-scope reaffirmations. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CLAUDE.md is the canonical agent maintenance guide loaded automatically by Claude Code. It captures: - What jvspatial is, in one paragraph - Order of trust for resolving context (PRD -> SPEC -> docs -> code) - Nine non-negotiable invariants with file:line citations - Common gotchas table mapping symptom -> cause -> fix - Dev loop commands - Safe-change playbooks for features, fixes, security paths, serverless paths, and new database backends - Explicit boundaries (sync APIs, CORS, secret comparison, internal imports, etc.) - Repo geography overview AGENTS.md is a thin shim for the multi-agent convention (Codex, Cursor, Aider, Continue, OpenHands). It points at CLAUDE.md as canonical and reserves space only for editor-specific quirks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds README.md to each top-level subpackage that lacked one: core, api, db, storage, cache, serverless, observability, logging, runtime. Each README is navigational, not encyclopedic. Sections cover: purpose, layout, public API (with what each name does), invariants specific to the subpackage, common modification patterns, related docs in docs/md/, and stability tier. When an agent opens a subdirectory, the README is the first hop and tells it what lives here and where to look next. File paths and citations cross-reference SPEC.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
First retrospective audit of jvspatial against the contracts now codified in SPEC/PRD/CLAUDE. Five parallel reviewers covered: async correctness, security boundaries, database adapter parity, walker/identity invariants, and serverless/config/stability. Headline findings (16 CRIT, ~42 HIGH, ~58 MED, ~34 LOW): 1. __entity_name__ override is silently rewritten by GraphContext on every save -- the 0.0.8 headline feature is data-corrupting for classes that use it (5 CRIT sites in walker.py + context.py). 2. Walker protection partially fictional: max_trail_length unwired, queue insert paths bypass max_size, resume() resets timer + counters, ProtectionViolation swallowed into report rather than raised as InfiniteLoopError/WalkerTimeoutError. 3. Five async-contract CRITs: async def __init__ on webhook walker, unawaited delete_file in storage service, two Path.write_text inside async graph exporters, unawaited webhook endpoint_func. 4. LocalFileInterface versioning methods (create_version, etc.) bypass PathSanitizer -- user-controlled file_path escapes the storage root. 5. Webhook HMAC verifier compares signature against a 7-char-too- short slice -- always returns False. Webhook signature auth is currently broken-by-default. 6. SHA-256 fallback in refresh-token/password-reset compares with `==` not hmac.compare_digest (CLAUDE.md non-negotiable). 7. DynamoDB throttle retry covers only save/get/delete; find/count/ batch ops surface ProvisionedThroughputExceededException directly to callers despite the documented contract. 8. SPEC §10.2 promises env allowlist enforcement; env_adapter only reads enumerated keys, never scans for stray JVSPATIAL_* keys to reject typos. Plus three divergent parse_bool implementations across env.py / env_adapter.py / runtime/serverless.py producing different truth values for the same input. The audit is non-destructive: zero code changes. Findings are the backlog for the next milestone. AUDIT.md includes a four-wave remediation sequence ordered by blast radius x cost. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ecurity, dynamodb Addresses 16 CRIT-class findings from AUDIT.md §1-§5 (Wave 1 of the post-audit hardening plan). All changes are surgical and SPEC-aligned; no API removals. --- Group A: identity model (audit §1) --- The 0.0.8 ``__entity_name__`` override (commit a3964ab) was ignored in the walker constructor, in GraphContext save/edge-query paths, and in several Node lookup helpers. The result: any subclass using the override had its IDs silently rewritten back to ``cls.__name__`` on every save (data corruption), and edge/neighbor queries by override class returned nothing. * core/entities/walker.py — declare ``__entity_name__`` and add ``_entity_name()`` classmethod parallel to Object's; thread through ID generation and persisted ``entity`` (§1.1, §1.2, §1.9). * core/context.py — save_object ID validation + regeneration uses ``_entity_name()`` (§1.3, §1.4). find_edges_between uses ``_entity_name()`` (§1.5). * core/entities/node.py — _node_query uses persisted ``entity`` field (not the wrong ``name`` key) and honors ``_entity_name()`` (§1.6). count_neighbors fast path resolves via ``_entity_name()`` (§1.7). _matches_node_filter compares against ``_entity_name()`` (§1.8). --- Group B: security CRITs (audit §4) --- * api/auth/service.py — replace ``==`` with ``hmac.compare_digest`` for SHA-256 refresh-token / password-reset-token fallback verification (§4.1 / SPEC §15.2). CLAUDE.md §2 non-negotiable. * storage/interfaces/local.py — add ``_sanitized_version_base`` helper and route every versioning method (create_version, get_version, list_versions, delete_version, get_latest_version) through it. Previously these computed ``self.root_dir / f"{file_path}.versions"`` without sanitization — ``../../etc/passwd`` escaped the storage root (§4.2 / SPEC §15.1). * api/integrations/webhooks/utils.py — drop the trailing ``[len(prefix):]`` slice in verify_hmac_signature. The earlier slice truncated 7 chars off a 64-char SHA-256 hex digest, making ``hmac.compare_digest`` always return False — webhook HMAC verification rejected every request (§4.3 / SPEC §15.2). --- Group C: async CRITs (audit §3) --- * api/integrations/webhooks/helpers.py — convert ``async def enhanced_init`` to sync ``def``. Python ignores async-ness on ``__init__``; the coroutine returned from the async form was never awaited and leaked on every webhook walker construction (§3.1). * api/integrations/webhooks/helpers.py — add missing ``await`` in the async-endpoint branch of webhook_wrapper (§3.2). Both arms of the if/else were identical, so coroutines leaked unawaited. * api/integrations/storage/service.py — add missing ``await`` on ``self.file_interface.delete_file(...)``; the unawaited call left ``success`` as the coroutine object and skipped the delete (§3.3). * core/graph.py — wrap Path.write_text calls inside ``generate_graph_dot`` and ``generate_graph_mermaid`` in ``asyncio.to_thread`` so blocking disk I/O does not stall the event loop inside async functions (§3.4-§3.5). --- Group D: walker protection (audit §2) --- SPEC §6.3 promised that exceeding ``max_steps`` / ``max_visits_per_node`` / ``max_execution_time`` raises documented exception types. The implementation silently swallowed ProtectionViolation into ``walker.report`` and continued. ``run()`` also called ``_protection.reset()`` unconditionally — ``resume()`` re-entering ``run()`` cleared step / visit / timer counters, defeating limits via pause/resume cycling. ``max_trail_length`` was unimplemented despite docstrings. Multiple WalkerQueue insert paths bypassed ``max_size``. * core/entities/walker_components/protection.py — add ``_started`` flag and ``start_if_needed()`` idempotent initializer; preserve explicit ``reset()`` for callers wanting a clean restart (§2.2). * core/entities/walker.py — Walker.run() now calls ``start_if_needed`` (so resume cannot reset counters) and catches ``ProtectionViolation`` to raise the documented ``InfiniteLoopError`` / ``WalkerTimeoutError`` / ``WalkerExecutionError`` types (§2.1, SPEC §6.3 / §17). Pop ``max_trail_length`` from kwargs/env and rebuild the trail tracker with the configured bound (§2.3). * core/entities/walker_components/walker_trail.py — back ``_trail`` with a bounded ``deque(maxlen=max_length)``; ``0`` means unlimited (SPEC §6.4 / §2.3). * core/entities/walker_components/walker_queue.py — every insert path (prepend, append, add_next, insert_after, insert_before) now respects ``max_size`` and emits a one-shot WARNING on first drop (§2.4, §2.5). insert_after / insert_before return the list of actually-inserted items rather than the requested list. --- Group E: DynamoDB throttle retry coverage (audit §5) --- * db/dynamodb.py — wrap every aioboto3 wire call (batch_get_item, batch_write_item, scan, query) in ``_run_with_throttle_retry`` so transient ProvisionedThroughputExceededException / ThrottlingException trigger the documented exponential backoff (§5.1 / SPEC §4.3). Was previously applied only to save/get/delete; find / count / batch ops surfaced throttles to callers as immediate failures. --- Tests --- Existing tests in tests/core/test_walker_protection.py and test_walker_trail_new.py cemented the swallow-everything behavior; updated to assert the documented exception types are raised (SPEC §6.3). New regression coverage (28 new test cases, all passing): * tests/core/test_entity_name_walker_and_save.py — Walker ``_entity_name`` classmethod; ID stability through save; find_edges_between with override. * tests/core/test_walker_protection_audit_fixes.py — ``start_if_needed`` idempotency vs ``reset``; run() raises InfiniteLoopError / WalkerExecutionError; max_trail_length wired; WalkerQueue insert paths respect max_size. * tests/storage/test_versioning_path_sanitizer_audit.py — path- traversal coverage on all five versioning methods. * tests/api/test_webhook_hmac_audit_fix.py — HMAC verify round-trip + tamper rejection. Verification: pytest tests/core tests/storage tests/db tests/api → 1537 passed; tests/integration → 47 passed; tests/cache + observability + runtime + serverless + logging + utils + testing → 192 passed, 1 otel-import skip. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds detailed [Unreleased] entries for the 16 CRIT findings closed in the previous commit, including the breaking behavioral change in Walker.run() (protection limits now raise documented exception types instead of being silently swallowed) and the new public surface (Walker.__entity_name__, TraversalProtection.start_if_needed, WalkerTrail max_length, JVSPATIAL_WALKER_MAX_TRAIL_LENGTH env var). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…s, pager, _id queries
Addresses 11 HIGH-class findings from AUDIT.md (Wave 2). All changes
SPEC-aligned; behavior matches the documented contract.
--- Group A: JsonDB sync stats (audit §3.6) ---
Six ``Path.exists()`` / ``Path.glob()`` calls inside ``async`` methods of
JsonDB performed blocking syscalls and stalled the event loop.
* db/jsondb.py — wrap stat/glob in ``asyncio.to_thread`` for
``_async_read_json``, ``count``, ``find_many``, ``find``. Extracted
the glob-and-filter logic into a static ``_list_collection_json_files``
helper so the to_thread call sites stay readable.
--- Group B: __init_subclass__ super() chains (audit §6.1-6.3) ---
``Node`` / ``Edge`` / ``Walker`` ``__init_subclass__`` did not call
``super().__init_subclass__``, so ``AttributeMixin.__init_subclass__``
never ran for their subclasses. ``protected`` / ``transient`` /
``private`` attribute registration was silently skipped.
* core/entities/{node,edge,walker}.py — add ``super().__init_subclass__(**kwargs)``
at the top of each.
--- Group C: auth races + API-key cache invalidation (audit §4.4-4.8) ---
``SessionManager._sessions`` / ``_user_sessions`` were mutated across
``await`` points without a lock — concurrent logout-vs-login raised
``RuntimeError: dictionary changed size during iteration`` and
``max_sessions_per_user`` enforcement was racy. The webhook-layer
``_API_KEY_CACHE`` had the same issue (``KeyError`` when a size-cap
eviction raced a reader). ``APIKeyService`` defaulted to
``get_default_context()`` when constructed without a context, putting
auth state on the wrong DB. ``APIKeyService.revoke_key`` did not
invalidate the cache so revoked keys authenticated for up to the
300-second TTL.
* api/auth/enhanced.py — single ``asyncio.Lock`` guards every
``SessionManager`` mutation. Cleanup helpers split into locked vs.
unlocked variants. Per-user cap now enforced under the lock and
evicts the oldest session by ``last_accessed`` when over the cap.
* api/integrations/webhooks/webhook_auth.py — single
``_API_KEY_CACHE_LOCK`` guards reads, eviction, and miss-population.
Introduces public ``invalidate_api_key_cache`` /
``invalidate_api_key_cache_hash`` hooks.
* api/auth/api_key_service.py — default to the prime database (never
``get_default_context()``). ``revoke_key`` invokes the cache
invalidation hook so revocation is immediate.
--- Group D: ObjectPager (audit §8.1-8.2) ---
The in-memory ``_cache`` was never invalidated on writes — callers got
stale rows after any save/delete. Keyset pagination via ``after_id``
combined with ``order_by`` returned wrong/missing rows because the
cursor only tracks ``id``.
* core/pager.py — drop ``_cache`` entirely; every ``get_page`` hits the
database. Reject ``after_id`` combined with ``order_by`` with a
clear ``ValueError`` rather than silently breaking cursor semantics.
* tests/core/test_pagination.py — existing tests asserted the cache
behavior; updated to assert the new (cache-free) behavior and absent
``_cache`` attribute.
--- Group E: default find_one_and_update _id-vs-id normalization (audit §5.3) ---
Default ``find_one_and_update`` / ``find_one_and_delete`` impls fed the
caller's query into ``QueryEngine.match`` against records stored by
non-Mongo backends (JsonDB / SQLite / DynamoDB) — but those backends
only persist ``id``. Callers following the Mongo convention of
``{"_id": ...}`` got silent misses.
* db/database.py — add ``_normalize_id_query`` helper; default impls
rewrite ``_id`` → ``id`` when only ``_id`` is present. MongoDB
override is unaffected. Callers passing both keys preserve verbatim.
--- New regression tests (11 cases) ---
* tests/db/test_default_compound_ops_id_normalization.py — verifies
``_id`` and ``id`` query keys are equivalent on default compound ops.
* tests/core/test_pager_audit_fixes.py — verifies the pager has no
cache attribute, rejects after_id + order_by, and never returns
stale rows after writes.
Verification: pytest across all subdirs → 1783 passed, 1 skipped
(otel), 2 warnings. No regressions.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds [Unreleased] entries for the 11 HIGH findings closed in the previous commit. Two breaking behavioral changes flagged: ObjectPager no longer caches, and ObjectPager rejects after_id + order_by combo. Two new public surface entries documented: invalidate_api_key_cache and invalidate_api_key_cache_hash. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…, query parity, bulk_save Addresses 12 MED-class findings from AUDIT.md (Wave 3). Closes the latent-timebomb tier of the post-audit hardening plan. --- Group A: env allowlist + parse_bool consolidation (audit §7.1-§7.3) --- SPEC §10.2 promised unknown JVSPATIAL_* keys would be rejected at startup. ``env_adapter`` only READ enumerated keys; never scanned for strays. Three divergent ``parse_bool`` implementations (env_adapter, runtime/serverless, app_builder inline) produced different truth values for the same input. * env_adapter.py — declare ``ALLOWED_ENV_KEYS`` (frozenset of every JVSPATIAL_* key the library reads), add ``enforce_env_allowlist`` hooked into ``validate_server_config_requirements``. Default: warn once per unknown key. Strict mode via ``JVSPATIAL_STRICT_ENV_ALLOWLIST=true`` raises ``ValueError`` so typos fail-fast in production. * env_adapter._parse_bool — delegates to ``env.parse_bool`` and logs a warning on unrecognized values rather than silently mapping to False. * runtime/serverless._parse_bool — delegates to ``env.parse_bool``; preserves the historical ``enabled``/``disabled`` aliases for ``SERVERLESS_MODE``. * api/components/app_builder.py — JVSPATIAL_DOCS_DISABLED now uses ``env.parse_bool`` instead of an inline 4-element truthy set. --- Group B: CORS wildcard warning + EXPOSE_ERROR_DETAILS prod guard --- * api/config_groups.py — ``CORSConfig`` gains a model_validator that emits a WARNING when ``cors_origins`` contains a wildcard. Add ``cors_allow_wildcard=True`` opt-out for callers who genuinely need it (audit §4.12 / SPEC §15.4). * api/components/error_handler.py — ``_expose_error_details_to_clients`` consults ``JVSPATIAL_ENVIRONMENT`` (or ``ENVIRONMENT``); when the runtime is signalled as production, the flag is ignored and a one-shot warning is logged. Generic 500 message is returned instead (audit §4.10 / SPEC §15.5). --- Group C: MongoDB supports_transactions honesty (audit §5.9) --- * db/mongodb.py — add ``MongoDB.is_transactional()`` async probe. Uses the ``hello`` admin command to detect replica set / sharded topology and caches the result. ``begin_transaction`` short-circuits to ``None`` on non-transactional topologies instead of attempting start_session/start_transaction every time. Class attribute ``supports_transactions = True`` is preserved as the documented API-surface contract. --- Group D: QueryBuilder ↔ QueryEngine operator parity (audit §5.2) --- ``QueryBuilder`` advertised ``$nor``, ``$mod``, ``$all``, ``$type``, ``$not`` (field-level), but ``QueryEngine._match_value`` returned False for any unknown operator and ``match`` ignored ``$nor``. Queries built via the public builder silently matched nothing. * db/query.py — implement ``$nor`` at top level; ``$mod``, ``$all``, ``$type``, ``$not`` at field level. Optimizer markers (``$hint``, ``$select``) injected by ``optimize_query`` are skipped explicitly rather than silently no-matching. Unknown operators (top-level or field-level) now raise ``QueryError`` so callers see the bug rather than mysteriously empty result sets. --- Group E: bulk_save partial-success semantics (audit §5.6, §5.7) --- * db/database.py — add ``BulkSaveResult`` dataclass and ``bulk_save_detailed`` API returning ``attempted``/``saved``/ ``failed_ids``. ``bulk_save`` is preserved as a thin wrapper that returns ``result.saved`` for back-compat. Default impl catches per-record exceptions, accumulates ``failed_ids``, and reports the breakdown. The ``ValueError`` on missing-id includes the offending index (audit §5.15). --- Tests (22 new cases, all passing) --- * tests/api/test_env_allowlist_audit.py — unknown-key warn vs strict raise, ALLOWED_ENV_KEYS spot checks. * tests/api/test_cors_wildcard_and_error_detail_audit.py — wildcard warning, opt-out, prod-guard for EXPOSE_ERROR_DETAILS. * tests/db/test_query_operator_parity_audit.py — $nor/$mod/$all/$type/$not semantics, unknown-op raises, optimizer-marker skip. * tests/db/test_bulk_save_detailed_audit.py — BulkSaveResult, back-compat int return, missing-id index in error. Verification: pytest across all subdirs → 1805 passed, 1 skipped (otel), 2 warnings. No regressions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds [Unreleased] entries for the 12 MED findings closed in the previous commit. Six new public surface entries (BulkSaveResult, bulk_save_detailed, MongoDB.is_transactional, CORSConfig.cors_allow_wildcard, JVSPATIAL_STRICT_ENV_ALLOWLIST, ALLOWED_ENV_KEYS + helpers, new QueryEngine operators). No breaking changes — additions and defensive-warning-only behavior shifts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…p, stability polish, dead code Addresses the LOW-severity / cleanup tier of the post-audit hardening plan. --- Group A: deprecate generate_id_async (audit §3.11) --- * core/utils.py — ``generate_id_async`` is now a deprecated alias for ``generate_id``. ID generation is pure computation; the async signature was a vestige. Emits a once-per-process ``DeprecationWarning`` pointing at the canonical sync API. Scheduled for removal in 0.1.0. --- Group B: SQLite cross-loop handling (audit §5.10 / SPEC §4.3) --- * db/sqlite.py — track the event loop that owns the aiosqlite connection. On cross-loop reuse with a file-backed database, the connection is silently rebound to the current loop (data persists on disk). For ``:memory:`` databases the existing connection is kept since rebinding would silently truncate the dataset. Replaces the previous opaque "Future attached to a different loop" error. --- Group C: stability-tier polish (audit §7.7, §7.13) --- * utils/stability.py — add public ``emit_experimental_once`` hook so opt-in flag-driven surfaces (e.g. ``JsonDBTransaction(best_effort=True)``) no longer reach into the private ``_emit_once`` implementation. * db/transaction.py — switch JsonDBTransaction to the public hook. * api/middleware/manager.py — derive docs CSP-relaxation prefixes from ``ServerConfig.docs_url`` / ``redoc_url`` / ``openapi_url`` at install time so callers that customize the docs URL keep Swagger UI rendering under the relaxed CSP. --- Group D: remove dead JSONTransaction (audit §5.14) --- * db/transaction.py — drop the unused ``JSONTransaction`` class (``JsonDBTransaction`` had replaced it). Remove from ``__all__`` and the module docstring. --- Group E: LOW hygiene (audit §4.11, §4.13, §7.14) --- * api/auth/service.py — drop JWT secret length from debug log (narrows search space for brute force). Log ``secret_configured=bool(...)`` instead. * api/auth/service.py — ``validate_token`` warning now logs ``db_type=type(database).__name__`` instead of the filesystem ``base_path`` (avoids leaking on-disk layout to log sinks). * serverless/tasks/stub.py — ``LoggingNoopTaskScheduler.schedule`` downgraded from per-call WARNING to DEBUG. The once-per-process startup error from ``serverless.factory._note_noop_in_serverless`` is sufficient; the per-call warning was a CloudWatch cost liability on misconfigured serverless deployments. --- Tests (10 new cases, all passing) --- * tests/db/test_sqlite_cross_loop_audit.py — same-loop reuse, cross-loop auto-rebind for file paths, owning-loop tracking. * tests/utils/test_wave4_polish_audit.py — public ``emit_experimental_once`` once-per-name semantics, deprecation emission on ``generate_id_async``, ``JSONTransaction`` removal. Verification: pytest across all subdirs → 1811 passed, 1 skipped (otel), 3 warnings. No regressions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds [Unreleased] entries for the LOW-tier cleanup landed in the previous commit. Adds Deprecated and Removed sections: generate_id_async is deprecated, JSONTransaction is removed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…eferred secret, SkipNode, polish
Closes the remaining outstanding audit items (LOW tier and a small
number of MED/HIGH leftovers).
--- Group A: Windows-reserved filenames (audit §4.18 / SPEC §15.1) ---
* storage/security/path_sanitizer.py — reject reserved Windows
filenames (CON, PRN, AUX, NUL, COM1-9, LPT1-9) regardless of host
OS so cross-platform storage cannot be DoS'd by an upload with a
reserved stem. ``CON.txt`` is rejected (Windows resolves the device
before the extension). Names that merely start with a reserved
stem (``CONFIG.json``, ``COMRADE.bin``) still pass.
--- Group B: .env.example accuracy (audit §7.5) ---
* .env.example — CORS section showed ``Default: *`` and example
``JVSPATIAL_CORS_ORIGINS=*``. Actual defaults are the localhost
whitelist defined in ``CORSConfig``. Updated to show the real
defaults and flag that wildcards trigger a startup WARNING.
--- Group C: runtime parse_bool strictness (audit §7.3) ---
* runtime/serverless.py — unrecognized non-empty ``SERVERLESS_MODE``
values now log a WARNING (preserving the False fallback for
back-compat). Silent garbage-to-False mapping hid typos.
--- Group D: SkipNode + Walker type_code enforcement ---
* core/entities/__init__.py — declare ``TraversalSkipped`` and
``TraversalPaused`` exception classes (previously referenced in
``jvspatial/exceptions.py`` but never defined).
* core/entities/walker.py — ``Walker.skip()`` raises the typed
``TraversalSkipped`` exception. ``_execute_visit_hooks`` matches
via ``isinstance`` instead of the fragile
``"Node skipped" in str(e)`` substring (audit §2.9 / SPEC §6.5).
* core/entities/walker.py — Walker rejects construction with a
non-``"w"`` ``type_code`` so the SPEC §1.1 ID-format invariant
(``w.EntityName.<hex>``) cannot be corrupted by a stray kwarg
(audit §2.10).
--- Group E: fail-closed deferred-invoke secret (audit §4.16) ---
* api/deferred_invoke_route.py — ``_deferred_invoke_secret_ok`` now
returns ``False`` when ``JVSPATIAL_DEFERRED_INVOKE_SECRET`` is
unset, emitting a single WARNING. Previously an unset secret
allowed any caller, exposing the internal endpoint on misconfigured
deployments. Disable the route entirely with
``JVSPATIAL_DEFERRED_INVOKE_DISABLED=true`` when not needed.
* tests/serverless/test_deferred_invoke.py — updated the two
pre-existing happy-path tests to set a secret + send the header
(the fail-closed change is intentional and the tests now match
the new contract).
--- Group F: SQLite id coercion + dead jvtmp filter (audit §5.16, §5.20) ---
* db/sqlite.py — ``save()`` now coerces ``record["id"]`` to ``str``
after the ``setdefault`` and persists the stringified form so
callers passing int/uuid ids can ``get()`` them back via ``str(id)``.
* db/jsondb.py — drop the dead ``not p.name.endswith('.jvtmp')``
filter on the ``*.json`` glob. Tmp files are named
``<id>.json.<pid>.<hex>.jvtmp`` (see ``_atomic._make_temp_path``)
so the glob already excludes them.
--- Tests (23 new cases, all passing) ---
* tests/storage/test_windows_reserved_audit.py — reserved-name
rejection across single filename and subdir contexts; non-reserved
names that share a stem prefix still pass.
* tests/core/test_wave5_walker_audit.py — ``TraversalSkipped``,
Walker type_code locked to ``"w"``.
* tests/api/test_deferred_invoke_fail_closed_audit.py — unset secret
denies, matching header / bearer allows, mismatched denies.
* tests/db/test_sqlite_id_coercion_audit.py — int id round-trips
through save + get(str(id)).
Verification: pytest across all subdirs → 1834 passed, 1 skipped
(otel), 3 warnings. No regressions.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds [Unreleased] entries for Wave 5 fixes. Two behavioral breaking changes flagged: deferred-invoke route is now fail-closed when the secret is unset, and Walker construction rejects non-``"w"`` type_code values. Plus two new public exception classes (TraversalSkipped, TraversalPaused). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Upstream-reported bug. ``wrap_function_with_params`` (and four other ``_find_request_parameter`` callers) detected Request parameters by iterating ``param.annotation`` raw values. With ``from __future__ import annotations`` (PEP 563) — or quoted forward refs — those annotations are *strings* like ``"Request"`` that match neither ``FastAPIRequest``/``StarletteRequest`` nor the ``__name__``/``__module__`` heuristic. The wrapper failed to detect the caller's Request parameter and corrupted the resulting route signature. ``ParameterModelFactory._create_function_model`` already calls ``typing.get_type_hints(func)`` to resolve forward refs; this fix mirrors that resolution in ``_find_request_parameter`` so both paths agree. * api/decorators/function_wrappers.py — add ``_resolve_type_hints`` helper that wraps ``typing.get_type_hints`` with a defensive fallback (empty dict on resolution failure, e.g. truly unresolvable forward ref). Extend ``_find_request_parameter`` with an optional ``func`` argument; when supplied, resolved hints take precedence over raw ``param.annotation``. Back-compat preserved when ``func`` is omitted. * All five internal call sites pass ``func``. Regression test (tests/api/test_request_detection_pep563_audit.py): PEP 563 handler is correctly detected; back-compat call without ``func`` still works. Verification: pytest across all subdirs → 1837 passed, 1 skipped (otel), 3 warnings. No regressions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Benchmark comparisonThreshold: ±25% (informational, does not block merge)
|
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
1 participant
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.
Type of Change
What type of change does this PR introduce? Mark all that apply:
Summary
What does this PR address?
This PR lands the full result of a structured five-wave foundation audit of
jvspatial, plus a parallel documentation rewrite that establishes the project's authoritative contract for downstream consumers and AI agents.It does three things at once:
Wave 1–Wave 5).__entity_name__per-subclass entity discriminator override,JVSPATIAL_DOCS_DISABLEDproduction posture,bulk_save_detailed()partial-success reporting,TraversalSkipped/TraversalPausedexception classes,WalkerTrail(max_length=...)bound, expandedQueryEngineoperator parity ($nor,$mod,$all,$type,$not),JVSPATIAL_STRICT_ENV_ALLOWLIST, and per-path docs CSP.SPEC.md(technical contract, every claim citingfile:line),PRD.md(product context, decision boundaries, non-goals),ROADMAP.md,CLAUDE.md+AGENTS.mdagent entry points, per-subpackageREADME.mdfiles, and a refresheddocs/md/index.Description
Bug Fixes:
The audit surfaced a large number of latent bugs. Highlights, grouped by area:
Identity & entity discriminator (Wave 1)
GraphContext.save_objectwas silently corrupting IDs of any entity using__entity_name__, because the ID-validation path compared againstcls.__name__and regenerated throughcls.__name__instead of the override. Root cause: the override was added later and several call-sites bypassedgenerate_id()/find_subclass_by_name(). Fixed by routing every identity-sensitive path through_entity_name().GraphContext.find_edges_between,Node._node_query,Node.count_neighbors,Node._matches_node_filter, andWalkerID generation were all making the same mistake. All now honor__entity_name__.Walker protection (Wave 1, Wave 2)
Walker.run()silently swallowedProtectionViolationintowalker.reportand returned normally, even though SPEC §6.3 documented that it raisesInfiniteLoopError/WalkerTimeoutError/WalkerExecutionError. Behavior now matches contract (BREAKING behavioral).WalkerQueue.prepend/append/add_next/insert_after/insert_beforebypassedmax_size, providing a silent protection bypass. All paths now respect the cap.TraversalProtectionwas reinitialized on everyrun(), so pause/resume cycles reset step / visit / wall-clock counters. Replaced withstart_if_needed().Async correctness (Wave 1, Wave 2, Wave 4)
awaitstatements (file delete, webhook wrapper async branch, graph DOT/Mermaid disk writes). Each leaked a coroutine and skipped the underlying operation.async def __init__on the webhook walker payload injector — Python ignores async__init__and returned an unawaited coroutine on every webhook walker construction.JsonDB._async_read_json,count,find_many,findwere callingPath.exists()/Path.glob()insideasyncmethods, blocking the event loop. All wrapped inasyncio.to_thread.SQLiteDBproduced an opaque"Future attached to a different loop"error when reused across event loops. Now silently rebinds theaiosqliteconnection for file-backed paths.Security (Wave 1, Wave 3, Wave 5)
verify_hmac_signatureslicedexpected_signature[len(prefix):], truncating 7 chars off a 64-char digest. Webhook HMAC verification rejected every request.LocalFileInterfaceversioning paths (create_version,get_version,list_versions,delete_version,get_latest_version) did not sanitizefile_path— caller-supplied../../etc/passwdescaped the storage root.AuthenticationService._verify_refresh_tokenSHA-256 fallback used==instead ofhmac.compare_digest(timing oracle).JVSPATIAL_DEFERRED_INVOKE_SECRETwas unset (open by default). Now fails closed (BREAKING behavioral).PathSanitizerdid not reject Windows-reserved names (CON,PRN,AUX,NUL,COM1-9,LPT1-9) on non-Windows hosts.validate_tokenwarning leakeddb_path. Both narrowed/removed.Persistence & query (Wave 2, Wave 3, Wave 5)
Database.find_one_and_updateandDatabase.find_one_and_deletedid not normalize{"_id": x}to{"id": x}, so Mongo-style queries silent-missed on JsonDB / SQLite / DynamoDB.QueryEnginesilently returnedFalsefor operators it didn't understand (including$nor,$mod,$all,$type,$notadvertised byQueryBuilder). Now raisesQueryErrorfor unknown operators and implements the missing ones.MongoDB.begin_transactionattemptedstart_session/start_transactionon standalone deployments every call. Now probes topology once viais_transactional()and caches.SQLiteDB.savedid not coercerecord["id"]tostr, breaking int /UUIDround-trips through SQLite's TEXT column.DynamoDB.{find, count, batch_get, batch_write}did not route through_run_with_throttle_retry, soProvisionedThroughputExceededExceptionandThrottlingExceptionfrom scan / query / batch surfaced immediately despite the documented backoff.Concurrency (Wave 2)
SessionManagermutations and the_API_KEY_CACHEhad no lock — concurrent create/invalidate/cleanup could raiseRuntimeError: dictionary changed size during iterationorKeyErroron a cleanup race. Both now hold anasyncio.Lock.Auth / DB placement (Wave 2)
APIKeyService(context=None)defaulted toget_default_context(). Auth state must live on the prime DB; now defaults to prime.APIKeyService.revoke_keydid not invalidate the webhook-layer cache, so a revoked key kept authenticating for up to 5 minutes.Entities (Wave 4)
Node,Edge,Walker__init_subclass__did not callsuper().__init_subclass__(), soAttributeMixin.__init_subclass__never ran andprotected/transient/privateregistration silently dropped on subclasses.Pagination / caching (Wave 5)
ObjectPagercached pages in an in-memory_cache, returning stale results after writes. Cache removed entirely (BREAKING behavioral); use backend-level read-through cache instead.ObjectPager.get_page(after_id=..., order_by=...)silently skipped or duplicated rows on writes between pages becauseafter_idonly tracksid. Now raisesValueError.Misc
Walker(type_code=...)accepted arbitrary values, corrupting thew.EntityName.<hex>ID format. Now raisesValueErrorfor any value other than"w".skip()raisedJVSpatialError("Node skipped")and downstream code did substring matching on the message. Now raisesTraversalSkipped._find_request_parameterwere unresolved, causing endpoint parameter detection to fail on annotated modules.Feature Request:
New product surface introduced or formalized in this PR:
JVSPATIAL_DOCS_DISABLEDenv var — when truthy,AppBuilder.create_appconstructs FastAPI withdocs_url=None,redoc_url=None,openapi_url=None,swagger_ui_oauth2_redirect_url=None. Recommended production posture. Motivation: deployments that do not want the spec leaked needed a single switch rather than overriding four FastAPI kwargs.__entity_name__per-subclass override — opt-in declarative override for the persisted entity discriminator. Motivation: applications need stable discriminators independent of Python class names (renames, two unrelated classes sharing__name__).bulk_save_detailed()+BulkSaveResult— partial-success reporting for bulk save operations.bulk_savebecomes a thin int-returning wrapper. Motivation: JsonDB and DynamoDB could partially fail and silently drop records.TraversalSkipped/TraversalPausedexceptions — typed exception classes for walker flow control. Replaces substring matching on"Node skipped".WalkerTrail(max_length=N)— wires the previously-undocumented bound throughWalker(max_trail_length=...)andJVSPATIAL_WALKER_MAX_TRAIL_LENGTH.0means unlimited.JVSPATIAL_STRICT_ENV_ALLOWLIST— truthy values turn unknown-JVSPATIAL_*keys into a startupValueErrorso typos fail-fast.MongoDB.is_transactional()async probe — caller-side topology check before opening a transaction.QueryEngineoperators —$nor(top-level),$mod,$all,$type,$not(field-level)./docs,/redoc,/openapi.json(and customized variants) so FastAPI's CDN-hosted Swagger UI renders; app routes keep the strict default.CORSConfig.cors_allow_wildcardopt-out — wildcard CORS origins now emit a startup WARNING unless explicitly opted in.emit_experimental_once(name, message)— public hook for opt-in experimental warnings.Refactor Request:
jvspatial.env,jvspatial.env_adapter,jvspatial.runtime.serverless, andjvspatial.api.components.app_builder— all delegate toenv.parse_bool.JVSPATIAL_DEBUG=onandSERVERLESS_MODE=onnow agree on truthiness.ALLOWED_ENV_KEYSfrozenset andenforce_env_allowlist()/discover_unknown_jvspatial_env_keys()helpers added tojvspatial.env_adapter; called fromvalidate_server_config_requirements()at server startup.fbdcf18) — reduces unnecessary index lookups on the hot path.Changes Made
High-Level Summary:
_idquery normalization,APIKeyServiceprime-DB default and revoke-time cache invalidation.JVSPATIAL_*env allowlist, CORS wildcard warning, Mongo transaction honesty + caching,QueryEngineoperator parity,bulk_save_detailed()partial-success reporting.generate_id_asyncdeprecation), SQLite cross-loop rebind, stability polish (publicemit_experimental_once), dead-code removal (JSONTransaction).TraversalSkippedexception class, SQLite id coercion, miscellaneous polish.__entity_name__override — declarative per-subclass entity discriminator with new regression suite.JVSPATIAL_DOCS_DISABLED+ per-path docs CSP — production docs hardening._find_request_parameterso endpoint parameter detection works on annotated modules.SPEC.md(technical contract, file:line citations), newPRD.md(product context, decision boundaries), newROADMAP.md, newCLAUDE.md+AGENTS.mdagent entry points, per-subpackageREADME.mdfiles, refresheddocs/md/index,AUDIT.md(150 findings across 5 dimensions),AUTHORS,README.mdtouch-ups.tests/api/,tests/core/,tests/db/,tests/storage/,tests/utils/pinning each wave's fixes.Checklist
Mark all that apply:
Steps to Test
pip install -e '.[dev,test]' pre-commit installInfiniteLoopError/WalkerTimeoutError/WalkerExecutionError) instead of swallowing intowalker.report— seetests/core/test_walker_protection_audit_fixes.py.ObjectPagerno longer caches — seetests/core/test_pager_audit_fixes.py.JVSPATIAL_DEFERRED_INVOKE_SECRET— seetests/api/test_deferred_invoke_fail_closed_audit.py.Walker(type_code=...)rejects values other than"w"— seetests/core/test_wave5_walker_audit.py.skip()raisesTraversalSkipped— see same suite.SPEC.md,PRD.md,ROADMAP.md,CLAUDE.md,AGENTS.md, and the subpackageREADME.mdfiles.JVSPATIAL_DOCS_DISABLED=trueposture —/docs,/redoc,/openapi.jsonshould all return 404 with no spec leak.Additional Context
AUDIT.md(150 findings) and the per-wave commit messages (c7b412b,8eaa993,809fe2e,66a93bd,980a9fc).CHANGELOG.mdentries document every Added / Fixed / Deprecated / Removed item with the audit reference and SPEC section.SPEC.mdclaim cites afile:lineso future drift is detectable.CHANGELOG.mdand above). They restore behavior to the documented contract; callers that depended on the swallow/silent-pass semantics will need to wrap calls intry/except. No public API signatures were removed.Questions or Concerns
type_codevalidation,TraversalSkipped) are intentional alignment with the documented contract, not new restrictions — but downstream consumers may have been depending on the prior silent-pass behavior. Worth coordinating ship timing if any are blocking.LLM-CODING-GUIDE.mdis preserved as a legacy reference and marked informational-only inCLAUDE.md§"Pointers to authoritative sources".SPEC.md/PRD.mdare the authoritative replacements. Confirm we want it retained vs. deleted.