fix(photon): auth & setup cluster — stale tokens, secret rotation, diagnostics, atomic auth.json (salvage x6) - #73562
Merged
Merged
Conversation
alt-glitch
added
type/bug
Contributor
૮ >ﻌ< ა ci reviewran on af1d812 ℹ️ InfoDesktop E2E visual evidence · View test artifacts · View job1 visual diff. inline evidence upload failed. Failed to upload diff-665a0833239e-onboarding-overlay-diff.png with gh image (exit code 1): Error uploading /home/runner/work/_temp/e2e-evidence/diff-665a0833239e-onboarding-overlay-diff.png: step 0 (get upload token): uploadToken not found on repo page — do you have write access to NousResearch/hermes-agent? (or, if NousResearch enforces SAML SSO, authorize at https://github.com/orgs/NousResearch/sso) |
teknium1
force-pushed
the
photon/auth-cluster
branch
from
July 28, 2026 19:31
35c463f to
3068278
Compare
_save_auth() wrote the bearer token with tmp.open('w') — created at
process umask (typically 0o644) — and only chmod'ed to 0o600 after the
write, leaving a window where the token sat world-readable. The temp
name was also fixed and predictable (auth.json.tmp), so it could be
pre-planted (symlink attack).
Create the temp file with os.open(O_WRONLY|O_CREAT|O_EXCL, 0o600) and a
per-process random suffix, fsync before the atomic replace, and clean
the temp file up on failure. Mirrors hermes_cli/auth.py:_save_auth_store
(#19673, #21148), which hardened the same pattern in the core writer.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review follow-up: if os.fdopen() raised before taking ownership of the descriptor returned by os.open(), the cleanup handler unlinked the temp file but leaked the fd. Close it explicitly on that path, mirroring the credential-writer cleanup from #62837. Strengthen the tests so the old writer could not pass them: an os.open spy asserts O_CREAT | O_EXCL and an explicit 0o600 mode (the final-mode check alone was also satisfied by the post-write chmod), and a forced fdopen-failure test asserts the raw fd is closed and no temp file is left behind. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… lock store_photon_token/store_project_credentials/store_user_numbers read-modify- wrote auth.json without hermes_cli/auth.py's _auth_store_lock(), the cross-process flock every other writer of that file (credential_pool refresh, model_switch, fallback_cmd, nous_portal adapter, etc.) already holds. A concurrent write from either side during the unlocked load-mutate-save window silently drops the other side's update.
…tup (#72763) Two related bugs in hermes photon setup / gateway_setup: Bug 1 -- stale token reused (401) ---------------------------------- _cmd_setup reused an existing dashboard token without validation. The device token has a short TTL (~3-4 days observed); reusing a stale token caused every management API call (find_project_by_name, regenerate_project_secret, etc.) to fail with 401. The operator saw confusing "spectrum provisioning failed: 401" errors. Fix: check GET /api/auth/get-session before using the stored token. On 401/403, clear the stale token with clear_photon_token() and fall back to a fresh device-login flow automatically. Bug 2 -- channel left disabled after successful setup ----------------------------------------------------- After all five provisioning steps completed, config.yaml still had photon.enabled: false, so the gateway never loaded the Photon adapter. Every inbound iMessage hit Photon's offline auto-responder without the operator being notified. Fix: call write_platform_config_field('photon', 'enabled', True, raw=True) as a final setup step so the gateway picks up the freshly configured channel on its next start. New public API in auth.py: - clear_photon_token() -- discard stored token from auth.json - check_photon_token_valid(token) -- lightweight session-check test References: #72763
Maintainer follow-up to the #72763 salvage: check_photon_token_valid() now delegates to the existing validate_photon_token() (session lookup + /api/projects/) instead of a bespoke get-session-only probe, since the device flow can mint tokens that pass the session check but fail the project APIs that setup actually uses. Semantics preserved: definitive auth rejection = stale, transient errors = probably-valid.
…alid hermes photon setup unconditionally called regenerate_project_secret() on every re-run, invalidating the credential held by a running sidecar and causing all outbound sends to fail with AuthenticationError. Now validates existing credentials via a lightweight list_users call before deciding to regenerate. Only rotates when no credentials exist or the existing ones are invalid, and warns the user to restart the gateway when rotation occurs. Fixes #50755
…gnostic chain
Problema
--------
Quando o npm install do sidecar Photon falhava, o Hermes descartava toda
evidencia e continuava normalmente - deixando o adapter de iMessage
silenciosamente ausente, sem nenhuma mensagem de erro acionavel.
Tres falhas independentes formavam o caminho de falha silenciosa:
1. check_requirements() sem logging
Cada branch de return False retornava sem emitir nenhum log. O core em
platform_registry.py so consome o bool de check_fn() e loga uma mensagem
generica com o install_hint - sem acesso ao motivo real da falha.
if not HTTPX_AVAILABLE: return False # sem log
if not shutil.which(node): return False # sem log
if not node_modules.exists: return False # sem log
2. node_modules/ parcialmente criado passava o guard (Risk 2)
npm cria node_modules/ antes de abortar em ENOSPC, timeout de rede ou
EACCES. O diretorio existia, check_requirements() retornava True (falso
positivo), o adapter era registrado, e o crash acontecia em runtime com
um erro de modulo ausente aparentemente nao relacionado ao setup.
3. stderr do npm descartado (Risk 3)
subprocess.run sem stderr=PIPE. O output de erro aparecia no terminal
durante o setup e sumia depois - diagnostico impossivel em CI/CD, Docker,
VPS headless, e qualquer reinstalacao posterior.
Correcoes
---------
adapter.py - check_requirements() agora loga por branch:
- httpx ausente -> logger.warning com nome do pacote
- node nao no PATH -> logger.warning com nome do binario e env var
- spectrum-ts ausente -> logger.debug com path do sidecar + ultimo erro npm
(DEBUG nao WARNING: estado normal pre-setup; check_fn() e chamado de
5 hot paths do core incluindo polling do /api/status)
adapter.py - content check em vez de existence check (Risk 2):
antes: if not (_SIDECAR_DIR / node_modules).exists()
depois: if not (_SIDECAR_DIR / node_modules / spectrum-ts).exists()
spectrum-ts e a unica dependencia do package.json. Checar sua presenca
garante que instalacao parcial/abortada e detectada no boot do gateway,
nao na primeira mensagem recebida via gRPC.
cli.py - stderr capturado e persistido (Risk 3):
subprocess.run passa agora stderr=subprocess.PIPE, text=True em ambas as
chamadas (npm ci e npm install fallback). O stderr capturado e:
- impresso em sys.stderr imediatamente (output visivel no terminal)
- persistido em _NPM_ERROR_LOG = sidecar/.photon-npm-error.log se
returncode != 0, limitado a 300 chars
- apagado de _NPM_ERROR_LOG se returncode == 0 (evita erro stale)
check_requirements() le _NPM_ERROR_LOG quando spectrum-ts esta ausente e
inclui o conteudo no DEBUG log - o erro do npm sobrevive ao terminal, ao
restart do gateway e a reinicializacao da maquina.
sidecar/.gitignore - adicionado node_modules/ e .photon-npm-error.log.
Isolamento - sem impacto no core:
- check_fn() continua retornando apenas bool; core nao e modificado
- Logging usa namespace plugins.platforms.photon.adapter, isolado de
gateway.* e hermes_cli.*
- Cada plugin tem seu proprio check_requirements() independente
- OSError no write/read de _NPM_ERROR_LOG e silenciado - nunca propaga
Testes - 24/24 passando:
test_check_requirements_risks.py (7 testes):
WARNING emitido quando httpx ausente
WARNING emitido quando node nao no PATH
DEBUG emitido (nao WARNING) quando spectrum-ts ausente, com path
node_modules/ vazio agora retorna False (Risk 2 resolvido)
_NPM_ERROR_LOG escrito no stderr do npm em falha
_NPM_ERROR_LOG apagado apos npm bem-sucedido
erro npm aparece no DEBUG log quando node_modules ausente
test_npm_error_log_regression.py (9 testes - vetores de falha da solucao):
return code contrato intacto (0 em sucesso, nao-zero em falha)
OSError no write do log silenciado, exit code ainda propagado
OSError no read do log silenciado, check_requirements() retorna False
stderr vazio nao cria arquivo de log
proc.stderr=None nao lanca AttributeError
log stale apagado apos reinstall bem-sucedido
DEBUG emitido mesmo sem log de erro (setup pela primeira vez)
Closes #50981
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Addresses teknium1 review on #50983: - cli.py's `hermes photon status` and adapter.py's _start_sidecar() still used the old node_modules/-existence check while check_requirements() had moved to a spectrum-ts content check. Extracted the check into a shared sidecar_deps_installed(), used by all three, so an empty/partial node_modules/ (aborted npm install) is rejected consistently instead of only in check_requirements(). - _install_sidecar()'s success-path _NPM_ERROR_LOG.unlink() only caught FileNotFoundError, so a PermissionError/OSError on a locked file would propagate. Broadened to OSError. - npm stderr was truncated only when read back in check_requirements(); an unbounded stderr was written to disk on every failed install. Now truncated to _NPM_ERROR_LOG_MAX_CHARS before write_text(). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
All Photon sidecar HTTP requests target 127.0.0.1 — they should never be routed through a system HTTP proxy. When trust_env=True (the default), httpx picks up macOS system proxy settings and routes localhost requests through the proxy. If the proxy returns a spurious response (e.g. 502), _reap_stale_sidecar() interprets it as 'port in use by a non-sidecar process' and refuses to start, yielding: 'pids: unknown, not a Photon sidecar'. Set trust_env=False on all five httpx.AsyncClient call sites in the Photon adapter so localhost sidecar communication bypasses the system proxy entirely.
Maintainer follow-up: _cmd_setup now validates existing tokens (#72763 salvage); the pre-existing setup tests monkeypatch a stored token, so without stubbing check_photon_token_valid they'd hit the real dashboard API and hang.
teknium1
force-pushed
the
photon/auth-cluster
branch
from
July 28, 2026 20:33
3068278 to
af1d812
Compare
This was referenced Jul 29, 2026
Closed
6 tasks
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
7 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.
Summary
Consolidated Photon auth/setup salvage: setup no longer 401s on stale device tokens and actually enables the channel in config.yaml, project secrets stop being needlessly rotated out from under a running sidecar,
check_requirements()failures are diagnosable, auth.json writes are atomic + locked, and system proxies can no longer break sidecar localhost calls.Salvages six contributor PRs onto current main with authorship preserved, in composition order: #60427 (@solyanviktor-star), #64902 (@pierrenode), #72803 photon-only (@webtecnica), #50761 (@liuhao1024), #50983 (@JoaoMarcos44), #47933 (@DI404N, widened to all 5 httpx sites).
Changes
store_photon_token/store_project_credentials/store_user_numbersnow use the shared_auth_store_locklike the ~15 other auth.json writers.clear_photon_tokenon 401, and setup now callswrite_platform_config_field("photon","enabled",True). Maintainer follow-up routes validation through the existingvalidate_photon_token(session +/api/projects/— catches the exact "session OK, project API 401" case)._cmd_setupvalidates the existing secret vialist_usersbefore regenerating; warns to restart the gateway when rotation does occur.check_requirements()False branches,sidecar_deps_installed()checksnode_modules/spectrum-ts(fixes partial-install false positive), last npm error persisted to.photon-npm-error.log.trust_env=Falseon sidecar-localhosthttpx.AsyncClient— widened from the PR's sites to all five on current main (adapter.py:435, 878, 1004, 1567, 1707).Validation
Fixes #72763. Fixes #50755. Fixes #50981.
Infographic