Secret management: unifying profile-scoped credential hydration across hermes-agent and hermes-webui #7347
shauneccles
started this conversation in
Ideas
Replies: 1 comment
|
Full design doc for anyone who wants the details before weighing in: https://gist.github.com/shauneccles/b835110a3de49002edabcb57c2b23d45 It covers the get_secret adoption gap in hermes_cli/, the non-isolating-mode finding, the five divergent .env parsers, and the six-step migration with the agent-side step 6 flagged as the unowned highest-leverage piece. |
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
What this is
A proposal for how profile-scoped secret hydration should work across hermes-agent and hermes-webui. It comes out of a PR I withdrew (#7343) after two review findings turned out to be the same defect in different places — there's no single chokepoint for "resolve this profile's credentials," so every new credential reader is a chance for the next divergence.
The core finding
The agent has a cohesive model —
get_secret/build_profile_secret_scope/UnscopedSecretError— but its migration stopped at theagent/package boundary and never reachedhermes_cli/, which is exactly the surface the WebUI consumes:get_secret(os.getenv/os.environhermes_cli/models.py(backs/api/models/live)hermes_cli/auth.py(registry the WebUI's scrub set derives from)hermes_cli/model_switch.pySo the WebUI cannot converge by adopting
get_secret— the code it calls doesn't read through it. Theos.environmirror the WebUI maintains is load-bearing, not NIH. Meanwhile the WebUI runs the agent scope in non-isolating mode (never callsset_multiplex_active), so isolation rests entirely on a hand-maintained 20-name scrub denylist — any name missing from that list leaks root's credential into a named profile. A denylist doing a boundary's job.Proposed target
Not "one read path" — that's unachievable while
hermes_cli/models.pyreads raw and the quota probe forks. Instead:Four channels are irreducible (contextvar,
_thread_ctx.env,os.environ, subprocessenv=), so the model is four layers: policy → resolve → publish → read. The key-authority table is a singlesecret_policymodule where scrub and hydrate become the same predicate at two moments — which is what stops them drifting (they're currently two separate, hand-maintained lists that already disagree).Things the review surfaced that aren't design trivia
api/onboarding.py:1048and:1113writeos.environ[env_var] = api_keypermanently, never restored. Saving a named profile's key writes it into the server process for its lifetime. This exists today, independent of the refactor..envparsers exist and disagree (onlybootstrap.pyhandlesexport, none handle inline comments/escaped quotes/BOM, two drop empty values). Unifying them is the riskiest step because it changes values for real users.api/providers.py:1633) scrubs then re-adds.envonly, so a vault-only credential is stripped and never restored.Migration (each step shippable)
0 land the working tree → 1 one
.envparser → 2 one policy table → 3 one resolver → 4 one read path → 5 fail-closed in four sub-steps (warn first, flip to raise last) → 6 migratehermes_cli/models.py+auth.pyin hermes-agent, then delete the mirror.Step 6 is the highest leverage (it's the only step that makes the architecture actually single) and has no owner — it's an agent-side change, which is why this is a discussion here rather than just a WebUI RFC.
Two decisions already made, flagging for visibility
routes.py:20051reads rawOPENAI_API_KEY, so profile B's TTS spends profile A's quota).API_SERVER_KEY→ process-global (it's the process↔agent channel). Raised as hermes-agent#96920 since the agent classifies it profile-scoped.What I'm asking
get_secretmigration intohermes_cli/, or should this stay two separate systems?docs/rfcs/be welcome once the direction's agreed, or is a discussion + incremental PRs the preferred path?Happy to do the implementation work either way.
All reactions