Release 0.1.0: security review remediation - #49
Merged
Merged
Conversation
Benchmark comparisonThreshold: ±25% (informational, does not block merge)
|
Benchmark comparisonThreshold: ±25% (informational, does not block merge)
|
Benchmark comparisonThreshold: ±25% (informational, does not block merge)
|
Benchmark comparisonThreshold: ±25% (informational, does not block merge)
|
Benchmark comparisonThreshold: ±25% (informational, does not block merge)
|
Benchmark comparisonThreshold: ±25% (informational, does not block merge)
|
Benchmark comparisonThreshold: ±25% (informational, does not block merge)
|
Benchmark comparisonThreshold: ±25% (informational, does not block merge)
|
eldonm
force-pushed
the
codex/security-review-remediation
branch
from
September 27, 2026 18:16
c6c953e to
ad5817f
Compare
Benchmark comparisonThreshold: ±25% (informational, does not block merge)
|
Benchmark comparisonThreshold: ±25% (informational, does not block merge)
|
eldonm
marked this pull request as draft
September 27, 2026 18:24
Benchmark comparisonThreshold: ±25% (informational, does not block merge)
|
Benchmark comparisonThreshold: ±25% (informational, does not block merge)
|
eldonm
marked this pull request as ready for review
September 27, 2026 19:47
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.
Summary
Remediates the 37 findings in the combined gap review and security audit. The high severity auth failures are addressed first in the implementation: database lookup errors deny JWTs, current database roles govern authorization, revocation uses the active session store, logout invalidates the bound refresh token, and public registration cannot grant admin.
The remaining changes harden webhook HMAC and API key scope, path containment, file access and production defaults, graph deletion and traversal, database writes and index inputs, scheduler and retry behavior, and other confirmed findings. The in-process asynchronous webhook mode was removed; handlers and idempotency storage now finish within the request. Durable asynchronous processing should use an external queue.
SPEC.md,CHANGELOG.md, operational docs, and the numbered review ledger were updated alongside code. ROADMAP §2 product decisions remain separate as requested.Review guide
SPEC.md,CHANGELOG.md,docs/md/security-review.md.Verification
pytest --cov=jvspatial --cov-report=term-missing -q --disable-warningspassed; total coverage 64%. Optional external-service tests were skipped where Redis, Postgres, MongoDB, or OTel dependencies were unavailable.pre-commit run --all-filespassed, including Black, isort, flake8, mypy, and Detect Secrets.git diff --checkpassed.origin/mainand this PR; after normalizing line numbers, there are no new diagnostics. The configured pre-commit mypy and PR CI checks pass.Agent guide migration
Root AGENTS.md now holds the complete agent guide; CLAUDE.md is removed. SPEC, PRD, documentation index, audit citations, and source comments point to AGENTS.md. Historical changelog text remains intact.
Release 0.1.0 notes
Security
where=predicates are rejected. Graph deletion, walker errors, DynamoDB bulk writes, and deferred operations now report failures instead of success.JVSPATIAL_FILES_PUBLIC_READ=trueonly when intentional.JVSPATIAL_ENVIRONMENT=productionunpublishes/docs,/redoc, and/openapi.jsonby default. SetJVSPATIAL_DOCS_DISABLED=falsefor an explicit override.JVSPATIAL_DEFERRED_INVOKE_ALLOW_LOOPBACK=trueonly for Lambda Web Adapter self-invoke.rate_limit.auth_entrypoint_rate_limit_enabled=False; the 5/60s cap remains on by default. Declared private helper methods can be replaced on entity instances, while undeclared underscore attributes remain rejected./status,/logs, and/graphretain enforced admin roles.Changed
AGENTS.mdis now the canonical agent guide. Its formerCLAUDE.mdcontent has been consolidated there;CLAUDE.mdis removed.Pre-merge 0.1.0 review addendum (e281236)
pre-commit run --all-files, focused regressions, and wheel build passed. The full local pytest run reached 100% with no failures; its Python process hung during interpreter shutdown. jvagent's suite reached 100% with no failures on the editable tree. Integral Core PR #65 passed its CI-faithful smoke run in an isolated worktree.OAuth key custody addendum (ad5817f)
The OAuth signing-key store now encrypts private PEM before persistence when
JVSPATIAL_OAUTH_KEY_ENCRYPTION_KEYis configured. A legacy plaintext row is rewrapped on first use. Missing or incorrect keys fail closed for encrypted rows.OAuthSigningKey.save()also encrypts a plaintext signing copy if a caller persists it again. The SPEC, security review, environment reference, operations guidance, agent guide, changelog, and targeted tests were updated. A production host must provision the same Fernet key across workers; key rotation, backup recovery, and managed KMS/HSM custody remain operational release work.Verification on this addendum: OAuth suite and
pre-commit run --all-filespassed. A local full pytest run had two walker test failures outside the changed OAuth paths (test_execute_direct_execution_walkerandtest_walker_endpoint_with_auth_false_allows_access). CI for this exact head must pass before merge; treat these local failures as unresolved until reconciled. Integral Core PR #65 is a draft dependency migration and remains blocked by seven PostgreSQL failures and unpublished distribution wheels.dd3702fmakes the keystore reject missing encryption keys inJVSPATIAL_ENVIRONMENT=production; the focused OAuth suite and all pre-commit checks pass. Core's exact source pin has been advanced to this commit.Final consumer repair pass (fe54330)
PostgreSQL
$existsnow matches the in-memory null semantics; graph transactions isolate their identity map and evict touched parent cache entries; index setup is scoped to each database instance. File-backed SQLite closes its prior aiosqlite connection when rebinding across event loops. Regression tests cover these paths, including a real PostgreSQL round trip. Test fixtures now bind their own graph contexts and restore mocked metadata so the full suite is order independent.The full local jvspatial suite and all pre-commit hooks pass. Integral Core's full JSON and PostgreSQL suites pass with this source code; its focused PostgreSQL tests and CI smoke pass after a frozen sync of the exact Git candidate. jvagent's full local suite passes against the same package code. PR CI is rerunning after a version-portable SQLite test assertion replaced
Connection.is_alive().Publication remains gated on PR CI and required review. The dependent registry-install and release-artifact gates remain pending until jvspatial 0.1.0 and the compatible jvagent package are published.
Review status: All required CI checks on
fe54330are green (Python 3.10, 3.11, 3.12, benchmarks, pre-commit, and pip-audit). This PR is ready for the required human review; it has not been merged or published.