Skip to content

feat(files): in-dashboard file manager (Files destination) (#49) - #316

Open
bogdanpydev wants to merge 1 commit into
mainfrom
feat/49-file-manager
Open

feat(files): in-dashboard file manager (Files destination) (#49)#316
bogdanpydev wants to merge 1 commit into
mainfrom
feat/49-file-manager

Conversation

@bogdanpydev

Copy link
Copy Markdown
Collaborator

Closes #49.

Implements the Files dock destination specced in FILES.md — the last Tier-1 product-surface gap from NEXT.md. "Files are first-class" was true on disk from day one but had no in-product browse surface; this is the zero-setup answer: open the dashboard, see your folders, upload and download from any device with a browser. Full vertical slice in one PR.

What's in it

browser → web-ui → brain → host-agent (as the user's UID) → filesystem, for the v1 op set — list/browse, download, upload, new folder, rename, move, copy, delete — over the two roots home (/home/<user>/) and shared (/srv/malmo/shared/).

  • internal/hostagent/fileops (new leaf) — pure, identity-agnostic FS primitives (Resolve + containment, list/mkdir/move/copy/delete/open/save). Two consumers (the fake in-process, the real worker child as the user) justify the shared package. Errors are the plain fs.ErrNotExist/ErrExist/ErrPermission + syscall.ENOSPC sentinels, mapped with errors.Is at every layer.
  • host-agent — a consumer-side FileManager seam + /v1/files/* handlers (metadata Pattern A + streamed GET/PUT /v1/files/content, io.Copy, no whole-file buffering). FakeFileManager runs the ops in-process as the dev operator, so the Files destination works under make dev.
  • internal/hostagent/filemgr (Linux-only) — the load-bearing security piece (FILES.md # Execution, DECISIONS.md 2026-05-31). Every op runs in a child re-exec'd as the requesting user's UID/GID via exec.Cmd.SysProcAttr.Credentialnot in-process setresuid, which is per-OS-thread and unsafe under Go's M:N scheduler. Running as the user makes POSIX 0750/02770 the kernel-enforced backstop, gives created files correct ownership natively, and contains symlink attacks for free. Carries supplementary groups (for the 02770 malmo-shared tree), passes the op spec via env (owner+root-only /proc, unlike world-readable cmdline), frames downloads with an OK/ERR header so pre-stream errors stay typed, and round-trips the error class across the process boundary (classify/reconstruct). Wired in both build profiles with a //go:build !linux stub, mirroring pamverifier.
  • hostclient — Pattern A methods + streaming FilesOpen/FilesSave (first request-body-streaming method) and a typed, status-preserving FileOpError (bypasses do's error flattening, mirroring ResolveHome).
  • brain internal/api — huma metadata handlers + raw streamed content handlers. Resolves the session to a username (every op runs as the session owner — no cross-user browse, for any role), accepts only home/shared, rejects ../absolute before forwarding, and gates writes on the data-drive-missing health issue (409 blocked-by-health-issue carrying issue_id). File ops are not audited and do not trigger the elevation re-prompt (FILES.md # Audit & elevation).
  • web-ui — replaces the FilesView.vue stub with the full view (root switch, breadcrumbs, folder-first listing with per-row Download/Rename/Move/Copy/Delete, New-folder input, upload with browser-native progress, show-hidden toggle, inline delete confirm). Download is a same-origin <a download>; upload is an XMLHttpRequest PUT (the only transport that reports upload progress). A FileDestinationDialog folder picker backs move/copy across roots. Types generate from the brain's OpenAPI.

Testing

  • make check ✅ and make check-web (typecheck + production build) ✅.
  • fileops 88.7%; host-agent handlers, hostclient file methods, and brain handlers covered end-to-end over a real UNIX socket (the brain harness mounts a real hostagent.Agent + FakeFileManager), plus direct unit tests for the error-mapping arms and validation.
  • /code-review low self-review: no findings.

Known gaps / scope

  • Real UID-drop path is outer-loop-verified only. The filemgr fork/credential-drop can't run in CI (needs root + real Linux users + a /srv/malmo/shared tree); a booted-VM smoke of upload/download + a cross-user denial + a shared-group write is the acceptance step. The cross-platform surface (fileops, the worker op logic, the error round-trip) is CI-tested (filemgr 51.7%, the uncovered part being exactly the privileged fork).
  • disk-full health issue is not registered in builtinDefinitions(); the brain maps a host no-space to 507 standalone (the "linked to the disk-full issue" wording is aspirational until that issue lands — HEALTH.md-owned, out of scope).
  • OpenAPI advertises ErrorModel for the file routes, not the custom {code, message, issue_id} shape huma emits for a returned StatusError; the web-ui client tolerates both (a shared error-code registry is the BRAIN_UI_PROTOCOL.md follow-up).
  • No web-ui unit tests — the project has no web-ui test tooling; adding vitest is a new dependency + CI change beyond this issue. The view rides the vue-tsc typecheck + production-build gate; rigorous testing is on the Go layers.
  • v1 cuts stand as specced (FILES.md # deferred): thumbnails/preview, search, zip download, trash+undo, in-place editing, resumable uploads, sharing links, bulk USB/network import, and the audited admin break-glass into another user's files.

Implements the Files dock destination specced in FILES.md (closes #49):
browse, download, upload, new folder, rename, move, copy, delete over two
roots — home (/home/<user>/) and shared (/srv/malmo/shared/) — with every
op executed as the requesting user's UID.

- internal/hostagent/fileops: pure, identity-agnostic FS primitives plus
  lexical path containment, shared by the fake and the real UID-drop worker.
- host-agent: FileManager seam + /v1/files/* handlers (metadata Pattern A
  + streamed GET/PUT /v1/files/content); FakeFileManager for the dev loop.
- internal/hostagent/filemgr (Linux-only): each op runs in a child
  re-exec'd as the requesting user's UID/GID via SysProcAttr.Credential
  (not in-process setresuid), making POSIX 0750/02770 the kernel-enforced
  backstop; wired in both build profiles with a non-linux stub.
- hostclient: Pattern A methods + streaming FilesOpen/FilesSave and a
  status-preserving FileOpError.
- brain api: huma metadata handlers + raw streamed content handlers;
  session->username (no cross-user browse), root/traversal validation,
  writes gated on data-drive-missing (409), not audited / no elevation.
- web-ui: full FilesView (roots, breadcrumbs, per-row actions, XHR-progress
  upload, anchor download) + move/copy destination picker.

Docs: FILES.md progress entry, architecture.md, progress index.
@bogdanpydev bogdanpydev self-assigned this Jul 9, 2026
@bogdanpydev
bogdanpydev requested a review from onel July 9, 2026 07:31
@greptile-apps

greptile-apps Bot commented Jul 9, 2026

Copy link
Copy Markdown

Confidence Score: 4/5

The upload replacement path needs a fix before merging.

  • Failed uploads can replace an existing file with partial content.
  • The raw upload health-block response drops the health issue id.
  • Cross-device move cleanup can leave duplicate source and destination state after a cleanup error.
  • The auth and root validation paths look sound in the changed code.

internal/hostagent/fileops/fileops.go and internal/api/files.go

Important Files Changed

Filename Overview
internal/hostagent/fileops/fileops.go Adds the core filesystem primitives; upload replacement and cross-device move cleanup need follow-up.
internal/hostagent/filemgr/filemgr_linux.go Adds the Linux file manager that resolves users, carries supplementary groups, and re-execs workers with dropped credentials.
internal/hostagent/filemgr/worker_linux.go Adds the worker dispatcher and download framing for file operations.
internal/api/files.go Adds brain file metadata and raw content handlers; raw upload errors lose the health issue id.
internal/hostclient/files.go Adds status-preserving host-agent file calls and timeout-less streaming upload/download methods.
internal/hostagent/files.go Adds host-agent HTTP handlers and filesystem error mapping for file operations.
web-ui/src/views/FilesView.vue Replaces the Files stub with the full browsing and file action UI.
web-ui/src/useFiles.ts Adds the web file API helper layer for JSON metadata calls and XHR uploads.
web-ui/src/components/FileDestinationDialog.vue Adds the destination folder picker for move and copy operations.

Reviews (1): Last reviewed commit: "feat(files): in-dashboard file manager (..." | Re-trigger Greptile

// file (O_TRUNC — v1 has no resumable upload, so an interrupted transfer
// restarts). A short write from a full disk surfaces as syscall.ENOSPC.
func Save(abs string, r io.Reader) error {
out, err := os.OpenFile(abs, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Failed Upload Truncates Target

When PUT /api/v1/files/content replaces an existing file, this opens the final path with O_TRUNC before the upload stream has completed. If the client disconnects, the worker is interrupted, or the disk fills after some bytes are copied, the API returns an error but the original file has already been replaced by partial content.

Comment thread internal/api/files.go
status, code, msg := fileErrResponse(err)
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(status)
_ = json.NewEncoder(w).Encode(protocol.Error{Code: code, Message: msg})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Upload Health Issue Loses Link

When data-drive-missing blocks PUT /api/v1/files/content, blockedByHealth() creates a fileError with an IssueID, but this raw-handler error path rewrites it as protocol.Error{Code, Message}. The metadata write routes can return the health issue id, while upload callers only see {code,message} and cannot link the failure to the blocking health issue.

if cerr := Copy(fromAbs, toAbs); cerr != nil {
return cerr
}
return os.RemoveAll(fromAbs)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Cross-Device Move Leaves Duplicate

On an EXDEV move, the fallback copies the destination first and then removes the source tree. If cleanup fails after the copy succeeds, the operation returns an error but the target already exists, so a retry now hits the non-clobbering destination check and the dashboard can leave the user with a duplicate source and destination.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement in-dashboard file manager (FILES.md)

1 participant