fix(skills): correct three security-sensitive stale claims in the bundled hermes-agent skill - #87619
Open
ygd58 wants to merge 1 commit into
Open
fix(skills): correct three security-sensitive stale claims in the bundled hermes-agent skill#87619ygd58 wants to merge 1 commit into
ygd58 wants to merge 1 commit into
Conversation
…dled hermes-agent skill Fixes NousResearch#87552 (partial -- 3 of 10 reported categories, the ones fully verified against runtime source within scope). The bundled skills/autonomous-ai-agents/hermes-agent skill had drifted from current CLI/runtime behavior in several security-sensitive ways. Verified and corrected three: 1. references/portal-auth-for-third-party-apps.md (issue item NousResearch#4): recommended agents build a DIY credential-relay proxy that reads the raw Portal bearer directly out of ~/.hermes/auth.json and forwards it -- but `hermes proxy` (verified via `hermes proxy --help`) already ships exactly this functionality as a supported, first-party command. Recommending a custom credential-extraction tool when a safe official alternative exists is the security-sensitive gap. Rewrote Layer 3 and the cheatsheet to point at `hermes proxy start/status/providers` instead, and noted `hermes login`'s current deprecation (verified via `hermes login --help`: "Deprecated. Use `hermes auth`... `hermes model`... `hermes setup`"). 2. references/background-systems.md (issue item NousResearch#6): recommended `terminal(background=True, notify_on_complete=True)` as an alternative to `cronjob` for work that "must outlive the process" -- but terminal background execution is ALSO process-local, exactly like delegate_task's own background mode this same paragraph correctly calls "Not durable" one sentence earlier. Removed the self-contradictory recommendation; only cronjob is durable. Also corrected the stale "3-minute hard interrupt per run" cron timeout claim: verified against cron/scheduler.py that current defaults are a 600s (10min, configurable via HERMES_CRON_TIMEOUT) inactivity timeout for the agent run and a separate 3600s (1hr) default for pre-run scripts, not a fixed 3-minute interrupt. 3. references/security-privacy.md (issue item NousResearch#7): described `approvals.mode: off`/`--yolo` as skipping "all approval prompts" / "bypass everything" -- but tools/approval.py's own code comments state hardline command blocks and user-defined approvals.deny rules fire "BEFORE the yolo bypass" unconditionally, by design ("Opting into yolo is trusting the agent with your files and services, not trusting it to wipe the disk or power the box off"). Corrected to describe yolo as disabling the interactive prompt for recoverable dangerous commands specifically, with hardline/deny still active. Also corrected the claim that "file writes never go through the approval prompt" with no protection at all: write_file/patch DO unconditionally block writes to a fixed set of sensitive paths (verified: get_write_denied_error() in tools/file_operations.py, called from both write_file_tool and the patch tool). Item NousResearch#5 (literal API keys in config.yaml examples) was checked but not found in the current source -- no literal `api_key:` example exists in providers-and-models.md or configuration.md, only schema/field-name references. Not changed; may be a false positive in the original report, or already fixed since. The remaining categories (NousResearch#1, NousResearch#2, NousResearch#3, NousResearch#8, NousResearch#9, NousResearch#10) were not independently re-verified against source within the scope of this PR and are left for separate follow-up -- this PR does not claim to close the full 10-category report. Added tests/skills/test_hermes_agent_skill.py per the issue's own proposed fix, scoped to the three categories this PR actually addresses (8 tests: 3 for Portal auth guidance, 2 for background durability wording, 3 for YOLO/approval wording). Verified as genuine regressions by stashing the content fix and confirming all 8 tests fail against the original wording. 8/8 pass in the new test file; 1184/1184 in the existing tests/skills/test_authoring_standards.py mechanical-standards suite across every bundled and optional skill (no regression).
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
1 participant
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.
Fixes #87552 (partial -- 3 of the 10 reported categories, the ones fully verified against runtime source within this PR's scope).
This is a large, well-researched issue spanning 10 categories of documentation drift. Rather than attempt a superficial pass across all 10, I verified and fixed the three most clearly security-sensitive ones against actual runtime source:
1. Portal credential-relay guidance (issue item #4)
portal-auth-for-third-party-apps.mdrecommended agents build a DIY proxy reading the raw Portal bearer directly out of~/.hermes/auth.json-- buthermes proxyalready ships exactly this functionality as a supported command (verified viahermes proxy --help). Rewrote to point athermes proxy start/status/providers, and notedhermes login's deprecation (verified via--help).2. Background execution durability wording (issue item #6)
background-systems.mdrecommendedterminal(background=True)as an alternative tocronjobfor work that must survive a process restart -- but terminal background execution is also process-local, contradicting the SAME paragraph's own correct "Not durable" callout one sentence earlier. Also corrected a stale "3-minute hard interrupt" cron timeout claim (verified againstcron/scheduler.py: actual defaults are 600s inactivity / 3600s pre-run-script timeout).3. YOLO/approval-mode overstatement (issue item #7)
security-privacy.mddescribed--yolo/approvals.mode: offas skipping "all approval prompts" -- buttools/approval.py's own code comments confirm hardline command blocks andapprovals.denyrules fire "BEFORE the yolo bypass" unconditionally, by design. Also corrected the claim that file writes have no protection at all (get_write_denied_error()blocks sensitive paths unconditionally).Scope note
Item #5 was checked but not found in current source (no literal
api_key:example exists). The remaining categories (#1, #2, #3, #8, #9, #10) were not independently re-verified within this PR's scope and are left for follow-up.Verification
Added
tests/skills/test_hermes_agent_skill.pyper the issue's own proposed fix, scoped to the 3 categories addressed. Verified as genuine regressions by stashing the content fix and confirming all 8 tests fail against the original wording.8/8 pass in the new test file; 1184/1184 in the existing skill-authoring-standards suite across every bundled/optional skill (no regression).