Skip to content

fix(desktop): authenticate gated file downloads for password remotes - #89013

Closed
686f6c61 wants to merge 2 commits into
NousResearch:mainfrom
686f6c61:fix/88987-basic-auth-download
Closed

fix(desktop): authenticate gated file downloads for password remotes#89013
686f6c61 wants to merge 2 commits into
NousResearch:mainfrom
686f6c61:fix/88987-basic-auth-download

Conversation

@686f6c61

Copy link
Copy Markdown
Contributor

What does this PR do?

A remote gateway with auth_required: true is classified as oauth. File save/read then used only the OAuth session partition (persist:hermes-remote-oauth). Password-only (dashboard.basic_auth) logins set the session cookie via POST /auth/password-login, often in the default session — so /api/fs/download and /api/fs/read-data-url 401'd while a browser cookie request succeeded. Native bearer login had the same hole on the streaming download path.

Gated downloads and cookie REST now: prefer the native bearer, then the OAuth partition, then (password-only) the default session.

Related Issue

Fixes #88987

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Security fix
  • Documentation update
  • Tests (adding or improving test coverage)
  • Refactor (no behavior change)
  • New skill (bundled or hub)

Changes Made

  • native-auth-decisions.ts: gatedCookieSessionOrder()
  • main.ts: downloadGatedToFile / fetchJsonViaGatedCookies used by saveGatewayFile, hermes:api, and fetchJsonForBackend
  • Tests for the cookie-jar order

Not runtime-tested against a live password-gated remote.

How to Test

cd apps/desktop
vitest run --project electron electron/native-auth-decisions.test.ts
# 15 passed

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Ubuntu Linux

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

N/A (no UI change)

auth_required maps to oauth mode, so save/read used the OAuth cookie
partition. Password-only gateways store the session cookie in the
default jar, and native login uses a bearer. Try the native bearer, the
OAuth partition, then the default session.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) area/auth Authentication, OAuth, credential pools sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Aug 18, 2026
gatewayWsUrlIpcResult must precede isGatewayAuthRejection.
@OutThisLife

Copy link
Copy Markdown
Collaborator

Superseded by #90546.

That salvage keeps the auth fix (gated downloads have to present credentials), rebased onto current main, and:

  • reuses resolveOauthRestAuth via resolveGatedDownloadAuth — same bearer-then-cookie ladder as hermes:api
  • drops the default-session cookie-jar walk and REST 401 retries
  • covers the decision with a behavior test

You're credited via Co-authored-by. Thanks for tracing the password-remote 401.

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

Labels

area/auth Authentication, OAuth, credential pools comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Desktop: attachment download fails (401) on remote gateway with password (basic) auth — "Couldn't fetch X from the gateway"

3 participants