Repository navigation
fix(security): repair rollback pins, lifecycle cleanup and bounded processing - #142
Conversation
…anup Reworks #121 through #128 against current main. Includes 24 new authenticated PostgreSQL/SQLite regressions. Exact source tree locally passed 110 related tests and wheel/sdist build; target-runtime CI is still required. No deployment, merge, production access, or workflow changes in this product tree.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…pe protections Preserve both parents and all main changes. Complete tree d3f0250 independently reconstructed locally from the main CI source artifact and unchanged document blob identities. Only the security repair delta remains relative to e942028. Product target-runtime CI and review remain required; no production access or main merge.
The expanded integration regression caught short-code contract drift. Preserve the immutable v1 schema and enforce 128 entries at runtime after authorization, before source traversal/materialization. Replace schema-only assertions with actual signed 129-entry rejection (zero storage IO/writes) and successful 128-entry positive control. Existing immutable-shortcode assertion stays unchanged. Target-runtime CI remains required.
|
Follow-up pushed as 27917aa, verified complete tree 54b39853ab2ac0f25164ed90ec0fd6b7e300a065. Please review this head rather than the previous Reproduced against unchanged Review focus:
Fresh CI: Rewrite contracts, Recovery safety. PR body contains hashes, exact verification scope and remaining boundaries. No merge, deployment, test waiver or legacy-code restoration. @codex security review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27917aa108
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fd=os.open(self.path.with_name(self.path.name+'.writer.lock'), | ||
| os.O_CREAT|os.O_RDWR|os.O_CLOEXEC|os.O_NOFOLLOW,0o600) |
There was a problem hiding this comment.
When two processes open the same SQLite database through different path aliases—such as its real path and a symlink—this derives different .writer.lock files, so the new fence does not serialize them. After an INSERT OR ROLLBACK implicitly releases SQLite's own lock, the alias-backed writer can enter while the first transaction is still running external compensation, exposing the half-compensated state this fence is intended to prevent. Resolve or otherwise canonicalize the database path before deriving the sidecar path.
Useful? React with 👍 / 👎.
Current revision and scope
Reworks current-architecture security proposals #121–#128, with a follow-up for rollback serialization and upload pin cancellation. Refs #80, #81, #83, #85; does not close umbrella acceptance issues.
27917aa1080f55953871a7fa42188562450fdeda.54b39853ab2ac0f25164ed90ec0fd6b7e300a065.e94202899ab48399a586b0c613a1b817f9783d58(includes integrate: deterministic clearing, bounty and task C acceptance increments #119). The follow-up is a normal child ofcbc5741; existing history and account-scope/recovery protections are preserved.27c18f99c408769fe1f24964c64ee29d9ce7d4c40f3a96208151b6cdb0c4e261.931f889e681e443d3d25bd6edb432fb3d8c25a7f2ad232c01b3d4a9d481e5e95.Follow-up repairs in 27917aa
to_threaddoes not stop Git; rollback could previously remove a pin before the still-running thread wrote it back. Pin/unpin mutations now settle before the cancellation reaches metadata rollback. Tests exercise a real delayed Git ref update, not just a schema check or mocked success.Operational note: PostgreSQL connections must be direct or session-pooled, not transaction-pooled. SQLite's reference backend uses a persistent
.writer.locksidecar; do not mix old/new SQLite writers during rollout. No deployment was performed.Earlier repairs retained
hosting.preview@1input schema and write pause. Real signed 129-entry rejection has no content-store IO or new resources/revisions; 128 entries succeed.pending_rewrapand completed upgrade results; reclaim only disposable pending/failed challenges and preserve the duplicate-upgrade fence.(seq, topic)event projection, once-only migration, atomic append, andlimit+1selection before decoding event bodies. Authorization, signed cursor bindings, and reason redaction remain intact.Actual verification
Local verification used Python 3.13.5, PostgreSQL 15.19, actual SQLite, Git, Ed25519 and age. It is not the target Python 3.15 / PostgreSQL 16 / Valkey 9 environment.
cbc5741, final 21 follow-up regressions54b3985, 17-file related suitecompileall, Ruff for both new test files,git diff --check, and wheel/sdist build passedThe 129-case suite includes all 21 follow-up regressions, previous security regression files, both metadata backends, transfers and their endpoint, post/query-ref lifecycle, hosting, market contract/account-scope and store-foundation checks, and published dictionary/route checks. It is a focused related suite, not a claim that all repository tests ran locally.
Earlier evidence remains historical, not relabeled as current-head results: original-main initial regressions 10 failed / 2 passed; original #125 predicate 2 failed; unindexed topic query 1 failed;
41bb241related suite 110 passed;8eb8b1dexpanded suite 284 passed / 1 failed on immutable preview schema; final schema correction and signed boundary checks 2 passed;cbc5741focused suite 88 passed. The schema failure was fixed, not waived. The previous 285-case combination was not rerun locally after that correction.The prior
cbc5741Rewrite contracts and Recovery safety runs both completed successfully. They are superseded for merge purposes by fresh exact-head runs:These new runs were queued/running when this record was updated. Merge still requires exact-head four-shard completeness/disjointness, conformance/token-budget, applicable recovery/build gates, and review. No timeout, allowlist, skip, or assertion weakening was used.
Boundaries and publication
src/msgd: not ported or claimed fixed. No legacy architecture was restored; other PR branches were not changed.force=false.