fix(auth): compare presented credentials as bytes, not str

[tang-vu]

Summary

secrets.compare_digest / hmac.compare_digest raise TypeError when either str argument holds non-ASCII characters. Every API-key check in the repo compared the caller-supplied token as a str, so an unauthenticated client can send a non-ASCII Authorization: Bearer token, X-API-Key header, or ods-session cookie and get an unhandled 500 with a stack trace instead of a clean 401/403.

Authorization and Cookie headers are decoded as latin-1, so any byte 0x80–0xFF reaches these comparisons straight off the network.

Reproduced against the real modules (not a mock):

=== dashboard-api verify_api_key ===        before fix      after fix
  valid key                                 HTTP 200        HTTP 200
  wrong ASCII key                           HTTP 403        HTTP 403
  non-ASCII key (0xe9)                      HTTP 500   ->   HTTP 403
  UTF-8 emoji token                         HTTP 500   ->   HTTP 403

=== session_signer.verify (docstring: "never raises") ===
  valid cookie                              (True, 'ok')
  tampered ASCII sig                        (False, 'bad-signature')
  non-ASCII sig                             RAISED TypeError  ->  (False, 'bad-signature')

session_signer.verify() documents that it returns a reason and never raises. A non-ASCII cookie signature broke that contract on exactly the forward_auth path Caddy calls (GET /api/auth/verify-session).

This is not an auth bypass. forward_auth treats a 500 as non-2xx and still denies the request, and a wrong key never compares equal. The defect is an unhandled exception on unauthenticated paths: wrong status code, stack traces in logs, and a broken documented contract.

Why this shape of fix

privacy-shield already carried the byte-wise form in _token_valid(), whose docstring says it exists so that

a non-ASCII presented token is a clean mismatch rather than a TypeError on the pre-auth path. Same secret + secrets.compare_digest as verify_api_key / _is_authenticated.

…but verify_api_key() in that same file never used it. The defect class was already recognised here and just applied incompletely. This PR routes privacy-shield through its own helper and applies the same byte-wise compare at the six remaining call sites:

File	Credential source
dashboard-api/security.py	Authorization: Bearer
dashboard-api/session_signer.py	ods-session cookie signature
privacy-shield/proxy.py	Authorization: Bearer (now via _token_valid)
token-spy/main.py	Authorization: Bearer
ape/main.py	X-API-Key
bin/ods-host-agent.py	Authorization: Bearer
extensions/library/services/open-interpreter/server.py	Authorization: Bearer

Keys are encoded at call time rather than precomputed at import, because tests/test_security.py monkeypatches security.DASHBOARD_API_KEY.

Strings decoded as latin-1 cannot contain surrogates, so the added .encode("utf-8") cannot itself raise — verified.

AI Assistance

Claude Code was used to grep for the defect class across services, write the reproduction script, draft the fix and the regression tests, and run the validation commands below. I read the final diff, confirmed the reproduction before and after, and confirmed the three pre-existing failures listed below are unrelated to this change.

Release Lane

• Stable hotfix targeting release/2.5.x
• Mainline change targeting main
• Next-minor work targeting the next feature/minor release
• Not sure; reviewer should help classify

Stable hotfix reason:

n/a — mainline.

Changed Surface

• Docs only
• Tests only
• Dashboard UI
• Dashboard API / host agent
• Installer / bootstrap / lifecycle
• Docker Compose / service manifests
• Model routing / Hermes / capabilities
• Network exposure / auth / proxy
• Dependencies / runtime wiring

Risk And Validation

• Risk level: High — docs/HIGH_RISK_CHANGE_MAP.md maps "Dashboard API auth/setup/host-agent" and "Network binding/proxy routes" to High, requiring focused pytest, security tests, dashboard API smoke.
• Validation run:
  • git diff --check
  • Markdown/link sanity for docs
  • Focused tests listed below
  • Dashboard lint/test/build
  • Extension audit / compose validation
  • Release-grade fleet or scoped hardware validation
  • Stable-lane patch validation, if targeting release/2.5.x
  • Not required because: no installer, compose, manifest, routing, or host-mutation surface is touched — the diff is seven credential comparisons and their tests.

Commands/results:

# Regression proof — the new tests fail on the OLD code with the exact defect.
# (source files stashed, new tests kept)
$ pytest tests/test_security.py tests/test_session_signer.py -q
  E  TypeError: comparing strings with non-ASCII characters is not supported
  session_signer.py:153: TypeError
  4 failed, 23 passed

$ pytest tests/test_proxy_auth.py -q          # privacy-shield
  E  TypeError: comparing strings with non-ASCII characters is not supported
  proxy.py:92: TypeError
  1 failed, 5 passed

# With the fix applied
dashboard-api  : pytest tests/ -q                    -> 1174 passed, 9 skipped, 3 failed
privacy-shield : pytest tests/test_proxy_auth.py -q  -> 6 passed
ape            : pytest tests/ -q                    -> 44 passed
token-spy      : pytest tests/ -q                    -> 3 passed

# The 3 dashboard-api failures are pre-existing on a clean tree (verified by
# stashing every change and re-running). They are Windows-environment issues,
# not regressions from this PR:
#   test_setup_sentinel.py::test_setup_test_emits_pass_sentinel_on_success
#   test_setup_sentinel.py::test_setup_test_emits_fail_sentinel_with_returncode_on_failure
#   test_user_extensions.py::TestScanUserExtensions::test_scan_symlink_skipped

# Lint (make lint equivalent — `make` is unavailable on this host)
bash -n on all 269 shell files                                  -> 0 failures
python -m py_compile main.py agent_monitor.py                   -> OK
ruff check ods/ --select E,F,W --ignore E501,E701,E731,E741,E402 -> All checks passed!
git diff --check                                                -> clean

Operational Change Check

• This is not an operational change.
• This is an operational change and validation is recorded above.
• This is an operational change and validation is intentionally deferred for:

Narrower equivalent, rather than a full fleet run: the change cannot affect install, bootstrap, compose generation, lifecycle, model download, or host mutation — it only replaces str operands with their UTF-8 bytes inside seven credential comparisons. Behaviour for a valid key and for a wrong ASCII key is unchanged and is covered by the pre-existing tests in each suite. The only behaviour that changes is non-ASCII input, which previously crashed.

Notes For Reviewers

• Rollback: revert the commit. No migration, no config, no schema, no persisted state.
• Constant-time property preserved: compare_digest on bytes is the documented constant-time path, and nothing short-circuits before it.
• token-spy and open-interpreter have no auth test suite (token-spy/tests/ only covers usage reporting; open-interpreter has no tests/), so those two sites are fixed without a new test rather than growing a suite in this PR. Happy to add one if you would prefer.
• ape/main.py returns 401 (not 403) for a wrong key. I preserved each service's existing status code rather than unifying them here.

[Lightheartdevs]

Thanks for tightening the auth preflight paths here; this catches a real unauthenticated error path without changing successful auth semantics.

Advisory status: looks merge-ready

What looks good:

• The fix applies the same bytes-based compare_digest pattern across the credential-bearing surfaces touched by the PR: dashboard-api, session cookie signing, privacy-shield, token-spy, APE, host-agent, and open-interpreter.
• The existing privacy-shield helper is reused instead of adding a second auth path, and dashboard/session/privacy tests cover the previous non-ASCII TypeError behavior directly.
• CI is green on the relevant repo gates, including dashboard API tests, Python lint/type check, shell/PowerShell lint, distro smoke, env validation, and secret scanning.

What I checked:

• Reviewed the PR body, existing discussion, commit, and diff for e6f9bc114871c0098c25b04d361184d9c843b304.
• Static-reviewed the changed auth call sites and scanned pr-1703 for remaining secrets.compare_digest / hmac.compare_digest usage.
• Ran git diff --check origin/main...pr-1703; it was clean.
• Checked gh pr checks 1703; all substantive checks are passing, with only optional review/security-notice jobs skipped.

Thanks again for pushing this forward.