Repository navigation
Drop Windows support - #439
Merged
Merged
Conversation
Specs a scoped bearer-token auth mechanism layered onto the existing get_current_user dependency, GUI token management, and a path-level Python client shipped inside the fileglancer package. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Thirteen TDD tasks: token storage and migration, scope table, bearer auth wiring, management endpoints, the Python client in four slices, the GUI page and dialogs, an E2E spec, and user docs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Wires bearer-token auth into the single get_current_user dependency all ~55 authenticated routes already use, so token auth covers the whole API without touching any route. Cookie/origin enforcement is untouched and skipped only when a bearer token is present. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds ApiTokenInfo/ApiTokenListResponse/ApiTokenCreateRequest/ ApiTokenCreateResponse models and GET/POST/DELETE /api/tokens routes so users can create, list, and revoke API tokens. These endpoints are session-only by omission from the auth.py scope table, so a token can never be used to mint or revoke another token. Also un-xfails test_a_token_cannot_mint_another_token from Task 3 (now a real 403) and adds three regression tests for previously-uncovered branches: empty-id and empty-secret malformed token parsing, and a pinned property that a bearer token never falls back to cookie auth.
Task 4 review fix round 1: ApiTokenCreateRequest now strips and rejects a whitespace-only name via a field_validator (previously length validation ran before the route's strip, letting " " through as a stored blank name). create_api_token now deduplicates scopes before storing them, so ["files:read", "files:read"] no longer stores as a repeated string.
Adds fileglancer.Fileglancer, the core of the Python client for Tasks 6-8 to build file/link/job methods on top of: construction from FILEGLANCER_URL/FILEGLANCER_TOKEN or explicit args, HTTP plumbing via httpx with server error bodies surfaced as FileglancerError, and client-side absolute-path <-> (file share, relative path) resolution mirroring resolvePathToFsp in pathHandling.ts.
Adds ls, stat, mkdir, rename, delete, read, write to Fileglancer, each taking an absolute filesystem path and resolving it via _resolve() before issuing the corresponding /api/files or /api/content request. Also fixes Fileglancer's constructor to wrap async-only transports (httpx.ASGITransport) in a small sync adapter, since httpx.Client is synchronous and ASGITransport only implements the async transport interface. This was required for the new tests, which drive the real app over ASGITransport with no server process.
Replaces the _SyncFromAsyncTransport shim (test-only scaffolding that had leaked into the shipped client) with fastapi.testclient.TestClient, which already wraps an ASGI app as a synchronous httpx.Client. Drops the now-unneeded transport parameter from Fileglancer.__init__.
GET /api/files omits the files key for non-directories, so ls() on a file previously returned [], indistinguishable from an empty directory. Now checks info.is_dir and raises FileglancerError first.
Adds create_data_link, data_links, data_link, delete_data_link, create_ng_link, ng_links, and delete_ng_link to Fileglancer, completing the create-link -> build-state -> shorten-link workflow. Every returned ProxiedPath has its path field rewritten to an absolute path.
Adds useApiTokensQuery/useCreateApiTokenMutation/useDeleteApiTokenMutation in frontend/src/queries/apiTokenQueries.ts, modeled on sshKeyQueries.ts, to back the upcoming token management UI (Task 9 of the API tokens plan). Also adds two resolvePathToFsp regression tests (prefix-boundary and longest-match cases) confirming the TS resolver's prefix guard matches the behaviour the Python client mirrors.
Review found the two resolvePathToFsp tests added alongside the API token query hooks were redundant with pre-existing coverage, and in one case did not actually exercise the prefix-boundary guard it claimed to test. Reverting pathHandling.test.ts to its exact pre-existing state; the apiTokenQueries module and its tests are unaffected.
Adds the user-facing API Tokens page (list, revoke, create-dialog with one-time secret display) consuming the completed apiTokenQueries hooks, registered unconditionally at /api-tokens and linked from the profile menu. Uses the existing FgDialog wrapper and design-system form atoms (FgFormField/FgInput/FgSelect/FgCheckbox) rather than raw Material Tailwind dialog/inputs, following SSHKeys/TempKeyDialog precedent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review round 1 fixes: reset the create-token mutation on close so a stale error/secret doesn't reappear on reopen, ignore close attempts while a create is in flight so cancel can't race the response into showing the secret dialog, add a RevokeTokenDialog confirmation step before a token is actually revoked, and scope the revoke-pending state to the specific token being revoked instead of disabling every card. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Covers the full user journey on the API Tokens page: create a token, confirm the secret is shown exactly once and never appears again, then revoke it through the confirmation dialog. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
api_tokens is per-test-user state of the same class as user_preferences but was missing from cleanDatabase's table list, letting tokens leak across local E2E runs (the webServer config reuses a fixed sqlite path). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Whitespace-only reformat (multi-line locator chains, wrapped tables array); no logic changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Directly prove the confirmation gate's property: clicking the card's Revoke button opens the dialog but does not itself remove the token, before the Revoke Token click confirms and removes it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
/nearline/... is a real site storage-tier name (checked for in StartTour.tsx), not a generic example. Swap for /data/alice/... in the user-facing help(fileglancer.client) docstring. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six fixes from the whole-branch review before merge: - Serialize API token timestamps with a timezone (_ensure_utc, now module-level) so the frontend "Expired" badge isn't wrong outside UTC; ApiTokenCard now reuses formatDateOnly instead of a duplicate local helper. - Note in the create-token dialog and docs that jobs:write runs code on the user's behalf and therefore has the same file access they do, not the containment the scope list implies. - Drop /api/auth/status and /api/cluster-defaults from the scope tables in auth.py - both are unauthenticated routes with no Depends(get_current_user), so required_scope is never consulted for them. - Correct a test_client_paths.py docstring that claimed a TypeScript fixture-sharing arrangement that doesn't exist. - _absolutize in client.py now returns a data link unmodified (path left FSP-relative) when its file share has disappeared from file_share_paths, instead of raising and breaking the whole listing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each scope checkbox now states what it allows, folded into the label so the description is associated with the input for screen readers rather than merely adjacent to it. Descriptions are a total Record over ApiScope, so adding a scope without describing it fails to compile. Selecting files:write or jobs:write surfaces a plain-English warning that the token is password-equivalent: whoever holds it acts as the user, and jobs:write in particular runs arbitrary code and so reaches every file regardless of the file scopes granted. jobs:read is described as reaching full job detail, parameters, environment, and log files, which is what the /api/jobs prefix actually grants -- "List jobs" understated it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Widens the create dialog to max-w-4xl (twice max-w-md) for the added text, and lays the scopes out as a single two-column grid: scope name, then a greyed description. One grid spanning every row rather than a grid per row is what aligns the second column, since a CSS grid column is as wide as its widest cell across all rows -- no hard-coded width. Checkboxes are top-aligned so a wrapping description does not drag them down, and the em-dashes are gone now that the columns carry the separation. Selecting a :write scope now checks its :read counterpart and locks it, replacing the "A :write scope also grants :read" note: the server grants read implicitly to any write scope, so an unchecked read box was showing a state the server would not honour. Deselecting write releases the lock and leaves read granted. Also fixes a real bug in FgCheckbox: h-4 w-4 is a flex basis, not a floor, so a flex parent shrank the box to fit a long label -- which rendered the boxes at visibly different sizes across labels of differing length. Adding shrink-0 fixes it for every consumer, not just this dialog. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Steps through every public Fileglancer client method, printing what it is about to do and the exact call before running it, then pausing so the result can be verified in the web UI or on disk. Enter runs a step, s skips, q quits. Everything is created inside one scratch directory under the cwd and removed in a finally block, including data links and Neuroglancer links, so quitting partway through or a failing step still cleans up. Includes the error paths as deliberate steps -- an unresolvable path, ls() on a file, a cross-share rename, a missing job -- since those messages are what a user actually meets when something goes wrong. Verified end to end against a live server with an all-scopes token: 24 steps, no residue. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_resolve rendered "Available mount points:" with nothing after it when the share list was empty, which reads as though the caller's path were at fault rather than the server having nothing mounted. Reported from a real setup with file_share_mounts: [] in config.yaml. Now distinguishes the two cases and names the setting to change. The walkthrough script checks the same condition up front so it can print the config snippet, and its unreachable-server message mentions the scheme, since a server started with --ssl-keyfile needs https://. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A real deployment mounts hundreds of shares, so listing them all buried the one line the caller needed to read. The message now names the offending path and points at file_share_paths() for discovery, which is a fixed ~100 characters regardless of how many shares exist. The empty-share-list case keeps its own distinct message, since there the server configuration really is the thing to look at. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Appends an Amendments section rather than editing the approved design, so the document records what was decided on 2026-08-24 and, separately, where the shipped code deliberately went elsewhere. Chief among them: the no-match resolution error no longer enumerates the mount points. The spec called that message load-bearing, which was right -- but a real deployment mounts hundreds of shares, so listing them buried the line the caller needed. The spec's intent is what motivated the change. Also records the dropped cross-language resolver fixture, the two dead scope-table rows, the untouched TableCard DataType, the UTC offset on token timestamps, and jobs:write being full-account access. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds api_token_scopes, defaulting to every scope except files:write and jobs:write. Both amount to full access to the user's files -- jobs:write runs arbitrary code as the user, so it reaches every file regardless of the files:* scopes granted -- so an admin now opts into them per server rather than inheriting them. Enforced in two places, which is what makes the setting a control rather than a formality: POST /api/tokens refuses a disabled scope, and token verification intersects each token's granted scopes with the enabled set, so removing a scope revokes it from tokens that already hold it instead of only restricting new ones. A files:write token degrades to files:read. The two 403s are worded differently on purpose. A missing scope is the user's to fix by minting a new token; a scope the server does not support is not, so that message names the administrator. An unknown scope name in the config fails at startup rather than silently narrowing what users can mint. The creation dialog offers only supported scopes, constrains its selection to them so a stale default cannot be submitted, and says to contact an admin when any are withheld. Cookie sessions are unaffected: the setting governs tokens only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A failed DELETE left the confirmation dialog open with no explanation. Nothing implied success -- the dialog stays open and the token remains listed, since invalidateQueries only runs on success -- but isPending cleared and the button re-enabled, so clicking again failed identically with no clue why: a 404 because someone else revoked it, a 403 from an expired session, a dropped connection. The dialog now renders the error, matching what CreateTokenDialog already does. Closing it resets the mutation, or a failure on one token would greet the user when they opened the dialog for a different one -- the same stale error bug already fixed on the create side. Closing is ignored while a revoke is in flight, so the outcome is always shown. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ests Windows CI caught two things. abspath joined a '/'-separated relative path onto mount_path verbatim, so a Windows mount path came back with mixed separators: 'C:\shares\data/sub/file.txt'. _resolve already normalizes backslashes to '/' on the way in; abspath now does the same on the way out, so the round trip is self-consistent and the documented "always in Linux form" claim holds. No effect on POSIX, where mount_path has no backslashes. The remaining failures were the tests, not the code: they compared client output against os.path.join output literally. Windows spells one directory several ways -- separator style, 8.3 short names (RUNNER~1 vs runneradmin), and case -- so a shared conftest.same_path helper normalizes both sides with normcase plus realpath before comparing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AccessLogMiddleware resolved identity from the session cookie only, so every programmatic request logged '-'. An admin investigating a misbehaving script had nothing to trace it to. get_user_from_token now leaves the resolved username and token id on request.state, which the middleware reads after awaiting call_next -- request.state is backed by scope["state"], so a value set in a dependency is visible upstream. Lines become: 127.0.0.1:37662 [rokickik fgt:4e626080362d] "GET /api/files/... " 200 The token id is the public half, not the secret, and is what the GUI shows, so it points at the exact token to revoke. The pre-existing cookie lookup is deliberately left in place rather than replaced by the same mechanism, even though it duplicates a query the dependency makes moments later: logout deletes the session during the request, so resolving identity only after call_next would log '-' for the request that did the logging out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
secondary is the purple brand accent (#6D28D9, commented "Purple color" in tailwind.config.js), not a muted grey. Using text-secondary for scope descriptions, card metadata and the empty state turned most of the page purple -- including the scope description column, which was supposed to be "slightly greyed". Switched to text-foreground/60, the muted-text idiom already used across the app, and dropped color="secondary" from the empty-state icon in favour of an explicit text-foreground/40 so it reads as decorative rather than inheriting by accident. The Apps pages use plain text-foreground throughout and never reach for secondary; this brings the tokens page in line. Page palette is now text-foreground for content, /60 muted, /40 decorative, and error/warning only where they carry meaning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The $FILEGLANCER_URL form in the client docstrings read like string interpolation to some people. Say "the FILEGLANCER_URL environment variable" instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Neuroglancer shows a folder tree over plain https by reading the first
path segment of a URL as an S3 bucket and asking that bucket root for
everything under the rest as a prefix:
GET /files/?list-type=2&prefix={sharing_key}/{link name}/&delimiter=/
Nothing answered at /files/, so the probe 404'd and browsing was off for
every data link. Reads were unaffected, which is why this only showed up
as "the folder tree doesn't work".
The whole /files space is now one bucket. A key is
{sharing_key}/{link name}/{path}, which is exactly what a share URL
already looks like, so existing links gain browsing without changing.
Since the first two segments have no counterpart on disk, they are
handed to x2s3 1.5.1's new virtual_prefix option, which strips them from
requests and puts them back on the keys it reports.
Sharing keys are secret, so the bucket root never enumerates them: a
missing, partial or unknown key all produce the same empty listing, and
the only key that can appear in a response is one the caller already
supplied in full. Empty rather than an error is deliberate -- Neuroglancer
remembers per-server whether S3 listing works at all, and an error there
would turn off browsing for the valid links too.
The existing per-key list branch is fixed the same way. It reported keys
without the link name, so they didn't resolve as URLs, and it 404'd at
the bucket root because a bucket has no path of its own to match.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
x2s3 reported a space in a key as '+', which no client turns back into a space -- boto3 unquotes and Neuroglancer calls decodeURIComponent -- so a key taken from a listing named a file that did not exist. Any file or directory with a space in its name listed but would not open. Fixed in x2s3 1.5.2, along with max-keys and continuation tokens being ignored inside a virtual prefix. Covered by a test that lists a link whose name and contents both contain spaces, for the stored and the decoded form of the link name, and reads each reported key back to prove it resolves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Response times were only ever in the text access log, where they could not be aggregated. Setting log_format: json (or FGC_LOG_FORMAT=json) now writes one Elastic Common Schema object per line to stdout, which a shipper can parse straight off the container's logs, so p95/p99 can be charted from event.duration and broken down by endpoint, user or status code. Text remains the default and stays human-readable. The access log middleware keeps emitting the same line and additionally binds the same facts as ECS fields, which text mode ignores. Each request also gets an id, returned in the x-request-id header and bound to every line logged while serving it, so a bug report quoting the id finds the whole request. Output configuration lives in a new fileglancer.logconf, kept free of Fileglancer imports so the setuid user worker can configure its logging without pulling in the server's auth stack. Workers inherit the format over FGC_LOG_FORMAT. event.duration here is time-to-first-byte, not transfer time, because BaseHTTPMiddleware returns as soon as the response starts. That is fine for the API; x2s3 serves the bulk data and does measure full transfer time. Charting download latency would mean rewriting the middleware as pure ASGI, the way x2s3's is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fileglancer is a POSIX server: the worker pool uses fork/setuid/SCM_RIGHTS, the job poll loop uses fcntl locking, and file permissions come from pwd/grp. It cannot run on native Windows, so the Windows CI leg only ever proved that the code imported there. Windows users run under WSL2, which is linux-64. Removes the windows-latest matrix leg, the win-64 pixi platform, the try/except ImportError shims around pwd/grp/fcntl, the Windows drive-letter path pattern, the ntpath job-path helpers, and 14 win32 test skips. Keeps client.py's backslash handling: FileSharePath.windows_path is a real UNC field users paste from a Windows desktop, independent of the client's OS. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
3a61c42 overwrote line 1 of cli.py, replacing `#!/usr/bin/env python3` with `#leaned up CLI auto-login session` — a commit-message fragment from a botched edit. The shebang was vestigial (the file is not executable and the CLI runs via the `fileglancer` console script), so drop the line rather than restore it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Stacked on #437.
Why
Fileglancer can't run on native Windows, and it isn't meant to. The worker pool is built on
fork+setuid+SCM_RIGHTSoverAF_UNIXsockets, the job poll loop usesfcntlfile locking, and file permissions are resolved throughpwd/grp. None of those exist on Windows. The generated job scripts are POSIX bash destined for Linux compute nodes.So the
windows-latestCI leg was never testing that Fileglancer works on Windows — it was testing that the modules import there. It produced no artifact (packaging and upload have always been Linux-only), and it never invoked the CLI at all.This isn't a loss for anyone. Windows users run Fileglancer under WSL2, which is
linux-64— the same platform we already build, test, and ship. That's the supported path, and it stays exactly as it is.What this saves
pixi run dev-install(solve + install of the whole conda environment) plus the frontend and backend suites on every PR and every push to main.e4a70084("port Windows job-file-path fix") and4dc6efa8("normalize separators in abspath") are both examples already in this stack.win-64removes 2858 lines frompixi.lockwith zero additions, so no version churn to review — and every futurepixi lockhas one less platform to solve.What's removed
CI —
windows-latestfrom the build matrix. Thebuildjob is now plainruns-on: ubuntu-latest, which also drops fourif: matrix.os == 'ubuntu-latest'guards and theshell: bashlines that only existed soset -euxworked on Windows.Platform —
win-64from[tool.pixi.workspace] platforms, withpixi.lockregenerated.POSIX import shims — six
try: import pwd/grp/fcntl … except ImportError: … = Noneblocks inserver.py,filestore.py,sshkeys.py,user_worker.py,worker_pool.py,apps/jobs.py, andapps/command.py. Nothing downstream checked forNoneexcept two dead guards incommand.py. They actively made things worse: instead of a clearImportErrorat startup, an unsupported platform got anAttributeErrordeep inside a request.Windows path handling in the apps subsystem — the
_WINDOWS_DRIVE_PATTERN(^[a-zA-Z]:/) that classifiedC:/…as absolute when composing apptainer bind flags, thentpath-based_join_stored_path/_stored_path_basename/_stored_work_dir_pathhelpers inapps/jobfiles.py(now plainposixpath, and_stored_work_dir_pathturned out to duplicate the existing_resolve_work_dir), and threepath_value.replace("\\", "/")calls inexpand_user_path,validate_path_for_shell, andvalidate_path_in_filestore.That last one was a small latent bug, not just dead weight: a backslash is a legal character in a POSIX filename, so those calls silently rewrote valid paths —
/data\subbecame/data/sub, and a test asserted it. Worth noting for review that removing them opens no injection surface: every path value reaches the generated script throughshlex.quote, which single-quotes, so a backslash was already inert there.Test skips — 14
skipif(sys.platform == "win32")markers acrosstest_apps.py,test_worker.py, andtest_filestore.py, plus therequires_pwd/requires_bashaliases and three now-unusedimport sys.What's deliberately kept
client.py's backslash normalization. This looks like the same pattern but isn't.FileSharePathhas a realwindows_pathfield (\\store\nearline) that users paste from a Windows desktop, so_resolvehas to accept UNC input no matter what OS the client itself runs on. Untouched — only a stale docstring intest_client_paths.pyclaiming Windows CI covered it was corrected.The
cursesImportError guard indebug.py. Five lines that turn a traceback intofileglancer debug requires a POSIX terminal (curses). Cheap, and still useful in any stripped-down environment.Drive-by
One unrelated one-line fix rides along in its own commit.
3a61c428overwrote line 1 ofcli.py, replacing the#!/usr/bin/env python3shebang with#leaned up CLI auto-login session— a commit-message fragment from a botched edit. The shebang was vestigial (the file isn't executable and the CLI runs through thefileglancerconsole script), so the line is simply dropped.pixi run fileglancer --helpstill works.Testing
pixi run -e test test-backend -- tests/→ 929 passed.Net: 36 insertions, 3018 deletions across 16 files.
@StephanPreibisch @JaneliaSciComp/fileglancer