feat(analyzer): implement MCP rug-pull detection (RP1-RP3)

[mimran-khan]

Summary

The mcp_rug_pull analyzer was a non-functional stub that unconditionally returned empty findings — indistinguishable from "checked and found nothing" in scan output. This gave users a false sense of security when scanning MCP-enabled skills.

This PR implements actual rug-pull detection with two analysis modes:

Manifest Comparison (when previous_manifest is supplied)

Rule	Detection	Severity
RP1	New parameters added to tools	HIGH
RP2	Tool/skill description changed	MEDIUM
RP3	Parameters removed from tools	MEDIUM

Static Risk Analysis (always runs)

Rule	Detection	Severity
RP1	Wildcard (*) permissions	HIGH
RP2	Dynamic tool loading patterns (fetch_tools, runtime discovery)	MEDIUM

Key Behavior Changes

1. No more silent no-ops: When previous_manifest is unavailable, the analyzer now logs a WARNING instead of silently producing "0 findings"
2. Actual detection: Identifies concrete rug-pull risk indicators
3. Actionable findings: Each finding includes specific remediation guidance

How Rug-Pull Detection Works

First scan:   manifest A → stored as baseline
Second scan:  manifest B → compared against A
                          → RP1 if new params added (data capture)
                          → RP2 if descriptions changed (prompt injection)
                          → RP3 if params removed (behavior divergence)

Static analysis runs on every scan regardless, catching:

• Overly broad permissions that amplify rug-pull impact
• Dynamic tool loading that makes definitions unpredictable between scans

Testing

15 new tests covering:

• RP1: New parameter detection
• RP2: Description change detection
• RP3: Parameter removal detection
• No-change produces no findings
• Empty manifests handled gracefully
• Wildcard permission flagged
• Specific permissions pass
• Dynamic loading patterns caught
• Normal code not flagged
• Node integration with/without previous_manifest
• Warning logged when comparison unavailable

Fixes #150

[rng1995]

Verdict: Request changes — the analyzer is a real improvement over the stub and the tests are good, but there's a likely duplicate PR, inconsistent rule-ID semantics, and the headline manifest-diff path is unreachable.

Blocking / please clarify

1. Possible duplicate of the still-open #125 (identical title, "implement MCP rug-pull detection (RP1-RP3)"). Only one should proceed — please confirm which supersedes so we don't merge both.
2. Inconsistent rule-ID semantics. In _compare_manifests (src/skillspector/nodes/analyzers/mcp_rug_pull.py): RP1 = new parameter (~L63), RP3 = removed parameter (~L92), RP2 = description change (~L121). But in _static_risk_analysis: RP1 = wildcard permission (~L168) and RP2 = dynamic loading (~L202). The same rule_id carries two different meanings, which produces inconsistent SARIF rule descriptors — note the rules[] dedup added in #158 keeps only the first message seen per id, so one of the two meanings will be silently mislabeled. Give each distinct concept its own rule id.
3. Manifest-comparison path is unreachable. node() reads state.get("previous_manifest") (~L240) but nothing in this PR populates it — there's no --previous-manifest CLI flag or state plumbing. So in practice previous_manifest is always None, only _static_risk_analysis runs, and the headline RP1/RP2/RP3 manifest-diffing never fires (confirmed by test_without_previous_manifest_warns). Please add the plumbing or clarify the follow-up that wires it.

Tests

Unit tests for both _compare_manifests and _static_risk_analysis are good. They don't catch (2)/(3) because they call the helpers directly / inject previous_manifest into state by hand, bypassing the missing CLI/state wiring.

[mimran-khan]

Thanks for the review @rng1995 — all three blocking points are valid.

After checking, PR #125 by @tcconnally covers the same scope with cleaner rule-ID semantics (each RP1/RP2/RP3 has a single consistent meaning) and was opened first. Our manifest-comparison path here is unreachable without CLI plumbing that doesn't exist yet.

Closing this in favor of #125. If the manifest-diffing logic is useful down the road, it can be proposed as a follow-up once #125 lands and the state/CLI plumbing is in place.