T09 · Insecure Skill Coding Practices
- Location
findskills.py:818- Finding
Community skill updates unconditionally bypass the P0 security block
- Content
View full analysis
Vulnerability Details
File Location:
findskills.py, lines 818–831
Vulnerability Type: Security-gate bypass during remote package updates
Risk Level: HighComplete Code Snippet
python def cmd_update(args) -> int: if args.offline: print("update requires network access", file=sys.stderr) return 1 local = discover_local() matches = [s for s in local if s["slug"] == args.slug or normalize_key(s["name"]) == normalize_key(args.slug)] if not matches: print("The requested skill is not installed", file=sys.stderr) return 1 rc = 0 for s in matches: target = Path(s["path"]).parent rc = _fetch_and_install( args.slug, target, True, args.yes, action="\u66f4\u65b0" ) or rc return rcThe corresponding security check in
_fetch_and_installis:python if report["risk_level"] == "P0" and not force: print(security_scan.render_markdown(report, "zh-CN"), end="") return 2Technical Analysis
_fetch_and_installexpects itsforceparameter to indicate whether a P0 finding may be overridden. The normal CLI exposes this decision throughargs.force, butcmd_updatedoes not passargs.force. It passes the constantTrue.Consequently, the expression
report["risk_level"] == "P0" and not forceis always false during an update. A remotely downloaded update is therefore installed even when the scanner identifies a P0 condition such as credential-file access, dangerous command execution, encoded execution, or a download-and-execute sequence.This contradicts the documented security policy that P0 or EXTREME packages are blocked. It also makes the update path less secure than the initial installation path.
The P1 check still uses
args.yes, but that does not mitigate the unconditional P0 bypass.Attack Path
- A user initially installs a legiti ...[truncated 1558 chars]
- Remediation
View remediation
Remediation Suggestions
-
Pass the actual CLI option instead of an unconditional constant:
python rc = _fetch_and_install( args.slug, target, args.force, args.yes, action="\u66f4\u65b0" ) or rc -
Prefer making P0 findings non-overridable if the documented policy is that EXTREME packages must never be installed.
-
If an override remains supported, require an explicit, narrowly scoped confirmation that identifies the slug, detected findings, source, and destination. Do not infer approval from unrelated flags.
-
Add regression tests covering:
- P0 update without
--force: must fail before replacing files. - P0 update with an explicitly supported override: must follow the documented policy.
- P1 update without
--yes: must fail. - Clean update: must succeed.
- P0 update without
-
Install updates transactionally: scan into a temporary directory, validate the package, preserve the existing version, and atomically replace it only after all gates pass.
-