Android SAF create/delete/rename, write-back error surfacing, and a single-use exec-websocket ticket#35
Merged
Merged
Conversation
…terminal The exec websocket took the base64 capability token as a ?token= query parameter (the browser WebSocket API cannot set headers), putting a credential that grants remote code execution into access/proxy logs and browser history. A new POST /v1/exec/ticket route, authenticated through the normal Authorization header, issues an opaque, unguessable, ~30s single-use ticket bound to the caller and the folder; the websocket now accepts ?ticket=&folder=, looks up the ticket, rejects replays/expired/unknown, marks it used, and authorises exec:pty over the folder from the authority captured at issue time. The long-lived token is no longer accepted on the websocket. TerminalPage now asks which folder to open the terminal in (a terminal runs code, so exec:pty is authorised over a specific folder, never node-wide), fetches a ticket with the Authorization header, and connects with ?ticket=. Route tests cover issue-auth, single-use replay rejection, and expiry.
…urfacing The DocumentsProvider now overrides createDocument, deleteDocument, and renameDocument, routing them to the existing CascadeNode.createDir/delete/ rename FFI verbs and re-querying so the new state is visible. CursorBuilder advertises FLAG_DIR_SUPPORTS_CREATE on directory rows and FLAG_SUPPORTS_DELETE plus FLAG_SUPPORTS_RENAME on rows, so the Files app surfaces the organise actions. An instrumented test round-trips createDocument -> renameDocument -> deleteDocument through DocumentsContract on the emulator. The openDocument write-back no longer fails silently into a bare thread's uncaught handler: it hands the caller the write end of a reliable pipe and, on an upload failure, records it with Log.e and calls closeWithError so the caller sees an IOException rather than assuming success (silent data loss).
DocumentsProvider.createDocument is (parentDocumentId, mimeType, displayName) in the framework contract, but the override declared it as (parentDocumentId, displayName, mimeType). All three parameters are String, so the override compiled, but at runtime the framework's mimeType landed in the displayName slot — so creating a directory produced a child document id built from the MIME type (/local/vnd.android.document/directory) instead of the display name, and the instrumented create→rename→delete round-trip failed on the emulator. Swap the two parameter names; the body already references them by name, so it now binds correctly.
move_entry hashed the destination unconditionally after the rename, but hash_file opens the target and reads it as a file — on a directory that is EISDIR, so renaming a directory failed with "Is a directory (os error 21)". The instrumented Android SAF round-trip surfaced it (renameDocument on a directory the provider had just created), but the same path is used by the CLI mv verb and the WebDAV/NFS rename, so any directory rename was broken. Hash only files; directories get an empty content hash, matching metadata and create_dir which never hash directories. Adds a unit test renaming a directory so this is covered without the emulator.
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.
Three follow-ups from the review of the prior UI-gap release, plus the Android organise-from-Files-app gap that was deferred. A workflow of Opus/Sonnet/Haiku agents did the initial implementation in parallel worktrees; the parallel run hit a subagent request-rate limit before its verify/review phases, so the work was salvaged from the worktrees, the half-finished TS side completed, and the full gate suite run directly.
Android SAF create/delete/rename (medium): the DocumentsProvider now overrides
createDocument,deleteDocument, andrenameDocument, routing them to the existingCascadeNode.createDir/delete/renameFFI verbs and re-querying so the new state is visible.CursorBuilderadvertisesFLAG_DIR_SUPPORTS_CREATEon directory rows andFLAG_SUPPORTS_DELETE/FLAG_SUPPORTS_RENAMEon rows, so the Files app surfaces the organise actions. An instrumented test round-trips createDocument -> renameDocument -> deleteDocument throughDocumentsContracton the emulator.Android write-back error surfacing (low, review #7):
openDocument's write-back ran on a bare thread and a failed upload threw into its uncaught handler while the caller had already assumed success — silent data loss, and the comment overclaimed it. It now hands the caller the write end of a reliable pipe and, on failure, records it withLog.eand callscloseWithErrorso the caller sees anIOException.Single-use exec-websocket ticket (low, review #8): the exec websocket took the base64 capability token as a
?token=query parameter (browsers cannot set WebSocket headers), putting a credential that grants remote code execution into access/proxy logs and history. A newPOST /v1/exec/ticketroute, authenticated through the normalAuthorizationheader, issues an opaque, unguessable, ~30s single-use ticket bound to the caller and folder; the websocket now accepts?ticket=&folder=, rejects replays/expired/unknown, marks the ticket used, and authorisesexec:ptyover the folder from the authority captured at issue time. The long-lived token is no longer accepted on the websocket. TerminalPage asks which folder to open the terminal in, fetches a ticket with the header, and connects with?ticket=. Route tests cover issue-auth, single-use replay rejection, and expiry.Gates green: workspace build/test/clippy (-D warnings), fmt, source-length, cargo doc (-D warnings), PWA lint+typecheck+test, the cascade-ffi iOS cross-compile, and the Android Robolectric unit tests. The instrumented SAF round-trip runs in CI's Android e2e job.