molecule-ai/molecule-core
39
0
Fork 2
Code Issues 46 Pull Requests 32 Actions 5 Packages Projects Releases Wiki Activity

test(mcp): rewrite GlobalScope_Blocked to assert OFFSEC-001 scrub contract (mc#664 Class 2) #680

Open
core-lead wants to merge 1 commits from fix/mc-664-class-2-mcp-offsec-contract-test into main
pull from: fix/mc-664-class-2-mcp-offsec-contract-test
merge into: molecule-ai:main
Conversation 3 Commits 1 Files Changed 1 +68 -8

1 Commits

Author SHA1 Message Date
Molecule AI Core-Security
9cb7cf70e3 test(mcp): rewrite GlobalScope_Blocked to assert OFFSEC-001 scrub contract (mc#664 Class 2)
Some checks failed
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 14s
Details
Harness Replays / detect-changes (pull_request) Successful in 19s
Details
CI / Detect changes (pull_request) Successful in 41s
Details
E2E API Smoke Test / detect-changes (pull_request) Successful in 46s
Details
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 46s
Details
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 19s
Details
Handlers Postgres Integration / detect-changes (pull_request) Successful in 51s
Details
sop-checklist / all-items-acked (pull_request) acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4 — body-unfilled: 7
Details
qa-review / approved (pull_request) Failing after 20s
Details
security-review / approved (pull_request) Failing after 22s
Details
sop-checklist-gate / gate (pull_request) Successful in 21s
Details
gate-check-v3 / gate-check (pull_request) Failing after 35s
Details
sop-tier-check / tier-check (pull_request) Successful in 20s
Details
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 54s
Details
Harness Replays / Harness Replays (pull_request) Successful in 9s
Details
CI / Shellcheck (E2E scripts) (pull_request) Successful in 10s
Details
CI / Canvas (Next.js) (pull_request) Successful in 11s
Details
CI / Python Lint & Test (pull_request) Successful in 12s
Details
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 12s
Details
CI / Canvas Deploy Reminder (pull_request) Has been skipped
Details
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m35s
Details
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 13s
Details
E2E API Smoke Test / E2E API Smoke Test (pull_request) Failing after 5m24s
Details
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 5m56s
Details
CI / Platform (Go) (pull_request) Failing after 15m44s
Details
CI / all-required (pull_request) Failing after 7s
Details 
Background — chain of defects
-----------------------------
mc#664 (Platform (Go) CI red) decomposes into:
  • Class 1 — 4 TestExecuteDelegation_* failures (parallel dispatch to core-be)
  • Class 2 — TestMCPHandler_CommitMemory_GlobalScope_Blocked (this PR)

Class 2 root cause: commit 7d1a189f (2026-05-10) hardened mcp.go to scrub
err.Error() out of the JSON-RPC error.message returned to the client,
replacing the third leak (the dispatchRPC tool-call branch, line ~427)
with the constant string "tool call failed". The internal error is now
log.Printf'd server-side only.

The existing test at mcp_test.go:432 asserted that the client-visible
message CONTAINED the substring "GLOBAL" — which was exactly the
internal err.Error() text the 7d1a189f scrub now removes. So the test
had silently flipped from "verifies behaviour" to "verifies the bug",
and once the scrub landed the test went red. PR #665 has been masking
this red via continue-on-error as an interim measure; this PR is the
proper fix for Class 2.

Wrong fix
---------
Un-scrub mcp.go (i.e. restore err.Error() into the client-facing
message). This would re-open OFFSEC-001 / #259 and defeat the security
hardening that was applied uniformly across 22 sibling files in
PRs #1193 / #1206 / #1219 / #168.

Right fix (this PR)
-------------------
Rewrite the test so it asserts the OFFSEC-001 scrub-works contract
on this very code path, matching the same style used by the four
canonical OFFSEC-001 tests already in this file (lines 1031–1149):

  • exact-equality on resp.Error.Code (-32000)
  • exact-equality on resp.Error.Message ("tool call failed")
  • negative-substring canaries on six tokens from the production-internal
    error string ("GLOBAL", "scope", "permitted", "bridge", "LOCAL", "TEAM")
    — if ANY leaks through to the client, the scrub has regressed and the
    test fires immediately
  • C3 invariant preserved (no DB calls — handler short-circuits)
  • Test renamed to _ScrubsInternalError so the contract is visible at
    the call site / in failure output

Per feedback_assert_exact_not_substring: the positive assertion uses
exact-equality (`!= "tool call failed"`) rather than substring-match,
so any future mutation of the constant breaks the test loudly.

Verification (local, falsified both ways)
-----------------------------------------
  Positive: against current main (7d1a189f scrub in place)
    $ go test -run TestMCPHandler_CommitMemory_GlobalScope_Blocked_ScrubsInternalError
    ok      .../internal/handlers   0.515s  PASS

  Falsification: temporarily reverted line 427 of mcp.go to
  `Message: err.Error()`, ran the test → all positive assertions failed
  AND all six leaked-token canaries fired (proves the test really does
  guard the contract, not just shape).

All other TestMCPHandler_* tests continue to pass. The four
TestExecuteDelegation_* failures observed in the full handlers/
package run pre-exist on origin/main and are Class 1 (core-be's
parallel work) — not touched here.

Tier
----
tier:high — this is the security-hardening contract test for the
OFFSEC-001 scrub. A weak version of this assertion is what allowed
the original gap on the GLOBAL-scope path to go unnoticed for so long.

Brief-falsification log
-----------------------
  • Brief halt-condition: "If reading of 7d1a189f differs from this
    brief's account: STOP" — confirmed identical (3rd hunk, line 425 in
    pre-patch mcp.go, dispatchRPC tool-call branch, scrubs err.Error()
    → "tool call failed", logs server-side).
  • Brief halt-condition: "If mcp_test.go line 433 has been modified
    since this brief was written: STOP" — confirmed unchanged
    (line 432–434 exact text matches brief description).
  • Brief widen-scope check: searched for sibling tests with the same
    anti-pattern (assert internal err.Error() content on the OFFSEC
    code path). Findings:
      – TestMCPHandler_RecallMemory_GlobalScope_Blocked (line 539)
        asserts `resp.Error != nil` only; does NOT assert on
        "GLOBAL"-substring, so it isn't broken by the scrub. BUT it
        also doesn't verify the scrub-works contract — a future
        regression would slip past it. Recommending a follow-up to
        strengthen it (and the corresponding RecallMemory v2 path,
        if any) in a separate single-purpose PR rather than widening
        scope here. NOT addressed in this PR per the brief's
        "1-2 siblings or report" discipline.
  • OFFSEC-001 issue lookup: 22 files were touched by the sibling
    scrub PRs (#1193 / #1206 / #1219 / #168). This PR addresses ONE
    test that was asserting against the now-scrubbed surface. No
    other red-on-main tests are believed to share this anti-pattern
    in mcp_test.go (grep verified).

References
----------
  • mc#664 (Platform (Go) red — chain root issue)
  • PR #665 (interim continue-on-error mask — to be reverted post-fix)
  • commit 7d1a189f (OFFSEC-001 scrub, the hardening this test now guards)
  • OFFSEC-001 / molecule-ai/molecule-core#259 (original security issue)
  • feedback_assert_exact_not_substring (assertion-style memory)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
 2026-05-12 09:18:27 +00:00