fix(test/mcp): align RecallMemory_GlobalScope with OFFSEC-001 scrub contract
Some checks failed
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 11s
CI / Detect changes (pull_request) Successful in 34s
E2E API Smoke Test / detect-changes (pull_request) Successful in 33s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 29s
Harness Replays / detect-changes (pull_request) Successful in 14s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 23s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 10s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 20s
qa-review / approved (pull_request) Failing after 13s
sop-checklist / all-items-acked (pull_request) [soft-fail tier:low] acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4 — body-unfilled: 7
security-review / approved (pull_request) Failing after 11s
sop-checklist-gate / gate (pull_request) Successful in 10s
gate-check-v3 / gate-check (pull_request) Successful in 19s
sop-tier-check / tier-check (pull_request) Successful in 11s
CI / Canvas (Next.js) (pull_request) Successful in 5s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 4s
CI / Python Lint & Test (pull_request) Successful in 4s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m7s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 5s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 1m24s
Harness Replays / Harness Replays (pull_request) Successful in 4s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 4s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 4m30s
CI / Platform (Go) (pull_request) Successful in 11m3s
CI / all-required (pull_request) Successful in 4s
Some checks failed
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 11s
CI / Detect changes (pull_request) Successful in 34s
E2E API Smoke Test / detect-changes (pull_request) Successful in 33s
E2E Staging Canvas (Playwright) / detect-changes (pull_request) Successful in 29s
Harness Replays / detect-changes (pull_request) Successful in 14s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 23s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 10s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 20s
qa-review / approved (pull_request) Failing after 13s
sop-checklist / all-items-acked (pull_request) [soft-fail tier:low] acked: 0/7 — missing: comprehensive-testing, local-postgres-e2e, staging-smoke, +4 — body-unfilled: 7
security-review / approved (pull_request) Failing after 11s
sop-checklist-gate / gate (pull_request) Successful in 10s
gate-check-v3 / gate-check (pull_request) Successful in 19s
sop-tier-check / tier-check (pull_request) Successful in 11s
CI / Canvas (Next.js) (pull_request) Successful in 5s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 4s
CI / Python Lint & Test (pull_request) Successful in 4s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m7s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 5s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 1m24s
Harness Replays / Harness Replays (pull_request) Successful in 4s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 4s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 4m30s
CI / Platform (Go) (pull_request) Successful in 11m3s
CI / all-required (pull_request) Successful in 4s
The test was asserting that the client-visible error.message equals the
descriptive internal reason ("GLOBAL scope is not permitted via the MCP
bridge"). After PR#680 and PR#772 enforced the OFFSEC-001 scrub contract
across all tool-dispatch failure paths, mcp.go returns the constant
"tool call failed" to callers — not the internal detail.
Update the test to:
- Rename to ..._Blocked_ScrubsInternalError (consistent with CommitMemory)
- Assert error.message == "tool call failed" (OFFSEC-001 positive)
- Add negative assertions (no internal tokens leak to client)
- Use proper json.Unmarshal error check
- Merge origin/main (PR#691 lint-required-context-exists-in-bp)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
f70188f00b
commit
23e408379d
@ -608,13 +608,13 @@ func TestMCPHandler_CommitMemory_CleanContent_PassesThrough(t *testing.T) {
|
||||
// tools/call — recall_memory
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
// TestMCPHandler_RecallMemory_GlobalScope_ReturnsDescriptiveError verifies C3
|
||||
// (GLOBAL scope blocked on MCP bridge) is enforced and the error message
|
||||
// propagates to the caller. Unlike "unknown tool:" errors (OFFSEC-001), which
|
||||
// are scrubbed to a constant "tool call failed" to avoid leaking tool names,
|
||||
// permission/usage errors like "GLOBAL scope is not permitted" are returned
|
||||
// verbatim so callers (including tests) can assert on permission messages.
|
||||
func TestMCPHandler_RecallMemory_GlobalScope_ReturnsDescriptiveError(t *testing.T) {
|
||||
// TestMCPHandler_RecallMemory_GlobalScope_Blocked_ScrubsInternalError verifies
|
||||
// C3 (GLOBAL scope blocked on MCP bridge) is enforced and that the OFFSEC-001
|
||||
// scrub contract applies: the client-visible error.message is the constant
|
||||
// "tool call failed", NOT the descriptive internal reason. The internal reason
|
||||
// ("GLOBAL scope is not permitted via the MCP bridge") is logged server-side
|
||||
// but must never reach the wire.
|
||||
func TestMCPHandler_RecallMemory_GlobalScope_Blocked_ScrubsInternalError(t *testing.T) {
|
||||
h, mock := newMCPHandler(t)
|
||||
// No DB expectations — handler must abort before touching the DB.
|
||||
|
||||
@ -632,20 +632,38 @@ func TestMCPHandler_RecallMemory_GlobalScope_ReturnsDescriptiveError(t *testing.
|
||||
})
|
||||
|
||||
var resp mcpResponse
|
||||
json.Unmarshal(w.Body.Bytes(), &resp)
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("response is not valid JSON: %v", err)
|
||||
}
|
||||
// (1) C3: an error must be reported.
|
||||
if resp.Error == nil {
|
||||
t.Fatal("expected JSON-RPC error for GLOBAL scope recall, got nil")
|
||||
}
|
||||
// Error code is -32000 (server error).
|
||||
// (2) OFFSEC-001 positive assertions — exact equality on the scrubbed
|
||||
// constants so any change (re-leak of err.Error(), code mutation) trips
|
||||
// the test.
|
||||
if resp.Error.Code != -32000 {
|
||||
t.Errorf("expected error code -32000, got %d", resp.Error.Code)
|
||||
t.Errorf("error code should be -32000 (Server error / dispatch-failure), got: %d", resp.Error.Code)
|
||||
}
|
||||
// Descriptive error message propagates to the caller. The OFFSEC-001 scrub
|
||||
// applies only to "unknown tool:" errors (to avoid leaking tool names).
|
||||
want := "GLOBAL scope is not permitted via the MCP bridge — use LOCAL, TEAM, or empty"
|
||||
if resp.Error.Message != want {
|
||||
t.Errorf("error message: got %q, want %q", resp.Error.Message, want)
|
||||
if resp.Error.Message != "tool call failed" {
|
||||
t.Errorf("error message should be the OFFSEC-001 constant %q, got: %q", "tool call failed", resp.Error.Message)
|
||||
}
|
||||
// (3) OFFSEC-001 negative assertions — the internal reason must NOT appear
|
||||
// in the client-visible message.
|
||||
leakedTokens := []string{
|
||||
"GLOBAL", // scope name
|
||||
"scope", // policy lexicon
|
||||
"permitted", // policy verb
|
||||
"bridge", // internal architecture term
|
||||
"LOCAL", // alternative scope name
|
||||
"TEAM", // alternative scope name
|
||||
}
|
||||
for _, tok := range leakedTokens {
|
||||
if bytes.Contains([]byte(resp.Error.Message), []byte(tok)) {
|
||||
t.Errorf("OFFSEC-001 scrub regression: client-visible error.message leaks internal token %q (got: %q)", tok, resp.Error.Message)
|
||||
}
|
||||
}
|
||||
// (4) C3 invariant preserved: handler must short-circuit before any DB call.
|
||||
if err := mock.ExpectationsWereMet(); err != nil {
|
||||
t.Errorf("unexpected DB calls on GLOBAL scope block: %v", err)
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user