fix(hermes_cli): stop hermes update from leaking default API keys into distribution profiles - #71435
Open
fangliquanflq wants to merge 10 commits into
Open
fix(hermes_cli): stop hermes update from leaking default API keys into distribution profiles#71435fangliquanflq wants to merge 10 commits into
fangliquanflq wants to merge 10 commits into
Conversation
Without a per-profile .env sentinel, hermes update's backfill copies the default profile's API keys into a freshly installed distribution profile.
If the sentinel write fails after the profile tree is created, refuse the install and remove a fresh target so hermes update cannot backfill default credentials into an env-less profile.
…ofiles Pre-fix dist installs lack .env; hermes update must seed a placeholder instead of copying the default profile's API keys into them.
Contributor
|
Thanks for tracing this through both distribution installation and the update-time migration. Current main confirms the premise: Problems
Suggested changes
This is an automated hermes-sweeper review. |
…fill summary Distribution profiles now get a placeholder .env; stop claiming every seeded profile was copied from default.
Contributor
Author
|
Addressed in 7afd76a.
|
…-env # Conflicts: # hermes_cli/main.py # tests/hermes_cli/test_profiles.py
Distribution-aware backfill no longer copies default secrets; stop claiming that in the install fail-closed message.
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
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.
What does this PR do?
Fresh
hermes profile installprofiles never got a per-profile.envsentinel (unlikehermes profile create). The nexthermes updatethen treated them as legacy and copied the default profile's.env(API keys / tokens) into the installed profile, breaking credential isolation.Bug Cause
install_distributionbootstrapped dirs and copied the dist payload but skipped seeding.env. Credentials are intentionally excluded from the payload (USER_OWNED_EXCLUDE), so the new profile had no secrets file.backfill_profile_envs(invoked fromhermes update) copies the default home.envinto any named profile missing.env- correct for pre-#44792 regular profiles, wrong for brand-new distribution installs that never shared those credentials.Reproduction Steps
.env(e.g.OPENAI_API_KEY=...).hermes profile install <local-or-git-dist> --name telem -y.profiles/telem/has no.env(only.env.EXAMPLEif the manifest lists env requirements).hermes update.profiles/telem/.envis a byte-copy of the default secrets.Expected: installed dist profile keeps its own empty/placeholder
.env(or none that is later filled with foreign secrets).Before fix:
hermes updatecopies default API keys into the dist profile.Fix
.envsentinel oninstall_distributionthatcreate_profilealready writes; fail closed on fresh installs if the write fails (remove the half-created tree).backfill_profile_envsso profiles withdistribution.yamlget a placeholder instead of a copy of the default.env(covers pre-fix installs already on disk).--forcepreserve, write-failure cleanup, and legacy dist without.env.Related Issue
No issue
Type of Change
Changes Made
hermes_cli/profile_distribution.py- seed empty.envafter dist install; fail closed on fresh targets if seed write failshermes_cli/profiles.py-backfill_profile_envswrites a placeholder for distribution profiles instead of copying default secretstests/hermes_cli/test_profile_distribution.py- regression coverage for seed, backfill isolation, force preserve, and seed write failureHow to Test
.env, and step 4 must not copy default secrets (including when.envis deleted to simulate a pre-fix install).Checklist
Code
scripts/run_tests.shon relevant tests and they passDocumentation & Housekeeping
cli-config.yaml.exampleif I added/changed config keys - N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows - N/A