test(scope): differential regression suite proves agent-scope-hardening holds (Spec 105 PR H1) - #1332
Merged
Merged
Conversation
…ng holds (Spec 105 PR H1) Spec 105 PR H1 — `scope-regression-suite` part 2 (FR-011/013/014, SC-007, gaps FR01x-G4..G7). Closes tasks T107-T116. This is the FINAL phase of the nine-PR agent-scope-hardening epic (A, B, D, E, H0, C, F, G already merged). ## Changes - **Two-fixture differential oracle** (`internal/server/scope_differential_test.go`, new): `newScopeFixture`/`runScopeScenario`/`normalizeScopeResponse` per contracts/differential-oracle.md, plus `TestScopeCoverage_EveryUserStoryScenario` registering every US1.1-1.8/US2.1-2.6/US3.1-3.4 scenario by id — some directly via the general three-server harness, most by re-running each prior PR's own dedicated differential fixture (plan.md: "H1 re-registers them by User Story id"). - **Pinned-reversal grep guard** (`internal/server/scope_pinned_reversal_guard_test.go`, new): `TestScopePinnedReversalsStayInverted` proves none of the four pre-105 insecure assertions gap-map.md's FR01x-G7 entry names have crept back to their original form, traced to their exact pre/post-105 diffs. - **Retained-effect fixtures** (`internal/server/scope_retained_effects_test.go`, new): real reproductions of 5 of 7 SC-001-excluded effects (shared-limiter contention, cross-server scan admission via the actual shadowing.cross_server impersonation-clone mechanism, prompt-name collision, global prompt cap, direct display-name collision), 1 via a companion package (`internal/upstream/manager_prompts_deadline_test.go`, new — shared vs independent prompt-refresh deadline, two discriminating regimes), 1 cited cross-package (log rotation). - **HTTP credential matrix** (`internal/server/scope_http_matrix_test.go`, new): real minted tokens over real loopback HTTP through mcpAuthMiddleware, covering every FR-014 applicability-matrix row at least once plus a content-level (raw wire-text) non-disclosure proof for retrieve_tools. - **Latency harness finalized** (`internal/server/scope_latency_test.go`): all four FR-011 operations (retrieve_tools, read_cache, prompts/list, tools/list) with real workloads, plus `.github/workflows/scope-latency.yml` (new) and `cmd/scope-latency-compare` (new, stdlib-only) for the merge-base p95 regression gate. - **Bug fix found via this suite**: `internal/logs.ReadUpstreamServerLogTail`/ `ReadUpstreamServerLogTailAttributed` dereferenced a nil `*config.LogConfig` — fixed with a TDD red/green pair (`internal/logs/nil_log_config_test.go`). - Docs and roadmap sync: `docs/features/agent-tokens.md` invariant wording finalized; `roadmap.yaml`'s `scope-refusal-shapes` task (stale `todo` despite PR G #1328 already merged) flipped to `done`. `scope-regression-suite` and the epic-level status are deliberately left for the maintainer to flip on merge. ## Testing - `go build`/`go vet` (bare + `-tags server`): clean - Full `-race` suite across internal/server, internal/upstream, internal/cache, internal/logs, cmd/scope-latency-compare: green - `golangci-lint --new-from-rev=origin/main` (bare + `--build-tags server`): 0 new issues - `./scripts/test-api-e2e.sh`: 70/70 passed - Cross-model review: opencode terra/sol/astra confirmed quota-exhausted; fell back to `codex exec --model gpt-5.6-sol`. 4 rounds, every finding verified genuine before fixing (non-vacuous confirmed by temporarily reverting the underlying guard and observing red). `VERDICT: clean` (round 4) — full history in tasks.md T116. Related #1223 #1224 #1225 #1226 #1227 #1279 #1282 #1283 #1284 #1285 #1325 #1326 #1328
Deploying mcpproxy-docs with
|
| Latest commit: |
446b25c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1d52b627.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://105-h1-regression-suite.mcpproxy-docs.pages.dev |
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 35534699095 --repo smart-mcp-proxy/mcpproxy-go
|
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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
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.
Summary
Spec 105 PR H1 —
scope-regression-suitepart 2 (FR-011/013/014, SC-007, gaps FR01x-G4..G7). Closes tasks T107-T116. This is the FINAL phase of the nine-PR agent-scope-hardening epic (spec 105), started with PR A (#1279) and completed through B, D, E, H0, C (#1325), F (#1326) and G (#1328) — all already merged tomain.Its job is to prove, with a genuine two-fixture differential oracle, that every acceptance scenario in
specs/105-agent-scope-hardening/spec.md(User Stories 1-3) actually holds against the already-shipped code, and that none of the pre-105 insecure test assertions the prior PRs inverted has crept back to its original form. This PR is test-only except for one small, genuine bug fix it found along the way (see below) — it does not change any production authorization logic.Changes
internal/server/scope_differential_test.go, new):newScopeFixture/runScopeScenario/normalizeScopeResponsepercontracts/differential-oracle.md, plusTestScopeCoverage_EveryUserStoryScenarioregistering every US1.1-1.8/US2.1-2.6/US3.1-3.4 scenario by id — some directly via the general three-server harness, most by re-running each prior PR's own dedicated differential fixture (plan.md: "H1 re-registers them by User Story id").internal/server/scope_pinned_reversal_guard_test.go, new):TestScopePinnedReversalsStayInvertedproves none of the four pre-105 insecure assertions gap-map.md's FR01x-G7 entry names have crept back to their original form, traced to their exact pre/post-105 diffs across commitsbac8f6b0d,46de8038c,e33d6a139.internal/server/scope_retained_effects_test.go, new): real reproductions of the SC-001-excluded, deliberately-unchanged effects — shared-limiter contention, cross-server scan admission (via the actualshadowing.cross_serverimpersonation-clone mechanism), prompt-name collision, global prompt cap, direct display-name collision, plus shared prompt-refresh deadline via a companion package (internal/upstream/manager_prompts_deadline_test.go, new, two discriminating regimes), and shared log rotation (cited cross-package).internal/server/scope_http_matrix_test.go, new): real minted tokens over real loopback HTTP throughmcpAuthMiddleware, covering every FR-014 applicability-matrix row at least once plus a content-level (raw wire-text) non-disclosure proof forretrieve_tools.internal/server/scope_latency_test.go): all four FR-011 operations (retrieve_tools,read_cache,prompts/list,tools/list) with real workloads, plus.github/workflows/scope-latency.yml(new) andcmd/scope-latency-compare(new, stdlib-only, unit-tested) for the merge-base p95 regression gate — verified end-to-end locally against this PR's actual merge-base commit.internal/logs.ReadUpstreamServerLogTail/ReadUpstreamServerLogTailAttributeddereferenced a nil*config.LogConfig(affecting admin and scoped readers identically — a robustness gap, not a scope/security issue). Fixed with a TDD red/green pair (internal/logs/nil_log_config_test.go).docs/features/agent-tokens.mdinvariant wording finalized (no overclaiming — most FR coverage is via re-run dedicated fixtures, not literally every FR through the general 3-server harness).roadmap.yaml'sscope-refusal-shapestask was found stillstatus: tododespite PR G (fix(scope): scope-first refusal precedence, hidden equals nonexistent on every surface (Spec 105 PR G, FR-010) #1328) having already merged — flipped todonewith its PR number.scope-regression-suite's own task status and the epic-levelagent-scope-hardeningstatus are deliberately left astodo/in_progressfor the maintainer to flip on merge, per this repo's "no self-merge" convention for these security PRs.Test plan
go build ./...,go vet ./...(bare +-tags server): clean-racesuite acrossinternal/server,internal/upstream,internal/cache,internal/logs,cmd/scope-latency-compare: greengolangci-lint run --config .github/.golangci.yml(bare +--build-tags server,--new-from-rev=origin/main): 0 new issuesgofmt -lclean on every touched/new file./scripts/test-api-e2e.sh: 70/70 passedcodex exec --model gpt-5.6-sol --sandbox read-onlyper CLAUDE.md's fallback rule. 4 rounds, every finding verified genuine before fixing (never accepted a finding without first confirming it against the actual code, and every fix was verified non-vacuous by temporarily reverting the underlying guard/logic and confirming the new test goes red, then restoring it). Round 1 found 7 real issues across 8 review areas (including two genuinely vacuous retained-effect fixtures that never exercised the mechanism they claimed to); round 2 found 2 more (including the nil-pointer panic above) plus a fix that itself needed strengthening; round 3 found the deadline fixture still didn't rule out an independent-concurrent alternative design; round 4:VERDICT: clean— "Both prior MUST-FIX findings are resolved: The complementary 150ms/100ms fixture distinguishes shared-sequential behavior from both independent-concurrent and independent-sequential deadlines. The real-HTTP retrieve_tools test provides content-level non-disclosure through authentication middleware, with an authorized positive control. Combined with the differential suite and existing direct/profile HTTP checks, further per-row content assertions have insufficient marginal value." Full round-by-round history inspecs/105-agent-scope-hardening/tasks.mdT116.Related #1223 #1224 #1225 #1226 #1227 #1279 #1282 #1283 #1284 #1285 #1325 #1326 #1328