App locking hardening - #31
Open
rawe0 wants to merge 13 commits into
Open
Conversation
- FileLockedException now maps to a FileLockedError (OSError subclass) so lock conflicts are distinguishable from file-not-found and other errors. - acquire_or_refresh_lock returns held/foreign/no_file instead of a bool; new ensure_write_lock (423 on foreign holder), lock_after_create and foreign_lock_holder (exact holder match via the revived get_lock). - lock_holder property computed after traitlets config, fixing the init ordering that made the per-user suffix a no-op; the suffix is now opt-out via lock_holder_suffix_client_id so migrating to a shared holder is a config change. - cs3_token initialized before reading the token file (no AttributeError when the file is missing); config key lock_as_attr renamed to lock_by_setting_attr, the key cs3client actually reads. - unlink/rename now carry the lock id; new vfs_touch; sync copyfile_sync. - SessionTracker.user_left returns None for unknown sessions (no more spurious unlocks) and sweep() supports the new background refresher that keeps open documents' locks alive and releases stale ones. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
POST/DELETE now require path+session_id, map the API path to the storage path once, run all gRPC calls via run_sync, report the foreign holder on conflicts, 404 unknown files/sessions instead of unlocking, and start the background lock refresher. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Hybrid manager: - open() locks only for write modes and no longer falls back to FUSE (the fallback double-yielded on flush errors and turned lock conflicts into unlocked local writes). - New files/notebooks/directories and empty-file touch are created via CS3 and locked right after, instead of writing through the FUSE mount where reva could not see (or lock) them. - hybrid_lstat/exists/isfile/isdir fall back to a CS3 stat on a local miss, fixing the create-then-stat race while FUSE catches up; exists/file_exists/ dir_exists/_base_model/get/_dir_model/_save_directory use them. - rename/delete reimplemented via CS3 (carrying the lock id) with a foreign-holder pre-check, since cephmount enforces nothing on writes. - lock_is_ours (FUSE xattr substring match) replaced by CS3 GetLock exact holder matching, only checked when content is requested so listings don't pay one GetLock per entry. - sync save() drops the upstream checkpoint block that awaited nothing, and delegates chunked uploads to LargeFileManager whose appends now resolve to the CS3 open (chunks no longer mix CS3 and FUSE backends). Pure manager: - saves acquire the lock first (423 when foreign), then lock new files after creation; empty new files use touch instead of a zero-byte PUT. - models mark foreign-locked files read-only; delete/rename pre-check the holder and translate lock conflicts to 423. - chunk-aware save() (copied from AsyncLargeFileManager) so chunked uploads assemble instead of each chunk truncating the file. Checkpoints: restoring the "0_0" placeholder is a no-op and a locked restore surfaces as 423. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Tests cover the tri-state acquire against an in-memory CS3 fake with EOS-like lock semantics, holder exact-matching and the shared-holder migration, session tracking, the /lock endpoints through a real jupyter server, both managers' lock-aware save/delete/rename/model flows, the FUSE-miss fallbacks, chunked uploads, and the background refresher. README documents the locking traits and why tus_enabled must stay off (cs3-python-client sends a misspelled X-Lock_Holder header on its TUS branch; to be fixed upstream). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
New locking plugin hooks IDocumentWidgetOpener.opened: each opened document POSTs /lock with a per-widget session id, heartbeats every expiration/3 (expiration read from GET /lock), follows renames, and DELETEs on close so the server can unlock when the last session leaves. A missed close (crashed tab) is covered by the server-side sweep. Foreign-locked documents are reported and open read-only via the contents model. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Creating a notebook failed with a 500 because acquire_or_refresh_lock assumed a missing file surfaces as NOT_FOUND. It does not: the EOS driver wraps the not-found while resolving the reference, so the storage provider's type switch falls through to a generic INTERNAL error, which cs3client raises as UnknownException and our handler turned into a bare OSError that escaped the acquire. RefreshLock is now treated as a pure optimization - any failure falls through to SetLock, which is authoritative. SetLock's own failures are disambiguated by an existence check, so a file that is not there yet reports no_file (it gets locked right after creation) while a genuine storage error still propagates. Foreign locks are unaffected: both "not the holder" and "already locked" come back as FAILED_PRECONDITION. The same opacity is handled where it could break the surrounding operation: lock_after_create stays best-effort, foreign_lock_holder fails open rather than failing the read or delete it annotates, and the background refresher survives storage errors instead of dying. The unknown-error fallback now keeps the original message, which is the only thing distinguishing these conditions. The test fake now mirrors reva's real error shapes (per-driver via missing_file_error), which is why this slipped through before; without the fix its new-notebook test reproduces the reported 500. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The config example used c.CS3FileManagerMixin.*, which the hybrid manager does not inherit from - those settings were silently ignored for it. CS3Mixin is the only class in both managers' MRO. Also document the labextension's locking plugin and what happens without it installed (locks taken on save, released only by expiry). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rawe0
force-pushed
the
app-locking-hardening
branch
2 times, most recently
from
August 11, 2026 14:43
95f5429 to
e5ae6d4
Compare
tomasr8
self-requested a review
August 12, 2026 12:18
rawe0
force-pushed
the
app-locking-hardening
branch
from
August 17, 2026 08:43
e5ae6d4 to
f0e9594
Compare
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.
No description provided.