feat(web_server): dashboard fs-allowlist hardening

[Thelightthatshines]

Fixes CRITICAL Finding 1 from security-review-2026-09-02: authenticated dashboard users could read arbitrary host files via /api/fs/* endpoints.

Adds HERMES_DASH_FS_ALLOW env var (opt-in path scoping) and _FS_DENY_BASENAMES (hardcoded secret file deny list). Backward compatible when env is unset.

7 new tests added, all passing.

[andrexibiza]

Reviewed the exact current object: head 08b5eeff5ffedffd145c1d336ec7a9936b323dc0 against base 8e4366d358bd93fd799fc22082e10ede69a21c6f, including both changed files, the existing /api/fs/* call graph, current repo policy, exact-head Actions state, and the adjacent security PR/issue cluster.

The instinct here is good: putting path containment at a shared choke point is directionally stronger than bolting a different root check onto every endpoint, and the symlink-escape witness is exactly the kind of regression a filesystem security boundary needs. I would preserve that work. I cannot call this merge-safe at this head yet, though; there are several class-level blockers.

Blocker 1 — the claimed critical read fix is fail-open by default

_fs_enforce_scope() returns immediately when HERMES_DASH_FS_ALLOW is unset/empty:

roots = _fs_allowed_roots()
if not roots:
    return candidate

The new tests deliberately pin that behavior in both test_fs_enforce_scope_allows_path_when_env_unset and test_fs_enforce_scope_empty_env_is_unrestricted.

That means the normal/default dashboard remains unrestricted, while the PR description says this fixes authenticated arbitrary host-file reads. On this base, /api/fs/read-text and /api/fs/read-data-url still have no _is_sensitive_path() guard. So the security postcondition is only true after an operator opts into a new setting; the vulnerable default remains the default.

This needs to be secure by construction, not opt-in. Preserve ordinary Files/editor usability with a safe default root/policy, but an unset configuration cannot mean “the critical finding is still present.” At minimum, the sensitive-file guard must remain authoritative regardless of root configuration.

Blocker 2 — the unset path regresses canonicalization and creates a new sensitive-download symlink bypass

Before this patch, _fs_path() always returned candidate.resolve(strict=False). At this head, the unset/empty branch above returns the unresolved candidate instead.

That is load-bearing for the already-guarded sibling /api/fs/download: _fs_regular_file() stats the path (following the symlink), then fs_download() asks _is_sensitive_path(target) about the path it was given. _is_sensitive_path() classifies the basename/path components; it does not independently canonicalize the symlink target.

So under the new default/unset behavior, a non-sensitive alias such as workspace/public-link -> ~/.hermes/.env can reach FileResponse while the guard sees public-link rather than .env. The old _fs_path() resolved that alias to .env before the guard and rejected it. The new symlink test does not catch this because it only exercises the configured-root branch, where resolution still happens.

Please canonicalize unconditionally first, then apply containment/classification. Add the inverse regression: env unset + /api/fs/download + benignly named symlink to a sensitive target => 403. This should be a class invariant, not just a root-escape test.

Blocker 3 — this creates a second, narrower secret-policy owner instead of composing with the existing one

_FS_DENY_BASENAMES duplicates part of the existing _is_sensitive_path() policy, but is substantially narrower and only runs when roots are configured. That creates two authorities for “what is sensitive,” with different activation rules.

The adjacent graph makes the drift concrete:

• #95303 / #95317: the missing sensitive guard on the two preview/read endpoints.
• #95306 / #95339 / #95344: the write side of the same filesystem-authority shape; #95344 also carries a stronger root-sandbox design for the HTTP write surface.
• #95356 and #98219: broaden the canonical sensitive classification to standard credential trees/dotfiles that a basename-only list cannot safely cover.

This PR should compose with the canonical _is_sensitive_path() owner after canonical resolution, not fork a private denylist. Please reconcile the competing/adjacent branches explicitly before merge. Preserve contributor history from those PRs rather than reimplementing their work here: #95317/#95339 are liuhao1024’s read/write guard work; #95344/#95356 are Finn763’s write-sandbox/credential-tree work; #98219 is loulanyue’s canonical-denylist parity work. They are not all duplicates of this PR, and this PR does not supersede them as written.

A sensible ownership graph is: one canonical path-resolution/containment policy; one canonical sensitive-path classifier; read/write/git consumers use those owners. Decide which write branch owns #95306 (#95344 is materially broader than #95339), then rebase the surviving work rather than letting parallel policy layers land independently.

Blocker 4 — HERMES_DASH_FS_ALLOW violates the current repository config contract

Current AGENTS.md explicitly rejects new HERMES_* variables for non-secret behavioral configuration: behavior belongs in config.yaml; an internal env bridge is acceptable, but the user-facing setting is not.

Filesystem roots are behavioral policy, not a secret. This therefore needs a config.yaml-owned setting (with the corresponding default/example/docs/schema path) rather than a new operator-facing env var. This also matters for profile/config ownership: a process-global environment answer is a poor long-term authority for a dashboard that already serves profile- and connection-scoped surfaces.

Blocker 5 — the mutation surface is wider than the proof surface

Changing _fs_path() is not a read-only change. That helper is currently consumed by:

• /api/fs/list
• /api/fs/read-text
• /api/fs/write-text
• /api/fs/read-data-url
• /api/fs/download
• /api/fs/git-root
• _git_path(), which feeds the mounted Git router

The seven new tests exercise read-text only. Once containment is enabled, this PR changes list, mutation, download, preview, and Git behavior as well. Security proof needs to match mutation scope.

Please add cross-surface contract coverage: allowed + rejected list/read/read-data-url/download, pre-effect write refusal (prove no temp/target mutation outside policy), allowed write, and Git-root/router containment. Also verify the local Desktop/Electron path remains intentionally compatible rather than accidentally relying on the HTTP contract.

Architecture / ownership gate

The new policy is being added directly into hermes_cli/web_server.py, an existing godfile (this hunk itself starts around line 2409, and the file is far larger). This repository is already actively extracting web_server.py slices (#84835, #79129 and the broader godfile work). A filesystem authority boundary should have a focused sub-2K owner with a thin compatibility seam in web_server.py, not become another policy cluster inside the godfile. That will also make the read/write/Git consumers share one testable contract without circular drift.

Exact-head release gate

The exact reviewed head is not green in hosted CI. The current exact-head runs are all terminal action_required, not success:

• CI: https://github.com/NousResearch/hermes-agent/actions/runs/33642063364
• Docker Build, Test, and Publish: https://github.com/NousResearch/hermes-agent/actions/runs/33642061793
• Nix flake check: https://github.com/NousResearch/hermes-agent/actions/runs/33642061738

Those may simply be fork/approval gates rather than code failures, but they are still not green receipts for this commit. No merge conclusion should be drawn until the exact head has settled green.

Interlock / closure

The PR currently cites an unnamed security-review-2026-09-02 finding but does not interlock the live public defect class. Please bind the final shape to #95303 (read) and #95306 (write) and state which of #95317/#95339/#95344/#95356/#98219 are complementary, superseded, or being consolidated. Right now there is unresolved file/policy collision across the same web_server.py filesystem surface, and merge order changes semantics.

The root-containment direction and symlink witness are worth keeping. Once canonicalization is unconditional, the default fails closed, the existing sensitive classifier is the single authority, config ownership matches repo policy, the whole consumer surface is tested, the godfile ownership is fixed, and the exact head is green, this becomes a much stronger security repair rather than an opt-in mitigation with a new default bypass.