_call_is_network_sink missed the HEAD/OPTIONS verbs on requests/httpx,
socket.create_connection, and urllib.request.urlretrieve. A bulk env dump
or reverse shell shipped through any of these slipped past the CRITICAL
exfil/reverse-shell rules whenever the URL was assembled at runtime (the
non-literal case the string-literal URL check can't cover). Also treat
socket.create_connection as the socket primitive in the reverse-shell shape.
http.client.HTTP(S)Connection is intentionally left out: only the lazy
constructor is statically visible (the request()/connect() that performs the
I/O is an instance method the call-name analyzer can't resolve), so flagging
the constructor would hard-block benign code that only builds a connection
object.
Cover the alias-resolved forms too: the sink check runs on the name after
from-import / import-as resolution, a path the suite exercised only on the
env-read side (#4087) and not on the sink side.
python-env-dump-exfil flags a file that both reads the bulk process environment
and reaches a network sink. The call-based sink check only listed requests
get/post/put/request and httpx get/post, so a bulk env dump sent through an
equally body-carrying method (requests.patch/delete, httpx.put/patch/delete, or
the generic httpx.request/stream) evaded the CRITICAL finding whenever the
destination URL was not a plain string literal (e.g. built at runtime) -- the
exact evasion the string-literal URL sink is meant to resist. requests.post was
caught but requests.patch was not, an arbitrary gap on clients the analyzer
already covers.
Complete the requests and httpx HTTP-verb surface in _call_is_network_sink so an
obfuscated-URL exfil through those methods is flagged like post/put.
Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
* Add phase 1 skill static scanning
* Rework SkillScan phase 1 as native scanner
* refactor(skillscan): align phase 1 with trimmed RFC contract
- SecurityFinding: 7 fields (rule_id, severity, file, line, message,
remediation, evidence); category/analyzer derive from the rule_id
prefix, confidence/column/fingerprint/metadata removed
- scan_archive_preflight()/scan_skill_dir() are pure functions: no
ScanContext, no policy schema; CRITICAL-blocks is a code constant and
skill_scan.enabled is applied by enforce_static_scan()/callers
- secret-* evidence is redacted before findings leave the scanner
- de-dup keys on (rule_id, file, line) so repeated occurrences keep
distinct locations for agent self-correction
- cloud-metadata detection consolidated into network-cloud-metadata
- nested zip members get a one-level stdlib magic-byte peek; an
executable member escalates package-nested-archive to CRITICAL
- install metadata sidecar removed (Phase 7 decides if it is needed)
- rule specs moved next to their analyzers; skillscan/rules/ removed
- tests updated + new anchors: redaction, dedup lines, nested-zip
escalation, single cloud-metadata rule, bundled-skill zero-CRITICAL
* fix(skillscan): tighten reverse-shell/secret/archive scan rules from review
Address PR #3033 review feedback on the native SkillScan analyzers:
- Reverse-shell false positives: split shell detection by signal strength
(/dev/tcp/, nc -e stay CRITICAL; bash -i, mkfifo -> new HIGH
shell-reverse-shell-heuristic, warn->LLM). The Python check is now
AST-anchored on real socket.socket/os.dup2/subprocess call sites instead
of raw-text substring matching, so prose/docstrings no longer hard-block.
- Secret evidence: _redact_secret_evidence returns [redacted] with no secret
bytes (was value[:6], which leaked 2 real token bytes past the prefix).
- Archive DoS: cap outer archive member count (_MAX_ARCHIVE_MEMBERS=4096);
scan_archive_preflight early-aborts with a package-too-many-members CRITICAL
finding (routes through the existing blocked->400 fail-closed path).
- shell-destructive-command: broaden the rm -rf matcher to sensitive system
roots (/home, /usr, /*, --no-preserve-root /) while leaving safe subpaths
unflagged.
- Dead code: collapse _decode_text_for_analysis to a single decode path and
drop the unused _TEXT_SUFFIXES set and _has_text_shebang helper.
- local_skill_storage: document why the host_path branch keeps app_config
possibly-None (lazy kill-switch resolution; avoids eager get_app_config in
config-free environments such as CI).
Tests: new negative/positive coverage in test_skillscan_native.py. Full
backend suite 6616 passed, 26 skipped.
