fix(security): block path traversal in skill_view file_path parameter by Farukest · Pull Request #221 · NousResearch/hermes-agent

Farukest · 2026-02-28T23:14:56Z

skill_view accepts a file_path parameter to read files within a skill directory, but does not validate the path for traversal. skill_view("any-skill", file_path="../../.env") reads ~/.hermes/.env containing API keys.

Changes

Added path traversal checks to tools/skills_tool.py matching the existing pattern in skill_manager_tool.py:

Reject .. in path components
Verify the resolved path stays within the skill directory via resolve() containment check

Tests

Added tests/tools/test_skill_view_traversal.py with 4 tests:

../../.env is blocked
references/../../.env is blocked
Legitimate file paths still work
Viewing a skill without file_path still works

All 4 tests pass.

Closes #220

skill_view accepts a file_path parameter to read files within a skill directory, but did not validate the path. Passing file_path="../../.env" allowed reading arbitrary files outside the skill directory, including API keys and other sensitive data. Added path traversal checks matching the existing pattern in skill_manager_tool.py: reject ".." in path components and verify the resolved path stays within the skill directory.

@Bartok9

skill_view accepted arbitrary file_path values like '../../.env' and would read files outside the skill directory, exposing API keys and other sensitive data. Added two layers of defense: 1. Reject paths with '..' components (fast, catches obvious traversal) 2. resolve() containment check with trailing '/' to prevent prefix collisions (catches symlinks and edge cases) Fix approach from PR #242 (@Bartok9). Vulnerability reported by @Farukest (#220, PR #221). Tests rewritten to properly mock SKILLS_DIR. Closes #220

teknium1 · 2026-03-02T10:00:28Z

Thanks for reporting the vulnerability and providing a fix @Farukest! Your test approach (mocking SKILLS_DIR) was the correct one and we used it in the final implementation.

We went with PR #242's code fix because it includes a trailing / in the startswith() check to prevent prefix collisions and has exception handling on resolve(). But your contribution was essential — you found the bug and showed the right mocking pattern.

Fixed in commit 1cb2311.

Farukest · 2026-03-03T19:09:56Z

Thanks for reporting the vulnerability and providing a fix @Farukest! Your test approach (mocking SKILLS_DIR) was the correct one and we used it in the final implementation.

We went with PR #242's code fix because it includes a trailing / in the startswith() check to prevent prefix collisions and has exception handling on resolve(). But your contribution was essential — you found the bug and showed the right mocking pattern.

Fixed in commit 1cb2311.

Makes sense, the trailing slash guard is a better approach. Thanks for the credit @teknium1 👍

andrueandersoncs · 2026-03-30T19:43:13Z

Starting Work

Branch: auto/issue-221-false-ai-advertising-ai-powered-meal-generation-
Worktree: /Users/andrue/.hermes/worktrees/vantage/issue-221-false-ai-advertising-ai-powered-meal-generation-

Plan

Locate the Weekly Plan generation UI and dropdown options
Remove/disable the "AI-Powered" option since no API is configured
Update "Auto (AI if available)" label to accurately reflect template-based generation
Fix the success message to say "template-based" not "AI-powered"
Add error handling for failed AI generation attempts
Run verification (tests, build, lint)
Merge to main and QA on Railway

andrueandersoncs · 2026-03-30T19:48:31Z

Fixed ✓

Commit: ddc8596 — merged to main and deployed to Railway

Changes Made

Success message accuracy (lines 276-281):
- Fixed modeLabel logic to require llmAvailable AND (mode === "llm" OR mode === "auto") to claim "AI-powered"
- When no API configured, always reports "template-based" regardless of user selection
- When API available but user selects "Templates Only", correctly reports "template-based"
UI clarity when API unavailable (lines 371-393):
- Shows "Meals: Templates only (AI not configured)" text instead of misleading dropdown
- No more "AI-Powered" option when no API key configured
Better dropdown labels when API available:
- "Auto (Templates + AI when available)" — clearer than "AI if available"
- "Templates Only" — clearer than just "Templates"

Verification

Tests: 448 passing ✓
Build: Clean TypeScript compile ✓
Lint: No new issues (pre-existing warnings in other files) ✓
QA on Railway:

Dropdown shows improved labels: "Auto (Templates + AI when available)", "AI-Powered", "Templates Only" ✓
Regenerate with "Auto" mode correctly reports: "New AI-powered plan generated from your profile." ✓ (Railway has API configured)

Acceptance Criteria

Remove or disable the "AI-Powered" option until OpenAI API is actually configured
Change misleading labels to accurately reflect template-based generation
Success message accurately reflects what was used

Note: The Railway deployment has the API configured, so it correctly shows AI options. In local dev without API key, users will see "Templates only (AI not configured)" instead of the misleading AI options.

@Bartok9

…usResearch#220) skill_view accepted arbitrary file_path values like '../../.env' and would read files outside the skill directory, exposing API keys and other sensitive data. Added two layers of defense: 1. Reject paths with '..' components (fast, catches obvious traversal) 2. resolve() containment check with trailing '/' to prevent prefix collisions (catches symlinks and edge cases) Fix approach from PR NousResearch#242 (@Bartok9). Vulnerability reported by @Farukest (NousResearch#220, PR NousResearch#221). Tests rewritten to properly mock SKILLS_DIR. Closes NousResearch#220

@Bartok9

…usResearch#220) skill_view accepted arbitrary file_path values like '../../.env' and would read files outside the skill directory, exposing API keys and other sensitive data. Added two layers of defense: 1. Reject paths with '..' components (fast, catches obvious traversal) 2. resolve() containment check with trailing '/' to prevent prefix collisions (catches symlinks and edge cases) Fix approach from PR NousResearch#242 (@Bartok9). Vulnerability reported by @Farukest (NousResearch#220, PR NousResearch#221). Tests rewritten to properly mock SKILLS_DIR. Closes NousResearch#220

…ecycle Acting implementation lane: Talos; review authority: Athena; control: Hermes.\n\nGitHub Issue: dagyrox/or7-platform#221\nHermes Kanban: t_c27eaab4; repair t_2c98e44f run 123\nCanonical work packet: https://github.com/dagyrox/or7-platform/issues/221\nPlanned tracked packet: docs/agentic-operations/work-packets/2026-08-21-hermes-kanban-lifecycle-isolation.md via t_b4fe713f\nExact reviewed head: aedc1ef\nAthena verdict artifact: /tmp/athena-hermes-pr9-body-rereview-aedc1efc.md (93b549ad9c8ab58e43210b6a898bab918c212695e743375a1e0268c6ab21fa7a)\nATHENA_VERDICT: READY_TO_MERGE

dagyrox · 2026-08-24T09:05:12Z

Provider fix opened as #93727. It adds the exact embedded-dispatcher quiesce/acknowledgment boundary, fixed fail-closed CLI states, cancellation-safe drain behavior, and runbook documentation. Acting lane: Talos builder; Hermes Kanban: t_c349e5f1; run: run-20260824-issue-221.

[Talos][Or7 NousResearch#221][Hermes PR #10] Reject bool/float protocol and PID values across owner, request, and ACK trust-boundary validation. Malformed owner state now makes the requester fail closed without control writes; deterministic red-green coverage preserves valid different-generation behavior.\n\nHermes Kanban: t_832c4f2d / run 178\nWork packet: /root/projects/or7-platform/.worktrees/issue221-activation-freeze-repair/docs/agentic-operations/work-packets/2026-08-21-hermes-kanban-lifecycle-isolation.md\nAuthority: Athena; Control: Hermes; Acting lane: Talos

* fix(kanban): add fail-closed embedded dispatcher quiesce Acting lane: Talos builder Issue: dagyrox/or7-platform#221 Consumer PR: dagyrox/or7-platform#223 Hermes Kanban: t_c349e5f1 / run 173 Work packet: docs/agentic-operations/work-packets/2026-08-21-hermes-kanban-lifecycle-isolation.md * fix(kanban): reject malformed quiesce records Acting lane: Talos builder Authority: Athena architecture/release review Issue: dagyrox/or7-platform#221 Provider PR: #10 Hermes Kanban: t_48ce0768 / run 176 Predecessor: 6882b31 Review artifact: /tmp/athena-hermes-pr10-exact-head-6882b314.md (SHA-256 3cbf369b32f23b5f6ef5149e48904fbea32314895de06afa91b1a4ce0bf996af) Work packet: docs/agentic-operations/work-packets/2026-08-21-hermes-kanban-lifecycle-isolation.md * fix(kanban): enforce exact control integer types [Talos][Or7 NousResearch#221][Hermes PR #10] Reject bool/float protocol and PID values across owner, request, and ACK trust-boundary validation. Malformed owner state now makes the requester fail closed without control writes; deterministic red-green coverage preserves valid different-generation behavior.\n\nHermes Kanban: t_832c4f2d / run 178\nWork packet: /root/projects/or7-platform/.worktrees/issue221-activation-freeze-repair/docs/agentic-operations/work-packets/2026-08-21-hermes-kanban-lifecycle-isolation.md\nAuthority: Athena; Control: Hermes; Acting lane: Talos --------- Co-authored-by: Talos <11076310+dagyrox@users.noreply.github.com>

@Bartok9

…usResearch#220) skill_view accepted arbitrary file_path values like '../../.env' and would read files outside the skill directory, exposing API keys and other sensitive data. Added two layers of defense: 1. Reject paths with '..' components (fast, catches obvious traversal) 2. resolve() containment check with trailing '/' to prevent prefix collisions (catches symlinks and edge cases) Fix approach from PR NousResearch#242 (@Bartok9). Vulnerability reported by @Farukest (NousResearch#220, PR NousResearch#221). Tests rewritten to properly mock SKILLS_DIR. Closes NousResearch#220

@Bartok9

…usResearch#220) skill_view accepted arbitrary file_path values like '../../.env' and would read files outside the skill directory, exposing API keys and other sensitive data. Added two layers of defense: 1. Reject paths with '..' components (fast, catches obvious traversal) 2. resolve() containment check with trailing '/' to prevent prefix collisions (catches symlinks and edge cases) Fix approach from PR NousResearch#242 (@Bartok9). Vulnerability reported by @Farukest (NousResearch#220, PR NousResearch#221). Tests rewritten to properly mock SKILLS_DIR. Closes NousResearch#220

Farukest mentioned this pull request Feb 28, 2026

Path traversal in skill_view allows reading arbitrary files including API keys #220

Closed

teknium1 closed this Mar 2, 2026

roadhero mentioned this pull request May 17, 2026

Revert: restore plugins/memory/mem0_oss/ (Phase 3 architectural rollback) fox-in-the-box-ai/hermes-agent#3

Merged

This was referenced May 24, 2026

fix(packaging): ship plugin.yaml via package-data + MANIFEST.in (closes #76) TechDevGroup/hermes-agent#77

Merged

T4: consult devagentic toolList; stop shipping the 50-tool zoo TechDevGroup/hermes-agent#157

Open

benbarclay mentioned this pull request May 28, 2026

docker: opt in to dashboard --insecure via env var, never derive from bind host #34188

Merged

dagyrox mentioned this pull request Aug 24, 2026

fix(kanban): add fail-closed embedded dispatcher quiesce #93727

Closed

dagyrox mentioned this pull request Aug 24, 2026

fix(kanban): add fail-closed embedded dispatcher quiesce dagyrox/hermes-agent#10

Merged

This was referenced Aug 26, 2026

R30-4 — Benjamin two-profile gateway canary through T+24h electricsheephq/evaOS-hermes-desktop-app-adapter#201

Closed

fix(desktop): restore managed profile boot and MCP reload electricsheephq/evaOS-hermes-desktop-app-adapter#214

Merged

Jensenwgd mentioned this pull request Sep 4, 2026

[Bug]: Assistant final response replaced with "[response interrupted]" placeholder despite finish_reason=stop #102566

Open

Navigation Menu

fix(security): block path traversal in skill_view file_path parameter - #221

fix(security): block path traversal in skill_view file_path parameter#221
Farukest wants to merge 1 commit into
NousResearch:mainfrom
Farukest:fix/skill-view-path-traversal

Farukest commented Feb 28, 2026

Uh oh!

teknium1 commented Mar 2, 2026

Uh oh!

Farukest commented Mar 3, 2026

Uh oh!

andrueandersoncs commented Mar 30, 2026

Uh oh!

andrueandersoncs commented Mar 30, 2026

Uh oh!

dagyrox commented Aug 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Farukest commented Feb 28, 2026

Changes

Tests

Uh oh!

teknium1 commented Mar 2, 2026

Uh oh!

Farukest commented Mar 3, 2026

Uh oh!

andrueandersoncs commented Mar 30, 2026

Starting Work

Plan

Uh oh!

andrueandersoncs commented Mar 30, 2026

Fixed ✓

Changes Made

Verification

Acceptance Criteria

Uh oh!

dagyrox commented Aug 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants