feat(security): Spec 076 US1 — 3 hard detect checks + wire scanner (T009-T012)#770
Merged
Conversation
…009-T012) Implement the MVP of the deterministic offline tool-scanner (Spec 076 US1): three near-zero-FP HARD checks delegated into the live tpa-descriptions scan. - detect/checks/unicode_hidden.go: flags zero-width / bidi / TAG-block / PUA runes in RAW description+schema text; escalates to near-certain when >=3 hidden classes are present or TAG-block chars decode to a printable message. - detect/checks/shadowing.go: flags distinctive cross-server name collisions and descriptions that reference another server's distinctive tool; ignores self-reference and generic-verb collisions. - detect/checks/payload_decoded.go: decodes base64/hex blobs and flags only when the decoded bytes are a shell/exfil command (curl|sh, chmod, rm -rf, raw IP:port); benign/binary decodes are skipped. Evidence = decoded content. - scanner/inprocess.go: builds a detect.RegistryView from the server's tool set, runs detect.Engine with the three checks, converts detect.Finding -> ScanFinding 1:1 (additive confidence/signals carried through). CLI/REST/MCP entry points unchanged. Legacy phrase rules + embedded-secret detection are retained until US2 lands detect's directive.imperative/secret.embedded, so the MVP regresses no existing coverage. TDD: per-check MUST-flag/MUST-NOT-flag tests written first; scanner-path and integration smoke tests assert the new finding shape. Offline import-guard still green (checks subpackage imports only detect + stdlib). Related #MCP-3576
Deploying mcpproxy-docs with
|
| Latest commit: |
427b3c1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://96981c3c.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://076-t2-mvp-checks.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 28279235674 --repo smart-mcp-proxy/mcpproxy-go
|
This was referenced Jun 26, 2026
Merged
… (Codex #770) CodexReviewer found that the in-process scanner stamped every detect.ToolView with the same serverName (only the scanned server's tools.json), so the shadowing.cross_server check — which only emits when a collision/reference points at a *different* server — could never fire end-to-end. Spec 076 FR-003 was unsatisfied for that check. Fix: feed the engine a real cross-server RegistryView. - New optional capability `allServerToolsProvider` (GetAllServerTools); the Service builds a peer snapshot (every other server's current tools) and carries it on ScanRequest.PeerTools. Implemented on configServerInfoProvider by iterating the live config's servers. Optional type-assertion → zero blast radius on the required ServerInfoProvider interface and its mocks. - inProcessToolScan/detectEngineFindings now tag each ToolView with its TRUE owning server (scanned server + peers), deterministically ordered, and filter emitted findings back to the scanned server (peers are context only). Tests (regression locked end-to-end, not just the check in isolation): - TestInProcessToolScan_ShadowingCrossServerThroughAdapter: peers present → shadowing.cross_server fires; no peers → it does not (the bug state). - TestEngineInProcessScan_ShadowingViaPeerTools: full StartScan path proves ScanRequest.PeerTools threads through the live adapter. Verification: go test -race ./internal/security/..., -tags integration E2E_TPA, golangci-lint v2 on scanner+server (0 issues), go build ./... — all green. Related #MCP-3576 Co-Authored-By: Paperclip <noreply@paperclip.ing>
…770) CodexReviewer's second finding: the in-process tpa-descriptions scanner dropped each tool's outputSchema, so a hidden-Unicode / decoded-payload / directive payload smuggled into the OUTPUT schema was invisible — even though Spec 076 FR-001 scans name+description+inputSchema+outputSchema and the detect checks already inspect ToolView.OutputSchema. - toolDef now parses `outputSchema`. - Legacy phrase + embedded-secret text concatenation includes the output schema. - The detect adapter populates ToolView.OutputSchema, so the structural checks (unicode.hidden / payload.decoded) see it via the engine too. Test: TestInProcessToolScan_DetectEngineOutputSchemaPayload — a base64 curl|sh blob placed only in outputSchema is flagged payload.decoded end-to-end. Verification: go test -race ./internal/security/..., golangci-lint v2 (0 issues), go build ./... — all green. Related #MCP-3576 Co-Authored-By: Paperclip <noreply@paperclip.ing>
![mcpproxy-gatekeeper[bot]](https://avatars.githubusercontent.com/u/216428285?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Gatekeeper approval — Codex review verdict: ACCEPT.
This approval is posted automatically by the MCPProxy Gatekeeper App on behalf of the Codex reviewer (verdict of record lives in the Paperclip review thread). Author≠approver satisfied; QA + CI gates enforced separately.
Auto-approved per Model B (MCP-1249).
Dumbris
added a commit
that referenced
this pull request
Jun 28, 2026
…confidence (MCP-3577)
Adds the US2 false-positive-discriminating SOFT checks to the Spec-076
offline detect engine, plus per-match confidence on the reused secret
matchers. Soft signals raise a finding for review and never auto-quarantine.
- T013 checks/directive_imperative.go: prompt-injection directives
(<IMPORTANT> tags, 'do not tell the user', 'ignore previous
instructions', 'before using this tool') matched over NORMALIZED text
with position discounting so example-position mentions are suppressed.
- T014 checks/capability_mismatch.go: declared-vs-implied capability gap
(a compute/string tool touching ~/.ssh, /etc/passwd, a URL or shell)
plus an unexplained data-sink param ('sidenote'); legitimate file/network
tools are not flagged.
- T015 internal/security/patterns: additive per-match confidence
(WithConfidence builder + ConfidenceFor) — Luhn-validated card 0.95,
generic bearer 0.3, documented examples 0.1, severity defaults otherwise.
Existing Match/IsValid/Scan behavior is unchanged.
- T016 checks/embedded_secret.go: wraps the patterns matchers with
confidence and masked evidence, skipping documented placeholders; the
three soft checks are registered in the scanner detect-engine wiring.
TDD with MUST-flag and hard-negative MUST-NOT-flag cases for each check.
Coordination: detectEngineFindings in inprocess.go is the shared US1/US2
integration point; this branch registers the three SOFT checks and the
Checks slice is the merge point with US1's hard checks (#770).
Dumbris
added a commit
that referenced
this pull request
Jun 28, 2026
…confidence (MCP-3577) (#775) Adds the US2 false-positive-discriminating SOFT checks to the Spec-076 offline detect engine, plus per-match confidence on the reused secret matchers. Soft signals raise a finding for review and never auto-quarantine. - T013 checks/directive_imperative.go: prompt-injection directives (<IMPORTANT> tags, 'do not tell the user', 'ignore previous instructions', 'before using this tool') matched over NORMALIZED text with position discounting so example-position mentions are suppressed. - T014 checks/capability_mismatch.go: declared-vs-implied capability gap (a compute/string tool touching ~/.ssh, /etc/passwd, a URL or shell) plus an unexplained data-sink param ('sidenote'); legitimate file/network tools are not flagged. - T015 internal/security/patterns: additive per-match confidence (WithConfidence builder + ConfidenceFor) — Luhn-validated card 0.95, generic bearer 0.3, documented examples 0.1, severity defaults otherwise. Existing Match/IsValid/Scan behavior is unchanged. - T016 checks/embedded_secret.go: wraps the patterns matchers with confidence and masked evidence, skipping documented placeholders; the three soft checks are registered in the scanner detect-engine wiring. TDD with MUST-flag and hard-negative MUST-NOT-flag cases for each check. Coordination: detectEngineFindings in inprocess.go is the shared US1/US2 integration point; this branch registers the three SOFT checks and the Checks slice is the merge point with US1's hard checks (#770).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Spec 076 · T2 / US1 (MVP) — MCP-3576
Implements the MVP of the deterministic, offline tool-scanner (Spec 076): three near-zero-FP HARD structural checks, delegated into the live
tpa-descriptionsscan viadetect.Engine(T1 foundation, #769). Builds offorigin/main(T1 is merged there).What's in (T009–T012)
detect/checks/unicode_hidden.go— flags zero-width / bidi / TAG-block / PUA runes in raw description+schema text; escalates to near-certain (critical) when ≥3 hidden classes are present or TAG-block chars decode to a printable ASCII message (evidence = the decoded message). Runs on raw text deliberately —Normalizestrips format runes.detect/checks/shadowing.go— flags distinctive cross-server tool-name collisions and descriptions that reference another server's distinctive tool. Ignores self-reference and generic-verb collisions (search,get, …) to hold FP near zero.detect/checks/payload_decoded.go— decodes base64/hex blobs and flags only when the decoded bytes are a shell/exfil command (curl … | sh,chmod,rm -rf, rawIP:port). Benign/binary decodes (icons, JSON) and short tokens are skipped. Evidence = decoded content.scanner/inprocess.go— builds adetect.RegistryViewfrom the server's tool set, runsdetect.Enginewith the three checks, convertsdetect.Finding → ScanFinding1:1 (additiveconfidence+signalscarried through). CLI/REST/MCP entry points unchanged (FR-015).Design decision — migration boundary (no regression)
US1 owns only the three structural checks;
directive.imperative(T013/US2) andsecret.embeddedare later tasks. To keep the MVP from regressing existing coverage, the legacy phrase rules (tpaRules) and embedded-secret detection remain alongside the engine until US2 lands their detect equivalents, at which pointinProcessToolScanfully delegates. Documented in code + the commit. Cross-server shadowing only sees this server's tools at this wiring point (per-server scan) — the check is correct; a future task can feed a global registry.Tests (TDD — failing test first)
*_test.go) per the contract.signals/confidence) flows throughinProcessToolScan.e2e_tpa_smoke_test.go,-tags integration) proves a base64curl|shpayload is flagged end-to-end throughStartScan.checkssubpackage imports onlydetect+ stdlib).Verification
```
go test -count=1 -race ./internal/security/detect/... ./internal/security/scanner/ # ok
go test -count=1 -tags integration ./internal/security/scanner/ -run E2E_TPA # ok
golangci-lint run --config .github/.golangci.yml ./internal/security/detect/... ./internal/security/scanner/... # 0 issues
go build ./... # ok
```
Docs:
docs/features/security-scanner-plugins.mdupdated to describe the new structural checks +confidence/signals(ENG-9).Related #MCP-3576