Skip to content

fix(skills-guard): stop flagging benign skill content + honor skill ignore files - #36231

Merged
teknium1 merged 1 commit into
mainfrom
hermes/hermes-22cdaeeb
Jun 1, 2026
Merged

fix(skills-guard): stop flagging benign skill content + honor skill ignore files#36231
teknium1 merged 1 commit into
mainfrom
hermes/hermes-22cdaeeb

Conversation

@teknium1

@teknium1 teknium1 commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Summary

The skill security scanner now stops flagging three classes of benign, intrinsic skill content, and honors a skill-provided .skillignore / .clawhubignore. This unblocks legitimate community skills that were previously hard-blocked on false positives.

Reported via mvanhorn/last30days-skill#464 — the scanner returned DANGEROUS with --force unable to override, on patterns that every compliant skill trips.

Changes

  • tools/skills_guard.pyread_secrets_file: exclude cat > file.env << output redirections (a setup doc writing the user's own keys into their own local .env is the opposite of exfiltration); only cat file.env reads are flagged.
  • tools/skills_guard.pyallowed_tools_field: dropped HIGH → LOW. allowed-tools: is REQUIRED SKILL.md frontmatter per the agent-skill spec; flagging it as privilege escalation made every compliant skill caution/dangerous. Kept as an informational finding for auditability; it no longer drives the verdict.
  • tools/skills_guard.pypython_os_environ: exempt os.environ.get("NONSECRET") config reads (the common, benign form). Added a dedicated CRITICAL python_environ_get_secret pattern so secret-named reads (OPENAI_API_KEY, GITHUB_TOKEN, …) are still caught — no coverage regression.
  • tools/skills_guard.pyscan_skill() now reads a skill's .skillignore (Hermes-native) or .clawhubignore (ClawHub compat), gitignore-style, and excludes matching paths from BOTH structural checks and pattern scanning. The ignore files are always excluded; SKILL.md can never be excluded.
  • tests/tools/test_skills_guard.py — 16 new tests covering each fix (positive + negative cases) and the ignore matcher.

Validation

Before After
tests/tools/test_skills_guard.py 64 pass 80 pass (64 + 16)
cat > x.env << heredoc CRITICAL exfil clean
cat x.env read CRITICAL exfil CRITICAL exfil (unchanged)
allowed-tools: frontmatter HIGH (→ caution) LOW (informational)
os.environ.get("CONFIG_DIR") HIGH exfil clean
os.environ.get("API_KEY") HIGH exfil CRITICAL (correctly)
dict(os.environ) bare dump HIGH exfil HIGH exfil (unchanged)

Scope note (honest)

These fixes are a scanner-quality win across all community skills, but they do not flip last30days-skill itself to installable. E2E scan of the actual install target (skills/last30days/, old vs new code) showed the verdict stays DANGEROUS: it genuinely contains 7 secret-named env reads (OPENAI_API_KEY, GITHUB_TOKEN, SCRAPE_CREATORS_API_KEY), vendored JS reading process.env[key], os.environ.copy(), and a 15MB skill dir. The author's .clawhubignore lives at the repo root, not the installed skill subdir, so it never entered the install scan to begin with. A real unblock for that skill needs a trust/attested-source path (separate work) or author-side secret-handling changes — not a looser scanner.

Reported-by: @kylehagler (mvanhorn/last30days-skill#464)

Infographic

skills-guard-false-positive-fixes

@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

🔎 Lint report: hermes/hermes-22cdaeeb vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 9577 on HEAD, 9577 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 4962 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have tool/skills Skills system (list, view, manage) comp/tools Tool registry, model_tools, toolsets labels Jun 1, 2026
@teknium1
teknium1 force-pushed the hermes/hermes-22cdaeeb branch from 5934b3a to c1dd910 Compare June 1, 2026 04:21
…gnore files

The skill security scanner blocked legitimate community skills on three
intrinsic false-positive patterns:

- read_secrets_file matched `cat > file.env <<` heredocs (writing the
  user's own keys into their own local .env), not just `cat file.env`
  reads. Exclude output redirections.
- allowed-tools frontmatter is REQUIRED by the agent-skill spec; every
  compliant skill declares it. Drop from HIGH privilege_escalation to a
  LOW informational finding so it no longer drives the verdict.
- python_os_environ flagged `os.environ.get("CONFIG_VAR")` config reads
  as HIGH exfiltration. Exempt non-secret `.get()` reads; add a dedicated
  CRITICAL python_environ_get_secret pattern so secret-named reads
  (OPENAI_API_KEY etc.) are still caught.

Also: scan_skill() now honors a skill-provided .skillignore / .clawhubignore
(gitignore-style) so dev/docs artifacts shipped in a skill root are excluded
from both structural checks and pattern scanning. SKILL.md is never ignorable.

80 tests pass (64 existing + 16 new).
@teknium1
teknium1 force-pushed the hermes/hermes-22cdaeeb branch from c1dd910 to c134fe1 Compare June 1, 2026 08:39
@teknium1
teknium1 merged commit ba6ffd4 into main Jun 1, 2026
23 checks passed
@teknium1
teknium1 deleted the hermes/hermes-22cdaeeb branch June 1, 2026 08:58
@mvanhorn

mvanhorn commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

goat emoji. Thanks @teknium1 !

SuperInstance pushed a commit to SuperInstance/hermes-construct that referenced this pull request Jun 1, 2026
…gnore files (NousResearch#36231)

The skill security scanner blocked legitimate community skills on three
intrinsic false-positive patterns:

- read_secrets_file matched `cat > file.env <<` heredocs (writing the
  user's own keys into their own local .env), not just `cat file.env`
  reads. Exclude output redirections.
- allowed-tools frontmatter is REQUIRED by the agent-skill spec; every
  compliant skill declares it. Drop from HIGH privilege_escalation to a
  LOW informational finding so it no longer drives the verdict.
- python_os_environ flagged `os.environ.get("CONFIG_VAR")` config reads
  as HIGH exfiltration. Exempt non-secret `.get()` reads; add a dedicated
  CRITICAL python_environ_get_secret pattern so secret-named reads
  (OPENAI_API_KEY etc.) are still caught.

Also: scan_skill() now honors a skill-provided .skillignore / .clawhubignore
(gitignore-style) so dev/docs artifacts shipped in a skill root are excluded
from both structural checks and pattern scanning. SKILL.md is never ignorable.

80 tests pass (64 existing + 16 new).
alt-glitch pushed a commit that referenced this pull request Jun 14, 2026
…gnore files (#36231)

The skill security scanner blocked legitimate community skills on three
intrinsic false-positive patterns:

- read_secrets_file matched `cat > file.env <<` heredocs (writing the
  user's own keys into their own local .env), not just `cat file.env`
  reads. Exclude output redirections.
- allowed-tools frontmatter is REQUIRED by the agent-skill spec; every
  compliant skill declares it. Drop from HIGH privilege_escalation to a
  LOW informational finding so it no longer drives the verdict.
- python_os_environ flagged `os.environ.get("CONFIG_VAR")` config reads
  as HIGH exfiltration. Exempt non-secret `.get()` reads; add a dedicated
  CRITICAL python_environ_get_secret pattern so secret-named reads
  (OPENAI_API_KEY etc.) are still caught.

Also: scan_skill() now honors a skill-provided .skillignore / .clawhubignore
(gitignore-style) so dev/docs artifacts shipped in a skill root are excluded
from both structural checks and pattern scanning. SKILL.md is never ignorable.

80 tests pass (64 existing + 16 new).
kossteg pushed a commit to kossteg/hermes-agent that referenced this pull request Jun 16, 2026
…gnore files (NousResearch#36231)

The skill security scanner blocked legitimate community skills on three
intrinsic false-positive patterns:

- read_secrets_file matched `cat > file.env <<` heredocs (writing the
  user's own keys into their own local .env), not just `cat file.env`
  reads. Exclude output redirections.
- allowed-tools frontmatter is REQUIRED by the agent-skill spec; every
  compliant skill declares it. Drop from HIGH privilege_escalation to a
  LOW informational finding so it no longer drives the verdict.
- python_os_environ flagged `os.environ.get("CONFIG_VAR")` config reads
  as HIGH exfiltration. Exempt non-secret `.get()` reads; add a dedicated
  CRITICAL python_environ_get_secret pattern so secret-named reads
  (OPENAI_API_KEY etc.) are still caught.

Also: scan_skill() now honors a skill-provided .skillignore / .clawhubignore
(gitignore-style) so dev/docs artifacts shipped in a skill root are excluded
from both structural checks and pattern scanning. SKILL.md is never ignorable.

80 tests pass (64 existing + 16 new).
T02200059 pushed a commit to T02200059/hermes-agent that referenced this pull request Jun 18, 2026
…gnore files (NousResearch#36231)

The skill security scanner blocked legitimate community skills on three
intrinsic false-positive patterns:

- read_secrets_file matched `cat > file.env <<` heredocs (writing the
  user's own keys into their own local .env), not just `cat file.env`
  reads. Exclude output redirections.
- allowed-tools frontmatter is REQUIRED by the agent-skill spec; every
  compliant skill declares it. Drop from HIGH privilege_escalation to a
  LOW informational finding so it no longer drives the verdict.
- python_os_environ flagged `os.environ.get("CONFIG_VAR")` config reads
  as HIGH exfiltration. Exempt non-secret `.get()` reads; add a dedicated
  CRITICAL python_environ_get_secret pattern so secret-named reads
  (OPENAI_API_KEY etc.) are still caught.

Also: scan_skill() now honors a skill-provided .skillignore / .clawhubignore
(gitignore-style) so dev/docs artifacts shipped in a skill root are excluded
from both structural checks and pattern scanning. SKILL.md is never ignorable.

80 tests pass (64 existing + 16 new).
xyshanren pushed a commit to xyshanren/hermes-agent-cn that referenced this pull request Jun 25, 2026
…gnore files (NousResearch#36231)

The skill security scanner blocked legitimate community skills on three
intrinsic false-positive patterns:

- read_secrets_file matched `cat > file.env <<` heredocs (writing the
  user's own keys into their own local .env), not just `cat file.env`
  reads. Exclude output redirections.
- allowed-tools frontmatter is REQUIRED by the agent-skill spec; every
  compliant skill declares it. Drop from HIGH privilege_escalation to a
  LOW informational finding so it no longer drives the verdict.
- python_os_environ flagged `os.environ.get("CONFIG_VAR")` config reads
  as HIGH exfiltration. Exempt non-secret `.get()` reads; add a dedicated
  CRITICAL python_environ_get_secret pattern so secret-named reads
  (OPENAI_API_KEY etc.) are still caught.

Also: scan_skill() now honors a skill-provided .skillignore / .clawhubignore
(gitignore-style) so dev/docs artifacts shipped in a skill root are excluded
from both structural checks and pattern scanning. SKILL.md is never ignorable.

80 tests pass (64 existing + 16 new).
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
…gnore files (NousResearch#36231)

The skill security scanner blocked legitimate community skills on three
intrinsic false-positive patterns:

- read_secrets_file matched `cat > file.env <<` heredocs (writing the
  user's own keys into their own local .env), not just `cat file.env`
  reads. Exclude output redirections.
- allowed-tools frontmatter is REQUIRED by the agent-skill spec; every
  compliant skill declares it. Drop from HIGH privilege_escalation to a
  LOW informational finding so it no longer drives the verdict.
- python_os_environ flagged `os.environ.get("CONFIG_VAR")` config reads
  as HIGH exfiltration. Exempt non-secret `.get()` reads; add a dedicated
  CRITICAL python_environ_get_secret pattern so secret-named reads
  (OPENAI_API_KEY etc.) are still caught.

Also: scan_skill() now honors a skill-provided .skillignore / .clawhubignore
(gitignore-style) so dev/docs artifacts shipped in a skill root are excluded
from both structural checks and pattern scanning. SKILL.md is never ignorable.

80 tests pass (64 existing + 16 new).
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
…gnore files (NousResearch#36231)

The skill security scanner blocked legitimate community skills on three
intrinsic false-positive patterns:

- read_secrets_file matched `cat > file.env <<` heredocs (writing the
  user's own keys into their own local .env), not just `cat file.env`
  reads. Exclude output redirections.
- allowed-tools frontmatter is REQUIRED by the agent-skill spec; every
  compliant skill declares it. Drop from HIGH privilege_escalation to a
  LOW informational finding so it no longer drives the verdict.
- python_os_environ flagged `os.environ.get("CONFIG_VAR")` config reads
  as HIGH exfiltration. Exempt non-secret `.get()` reads; add a dedicated
  CRITICAL python_environ_get_secret pattern so secret-named reads
  (OPENAI_API_KEY etc.) are still caught.

Also: scan_skill() now honors a skill-provided .skillignore / .clawhubignore
(gitignore-style) so dev/docs artifacts shipped in a skill root are excluded
from both structural checks and pattern scanning. SKILL.md is never ignorable.

80 tests pass (64 existing + 16 new).
donbowman pushed a commit to donbowman/hermes-agent that referenced this pull request Jul 13, 2026
…gnore files (NousResearch#36231)

The skill security scanner blocked legitimate community skills on three
intrinsic false-positive patterns:

- read_secrets_file matched `cat > file.env <<` heredocs (writing the
  user's own keys into their own local .env), not just `cat file.env`
  reads. Exclude output redirections.
- allowed-tools frontmatter is REQUIRED by the agent-skill spec; every
  compliant skill declares it. Drop from HIGH privilege_escalation to a
  LOW informational finding so it no longer drives the verdict.
- python_os_environ flagged `os.environ.get("CONFIG_VAR")` config reads
  as HIGH exfiltration. Exempt non-secret `.get()` reads; add a dedicated
  CRITICAL python_environ_get_secret pattern so secret-named reads
  (OPENAI_API_KEY etc.) are still caught.

Also: scan_skill() now honors a skill-provided .skillignore / .clawhubignore
(gitignore-style) so dev/docs artifacts shipped in a skill root are excluded
from both structural checks and pattern scanning. SKILL.md is never ignorable.

80 tests pass (64 existing + 16 new).
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…gnore files (NousResearch#36231)

The skill security scanner blocked legitimate community skills on three
intrinsic false-positive patterns:

- read_secrets_file matched `cat > file.env <<` heredocs (writing the
  user's own keys into their own local .env), not just `cat file.env`
  reads. Exclude output redirections.
- allowed-tools frontmatter is REQUIRED by the agent-skill spec; every
  compliant skill declares it. Drop from HIGH privilege_escalation to a
  LOW informational finding so it no longer drives the verdict.
- python_os_environ flagged `os.environ.get("CONFIG_VAR")` config reads
  as HIGH exfiltration. Exempt non-secret `.get()` reads; add a dedicated
  CRITICAL python_environ_get_secret pattern so secret-named reads
  (OPENAI_API_KEY etc.) are still caught.

Also: scan_skill() now honors a skill-provided .skillignore / .clawhubignore
(gitignore-style) so dev/docs artifacts shipped in a skill root are excluded
from both structural checks and pattern scanning. SKILL.md is never ignorable.

80 tests pass (64 existing + 16 new).
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
…gnore files (NousResearch#36231)

The skill security scanner blocked legitimate community skills on three
intrinsic false-positive patterns:

- read_secrets_file matched `cat > file.env <<` heredocs (writing the
  user's own keys into their own local .env), not just `cat file.env`
  reads. Exclude output redirections.
- allowed-tools frontmatter is REQUIRED by the agent-skill spec; every
  compliant skill declares it. Drop from HIGH privilege_escalation to a
  LOW informational finding so it no longer drives the verdict.
- python_os_environ flagged `os.environ.get("CONFIG_VAR")` config reads
  as HIGH exfiltration. Exempt non-secret `.get()` reads; add a dedicated
  CRITICAL python_environ_get_secret pattern so secret-named reads
  (OPENAI_API_KEY etc.) are still caught.

Also: scan_skill() now honors a skill-provided .skillignore / .clawhubignore
(gitignore-style) so dev/docs artifacts shipped in a skill root are excluded
from both structural checks and pattern scanning. SKILL.md is never ignorable.

80 tests pass (64 existing + 16 new).
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
…gnore files (NousResearch#36231)

The skill security scanner blocked legitimate community skills on three
intrinsic false-positive patterns:

- read_secrets_file matched `cat > file.env <<` heredocs (writing the
  user's own keys into their own local .env), not just `cat file.env`
  reads. Exclude output redirections.
- allowed-tools frontmatter is REQUIRED by the agent-skill spec; every
  compliant skill declares it. Drop from HIGH privilege_escalation to a
  LOW informational finding so it no longer drives the verdict.
- python_os_environ flagged `os.environ.get("CONFIG_VAR")` config reads
  as HIGH exfiltration. Exempt non-secret `.get()` reads; add a dedicated
  CRITICAL python_environ_get_secret pattern so secret-named reads
  (OPENAI_API_KEY etc.) are still caught.

Also: scan_skill() now honors a skill-provided .skillignore / .clawhubignore
(gitignore-style) so dev/docs artifacts shipped in a skill root are excluded
from both structural checks and pattern scanning. SKILL.md is never ignorable.

80 tests pass (64 existing + 16 new).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have tool/skills Skills system (list, view, manage) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants