Repository navigation
fix(gridfs): operator-audited stale ownership recovery and crash matrix [WIP] - #13
Conversation
32cfee1
into
fix/gridfs-id-ownership-v01
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 460b958. Configure here.
| await chunks.deleteMany({ files_id: id }); | ||
| } else if ((sourceStatus === 'uploading' || sourceStatus === 'cleaning' || | ||
| sourceStatus === 'aborting') && !visible) { | ||
| await chunks.deleteMany({ files_id: id }); |
There was a problem hiding this comment.
Stale visibility drives chunk deletion
High Severity
Recovery records whether a files document exists before the durable recovering claim, then later runs chunks.deleteMany from that stale !visible flag for uploading, cleaning, and aborting. A file that becomes visible in that window still loses its chunks, violating the visible-file preservation invariant.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 460b958. Configure here.
| async function verifyVisibleFile(db, bucket, id, session) { | ||
| const files = db.collection(bucket + '.files'); | ||
| const doc = await files.findOne({ _id: id }); | ||
| if (!doc) return false; |
There was a problem hiding this comment.
Missing file treated as verified
Medium Severity
verifyVisibleFile returns false when the files document is gone instead of failing closed. The finalizing, uploading, and renaming callers ignore that result and still delete the ownership row, so a file that disappeared after the earlier visibility snapshot is treated as successfully verified.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 460b958. Configure here.
| // Recovery intentionally retains the original kind/ownerToken while | ||
| // changing status to recovering. A late original writer must not be | ||
| // able to remove that new recovery fence with its stale ownerToken. | ||
| if (released.deletedCount !== 1) throw uploadBusy(operation, ctx, owner.id); |
There was a problem hiding this comment.
Cleanup deletes chunks past recovery
High Severity
The new status match in releaseOwner only stops a late writer from deleting a recovering row. cleanupFailedDirect still deleteManys chunks by files_id before that check, so a late failed upload can wipe bytes after recovery has fenced the owner or already released the id for reuse.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 460b958. Configure here.
Scope
Follow-up candidate for PR #12; do not merge until reviewed and all checks pass.
scripts/maintenance/gridfs-recover.cjs. Requires verified external writer quiescence, exact observed status/updatedAt/ownership token, a durable recovering claim and audit record. Never auto-releases based on elapsed time.Merge gate
Note
High Risk
Changes GridFS ownership release rules and adds a maintenance tool that can delete chunks and complete deletes; misuse or unquiesced writers could cause data loss despite safeguards.
Overview
Introduces an operator-only maintenance path for stranded GridFS ownership rows in
<bucket>.uploads, plus CI and docs to gate its use.A new
scripts/maintenance/gridfs-recover.cjsutility defaults to dry-run, requires exactstatus/updatedAt/ token match and an explicit--quiescedapply, writesownership_recovery_audit, and holds a durablerecoveringclaim while it reconciles interrupted direct upload, delete, cleanup, rename, append, finalize, and abort states—with visible-file chunk checks and fail-closed behavior on SHA conflicts or ambiguous cases.releaseOwnernow deletes only whenkind,ownerToken, and expectedstatusall match, so a delayed original writer cannot drop an active recovery fence.English and Chinese runbooks (
gridfs-recovery.md) document drain/migration/rollback steps;gridfs.mdlinks to them. A largegridfs-stale-recovery.test.tsmatrix is wired intonpm test, the memory-server matrix, and GridFS GitHub workflows, which also gain concurrency cancellation, timeouts, and triggers forfix/gridfs-id-ownership-v01.Reviewed by Cursor Bugbot for commit 460b958. Bugbot is set up for automated code reviews on this repo. Configure here.