E2 "Env Variable Harvesting" matches one spelling, so whitespace alone flips DO_NOT_INSTALL to SAFE

[shi-eric]

Summary

Rule E2 (Data Exfiltration / Env Variable Harvesting) appears to match a literal os.environ.copy() rather than the behavior it names. Every semantically identical way of copying the environment goes undetected, including inserting spaces around the dots.

Because E2 is emitted at fixed HIGH, its presence alone moves the published risk assessment from MEDIUM / CAUTION to HIGH / DO_NOT_INSTALL. The rule penalizes idiomatic code while passing every form an actual credential harvester would use.

Environment

• SkillSpector v2.5.1
• skillspector scan <dir> --no-llm --format json (static-only, so the results are deterministic)
• Linux, Python 3.12

Reproduction

A minimal skill: one SKILL.md plus one script. Only the marked expression varies between runs.

import os
import subprocess


def run(command):
    env = os.environ.copy()          # <-- only this line varies
    env["CACHE_PATH"] = "/tmp/cache"
    return subprocess.run(command, env=env, check=False)

Scanning each variant:

Expression	E2 fires	Score	Severity	Recommendation
os.environ.copy()	yes	52	HIGH	DO_NOT_INSTALL
dict(os.environ)	no	33	MEDIUM	CAUTION
{**os.environ}	no	33	MEDIUM	CAUTION
dict(os.environ.items())	no	33	MEDIUM	CAUTION
__import__("copy").copy(os.environ)	no	43	MEDIUM	CAUTION
os . environ . copy ()	no	9	LOW	SAFE

All six do the same thing. The last row states the problem most plainly: inserting PEP8-irrelevant whitespace into the same expression takes a skill from DO_NOT_INSTALL to SAFE.

This also rules out a whitespace-tolerant pattern such as os\.environ\s*\.\s*copy\s*\(\). That regex would match the spaced variant, and it does not match.

Expected behavior

Detection should key on the behavior, reading the full environment mapping, rather than one spelling of it. An AST-level check for reads of os.environ as a whole would cover all six variants uniformly.

Actual behavior

Only the exact os.environ.copy() token sequence is flagged:

ID: E2
CATEGORY: Data Exfiltration
PATTERN: Env Variable Harvesting
SEVERITY: HIGH
CONFIDENCE: 0.6
FINDING: os.environ.copy()
EXPLANATION: Code accesses environment variables that may contain secrets (API keys, tokens). This is a common pattern for credential theft.

Why this matters

The rule inverts its own intent. Code written the obvious way is blocked, and code written any other way passes. Anyone actually exfiltrating credentials writes dict(os.environ) and scores CAUTION.

The cost lands on honest code. A skill that copies the environment solely so a child process inherits it with a few cache paths redirected is flagged at HIGH, which is enough to publish DO_NOT_INSTALL. From there the only ways forward are carrying a baseline fingerprint for a finding that was reviewed and understood, or rewriting the expression into a spelling the rule does not match. The second option changes nothing about what the code does or what it can reach. A security scanner should not be producing diffs like that.

Related, same area

Fixed HIGH from a 0.6-confidence match. E2 reports confidence: 0.6 but is emitted at HIGH regardless, and severity is what drives the headline recommendation. Consider scaling severity by confidence, or holding regex-only findings below the level that produces DO_NOT_INSTALL. (Consumers compound this: SkillEvaluator's SecurityValidator treats any HIGH as a blocking error and never reads the confidence field it already parses. Filing separately against skill-evaluator; that issue and this one should be fixed independently.)

No exemption path short of a baseline. skillspector scan --help exposes --baseline as the only suppression mechanism. There is no per-rule severity override and no inline exemption. Recording "reviewed, this is fine" therefore requires a fingerprint file that must be regenerated whenever the surrounding file changes, which is heavy for a single audited false positive.

LP1 looks spelling-sensitive too. In the same experiment, LP1 (MCP Least Privilege, "uses 'env' capability that is not listed in its permissions", HIGH, confidence 0.75) fires on os.environ.copy() and on dict(os.environ), but disappears on os . environ . copy (). The capability detector appears to share the literal-matching weakness, so the fix may belong at a shared layer rather than in E2 alone. LP1's separate problem, that the Agent Skills spec defines no permissions field, is already tracked in #300.

Suggested fix

1. Replace the literal match with an AST visitor that detects reads of os.environ as a whole mapping, covering .copy(), dict(), {**...}, .items(), and aliased imports.
2. Derive severity from confidence, or hold regex-only matches below the level that flips the recommendation to DO_NOT_INSTALL.
3. Offer a lighter-weight exemption than a fingerprint baseline for reviewed findings.