fix(ci): stop the Windows recovered-panic heap crash and the Pass-2 temp-dir race - #1288
Merged
Merged
Conversation
A controller that reports success without a server config (the test doubles do) was dereferenced in handleAddFromRegistry; chi's recoverer turned the fault into a bare 500. Under Go 1.26 on windows/amd64 the recovered hardware exception corrupts the Go heap (golang/go#81238), so the internal/httpapi test binary died later with "found pointer to free object" / "unexpected fault address" on unrelated PRs (#1282, #1285). Return a JSON 500 instead, the way redactedRegistrySummary already tolerates a nil entry, and pin it with a test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deploying mcpproxy-docs with
|
| Latest commit: |
841bcb4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://dc6ebae2.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-httpapi-registry-add-nil.mcpproxy-docs.pages.dev |
TestServiceStartScanDeepOnRunsSourceResolutionAndPass2 failed on ubuntu CI with "TempDir RemoveAll cleanup: directory not empty": the Pass-2 goroutine writes tools.json into the server working dir after ResolveFullSource returns and before it registers its engine job, so an idle engine can mean "Pass 1 cleared, Pass 2 not started yet" and the test's teardown raced the write. Wait for the Pass-2 job to reach a terminal status in storage (its last write on every exit path) first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Dumbris
changed the title
Contributor
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 35112724985 --repo smart-mcp-proxy/mcpproxy-go
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Dumbris
added a commit
that referenced
this pull request
Sep 16, 2026
…1288) This branch diverged from main before #1288 landed, so PR-B's CI hit the exact regression it fixed: handleAddFromRegistry's success path dereferenced a nil cfg, chi's recoverer turned the panic into a bare 500, and the recovered hardware fault corrupted the Go heap on windows/amd64 under Go 1.26 (golang/go#81238) — the internal/httpapi test binary crashed with "found pointer to free object" and took the whole Windows unit-test job (and the binaries job's httpapi/scanner test bundle) down with it. Cherry-picked the same fix main already carries: the nil-config guard with a JSON 500 (and its pinning test), plus the unrelated Pass-2 temp-dir teardown race fix in the scanner test this same upstream commit bundled. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Dumbris
added a commit
that referenced
this pull request
Sep 18, 2026
…ess, telemetry v13 (Spec 107 PR-B) (#1292) * docs(specs): Spec 107 — server edition SSO front door hardened for real IdPs Generic OIDC provider, IdP-group → server allowlist, attributable JSONL audit line, and a freeze/cut of the latent multiuser/credential-injection code, split into four PRs (A freeze/cut, B OIDC + front door + telemetry, C entitlement predicate + tenant session, D audit line). Judge-panel drafted; cross-model reviewed over nine rounds (codex gpt-5.6-sol), ~130 findings resolved. Un-parks roadmap epic sso. * test(spec-107): PR-A red tests, server-tag lint job and release notice Related #1177 * refactor(serveredition): freeze the latent credential-injection chain and normalise removed config keys (Spec 107 PR-A) Related #1177 * fix(storage): enforce the agent-token cap per owner, not per deployment (Spec 107 PR-A) Closes #1177 * fix(spec-107): address adversarial review of PR-A * fix(spec-107): codex round 1 for PR-A * fix(spec-107): codex round 2 for PR-A * fix(spec-107): codex round 3 for PR-A * fix(spec-107): codex round 4 for PR-A * docs(spec-107): PR-A verification and task ticks * docs(roadmap): link PR-A * test(oauthserver): fake OIDC identity provider and server-edition dev rig (Spec 107 PR-B) * test(serveredition): red tests for the generic OIDC provider, subject binding and refusals (Spec 107 PR-B) * feat(serveredition): generic OIDC provider with JWKS-verified ID tokens, groups capture and subject binding (Spec 107 PR-B) Related #1169 * test(serveredition): red tests for trusted proxies, public URL, cookie security and forced MCP auth (Spec 107 PR-B) * feat(serveredition): front door behind an ingress — public URL, trusted proxies, cookie security policy, forced MCP auth, provider probe (Spec 107 PR-B) * feat(config,telemetry): hot-reload clauses, docs and OAS for the PR-B keys; telemetry v13 server-edition fields (Spec 107 PR-B) * fix(spec-107): gate fixes for PR-B * docs(spec-107): PR-B automated-check evidence * fix(spec-107): cross-review round 1 for PR-B * fix(spec-107): cross-review round 2 for PR-B * fix(spec-107): cross-review round 3 for PR-B * fix(spec-107): cross-review round 3 for PR-B * fix(spec-107): cross-review round 4 for PR-B * fix(spec-107): cross-review round 5 for PR-B * fix(spec-107): cross-review round 6 for PR-B * docs(spec-107): PR-B real-instance verification, gate re-run, task ticks T062/T063: real-instance verification of the OIDC front door (happy-path login, groups, /auth/me, agent-token mint + /mcp initialize/tools-list, ErrorMode tamper matrix, cookie Secure matrix behind a simulated trusted vs untrusted proxy) and the telemetry v13 payload (server vs personal). T064: full gate set re-run on HEAD after cross-review round 6 touched the config load path, oauth_handler.go and telemetry.go — all green. T066: tick completed PR-B tasks (T032-T066); ROADMAP.md regenerated. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(roadmap): link PR-B Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): pull forward the Windows heap-corruption + Pass-2 race fix (#1288) This branch diverged from main before #1288 landed, so PR-B's CI hit the exact regression it fixed: handleAddFromRegistry's success path dereferenced a nil cfg, chi's recoverer turned the panic into a bare 500, and the recovered hardware fault corrupted the Go heap on windows/amd64 under Go 1.26 (golang/go#81238) — the internal/httpapi test binary crashed with "found pointer to free object" and took the whole Windows unit-test job (and the binaries job's httpapi/scanner test bundle) down with it. Cherry-picked the same fix main already carries: the nil-config guard with a JSON 500 (and its pinning test), plus the unrelated Pass-2 temp-dir teardown race fix in the scanner test this same upstream commit bundled. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(spec-107): record the CI red/fix cycle for PR-B Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(spec-107): record PR-B CI green Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(spec-107): cross-review round 7 for PR-B * fix(spec-107): cross-review round 8 for PR-B * fix(web-ui): update hard-reload auth-guard test for the FR-030 provider probe The merge of origin/main (#1294, shared in-flight auth probe) into 107-b-oidc-front-door (#1292, generic OIDC provider) combined without a conflict, but PR-B's auth store change (probe() now calls authApi.getProvider() instead of api.getStatus() for edition detection, Spec 107 FR-030/FR-041) left the test file's mocks pointed at the old api.getStatus() call, so every test silently no-oped through probe()'s catch block. Repoint the spy at authApi.getProvider() and update the resolved payload shape ({ display_name } instead of { data: { edition } }); the in-flight-probe-sharing behaviour under test is unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(test): match TestLoadConfig_ListenFlag to loadConfig's 3-value signature Mirrors main's ce5c2f7 fix for the same loadConfig signature mismatch that broke the Build Binaries CI job on this PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Dumbris
added a commit
that referenced
this pull request
Sep 18, 2026
…nt session on core REST and Web UI (Spec 107 PR-C) (#1293) * docs(specs): Spec 107 — server edition SSO front door hardened for real IdPs Generic OIDC provider, IdP-group → server allowlist, attributable JSONL audit line, and a freeze/cut of the latent multiuser/credential-injection code, split into four PRs (A freeze/cut, B OIDC + front door + telemetry, C entitlement predicate + tenant session, D audit line). Judge-panel drafted; cross-model reviewed over nine rounds (codex gpt-5.6-sol), ~130 findings resolved. Un-parks roadmap epic sso. * test(spec-107): PR-A red tests, server-tag lint job and release notice Related #1177 * refactor(serveredition): freeze the latent credential-injection chain and normalise removed config keys (Spec 107 PR-A) Related #1177 * fix(storage): enforce the agent-token cap per owner, not per deployment (Spec 107 PR-A) Closes #1177 * fix(spec-107): address adversarial review of PR-A * fix(spec-107): codex round 1 for PR-A * fix(spec-107): codex round 2 for PR-A * fix(spec-107): codex round 3 for PR-A * fix(spec-107): codex round 4 for PR-A * docs(spec-107): PR-A verification and task ticks * docs(roadmap): link PR-A * test(oauthserver): fake OIDC identity provider and server-edition dev rig (Spec 107 PR-B) * test(serveredition): red tests for the generic OIDC provider, subject binding and refusals (Spec 107 PR-B) * feat(serveredition): generic OIDC provider with JWKS-verified ID tokens, groups capture and subject binding (Spec 107 PR-B) Related #1169 * test(serveredition): red tests for trusted proxies, public URL, cookie security and forced MCP auth (Spec 107 PR-B) * feat(serveredition): front door behind an ingress — public URL, trusted proxies, cookie security policy, forced MCP auth, provider probe (Spec 107 PR-B) * feat(config,telemetry): hot-reload clauses, docs and OAS for the PR-B keys; telemetry v13 server-edition fields (Spec 107 PR-B) * fix(spec-107): gate fixes for PR-B * docs(spec-107): PR-B automated-check evidence * fix(spec-107): cross-review round 1 for PR-B * fix(spec-107): cross-review round 2 for PR-B * fix(spec-107): cross-review round 3 for PR-B * fix(spec-107): cross-review round 3 for PR-B * fix(spec-107): cross-review round 4 for PR-B * fix(spec-107): cross-review round 5 for PR-B * fix(spec-107): cross-review round 6 for PR-B * docs(spec-107): PR-B real-instance verification, gate re-run, task ticks T062/T063: real-instance verification of the OIDC front door (happy-path login, groups, /auth/me, agent-token mint + /mcp initialize/tools-list, ErrorMode tamper matrix, cookie Secure matrix behind a simulated trusted vs untrusted proxy) and the telemetry v13 payload (server vs personal). T064: full gate set re-run on HEAD after cross-review round 6 touched the config load path, oauth_handler.go and telemetry.go — all green. T066: tick completed PR-B tasks (T032-T066); ROADMAP.md regenerated. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(roadmap): link PR-B Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): pull forward the Windows heap-corruption + Pass-2 race fix (#1288) This branch diverged from main before #1288 landed, so PR-B's CI hit the exact regression it fixed: handleAddFromRegistry's success path dereferenced a nil cfg, chi's recoverer turned the panic into a bare 500, and the recovered hardware fault corrupted the Go heap on windows/amd64 under Go 1.26 (golang/go#81238) — the internal/httpapi test binary crashed with "found pointer to free object" and took the whole Windows unit-test job (and the binaries job's httpapi/scanner test bundle) down with it. Cherry-picked the same fix main already carries: the nil-config guard with a JSON 500 (and its pinning test), plus the unrelated Pass-2 temp-dir teardown race fix in the scanner test this same upstream commit bundled. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(spec-107): record the CI red/fix cycle for PR-B Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(spec-107): record PR-B CI green Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(spec-107): cross-review round 7 for PR-B * fix(spec-107): cross-review round 8 for PR-B * test(serveredition): two-fixture non-disclosure harness and single-predicate guard (Spec 107 PR-C) * test(scope): red tests for group grants, owner resolution, scoped index search and minting doors (Spec 107 PR-C) * feat(scope): one entitlement predicate with IdP-group grants, owner resolution on every authentication, scoped index search (Spec 107 PR-C) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * test(httpapi,frontend): red tests for the tenant session principal, route allowlist and Web UI gating (Spec 107 PR-C) * feat(httpapi,web): tenant session principal on core REST, profiles/SSE/activity projections, Web UI on the SSO session alone (Spec 107 PR-C) T083: SessionPrincipalResolver type in internal/httpapi/session_principal.go; rewrote apiKeyAuthMiddleware with FR-001 precedence (X-API-Key > Authorization > ?apikey > session cookie). Auth context now excludes session principals from CanRevealSecrets. T084: Session principal resolver built in setup.go via userHandlers.ResolveAgentTokenOwner, installed via Dependencies and wired in serveredition_wire.go. T085: GET /profiles omits profiles whose entitlements don't intersect session scope; GET /profiles/active returns empty when active profile is hidden. SSE and activity paths re-resolve session principals before every frame for live-narrowing. T086: Activity filtering and masking for tenant sessions; GET /user/activity added; admin_user principals answer unconditionally with empty list (SC-006). T088–T089: Frontend principal-kind gating for tenant sessions (onboarded to /login, hidden config/routing/servers/stats/security/activity surfaces). AdminUsers adds Groups column; AdminServers shows server-scoped group/grant chips. Playwright e2e test added (quickstart.md spec). Both editions build clean; all unit-tests.yml -tags server race suites pass. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * docs(spec-107): access block reference, agent-token invariant, hot-reload and OAS wiring for PR-C * fix(spec-107): gate fixes for PR-C Full plan.md gate set run against PR-C HEAD (1e379ff): all gates pass, no PR-C code required fixing. golangci-lint's pinned v2 binary refuses the go1.26 module (pre-existing tooling gap); the @latest fallback found 16 issues, all pre-existing and outside the PR-C diff. The isolated test-api-e2e.sh run's two failures (launcher-lifecycle reconnect/log capture) are spec-046 fixture flakes unrelated to any PR-C file, reproduced identically on two runs. Records the full pass/fail table under '## PR-C / ### Automated checks' in verification.md. * fix(spec-107): cross-review round 1 for PR-C Four genuine defects found and fixed across three opencode gpt-5.6-sol/terra review chunks (a fourth chunk was clean plus two low-value findings rejected as pinned-by-design or non-security): - tenantSessionAllowlist matched on the DECODED r.URL.Path, but chi routes on RawPath — a server name containing a literal "/" (nothing forbids one; only ":" is refused) reaches the real handler via its percent-encoded id while the allowlist Cuts the decoded path on the wrong boundary, bypassing the named must-refuse for /servers/{id}/tool-calls. Fixed by matching on RawPath when set. - visibleSharedServers/visibleSharedServer independently re-read the live admin-config snapshot after the entitlement predicate had already read its own, opening a hot-reload race where a server could be entitled against one snapshot and disclosed from another. Fixed with tenantEntitledSnapshot, which fetches the snapshot once and threads it through both the predicate and the disclosure lookup. - GET /user/activity read AllowedServers off the AuthContext, but the ServerEditionAuthMiddleware this door is actually mounted behind never populates that field (only the separate SessionPrincipalResolver path does) — so every tenant on this door saw every user's activity for every server, entitled or not. Fixed by resolving the entitlement set live through the one predicate instead. - Dashboard.vue's loadActivitySummary was the one dashboard loader missing the tenant guard its four siblings already carry, drawing a spurious 403 against the now-allowlist-refused /activity/summary on every tenant page load and 30s refresh. Also corrected RELEASE_NOTICE.md's inverted claim about the access block's default (absent = Shared-only, NOT deny-all; only a present- but-empty block is deny-all) and widened the Playwright refused-route smoke list with several named must-refuse examples. Two findings verified and rejected: the nil-vs-empty access-block collapse in ServerEditionAccessProjection is deliberate, pinned by an existing config_hotreload_serveredition_test.go case; rejecting an empty group_servers key is defensive config validation, not a spec violation. * docs(spec-107): record PR-C cross-review round 1 Appends the round 1 entry (4 genuine findings fixed, 2 rejected with reasons, full verification command list) under PR-C. * fix(spec-107): cross-review round 2 for PR-C Cross-model review (opencode gpt-5.6-sol/terra) round 2 of PR-C: - entitledServerNamesFor read the admin-config servers once (round 1's fix) but the access block through a separate, independent live call, so a hot reload landing between the two reads could splice a servers snapshot from one configuration version to an access snapshot from another. A new EntitlementSnapshotProvider derives both values from one liveConfig() read, wired in setup.go. - The /events heartbeat branch never re-resolved the session principal, unlike its status/runtime-event siblings, so a disabled tenant's otherwise-idle stream never closed (FR-005). - The tenant profile projection only omitted a profile when scoping narrowed a non-empty effective set to empty, missing the case where the effective set was already empty. - Three tenant-dashboard call sites (Usage.vue's activity/usage poll, refreshSecurityScannerStatus, the ModeSwitcher control) and one Activity.vue fallback path issued or exposed admin-only core requests to a tenant session, contradicting FR-041; a doc paragraph misattributed an administrator-only capability to tenants. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(spec-107): record PR-C cross-review round 2 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(spec-107): PR-C real-instance verification, gate re-run, task ticks T079/T090: real-instance verification against the dev-server-edition rig (quickstart §5-§7) - entitlement-filtered REST, non-disclosing by-name refusals matching a nonexistent server, "*" token materialisation to the entitled set, hot reload narrowing a live session/token with no restart or rotation, JWT self-renewal refused, live administrator parity. Web UI/Playwright run against the rig with a properly embedded frontend (make build-server, since the rig script's bare go build does not embed it): 4/5 pass; the 5th hit a pre-existing router deep-link/reload bug (title updates, body does not) unrelated to the PR-C diff, flagged as a follow-up rather than fixed inline. T093: full gate set re-run against HEAD (aac5053, after both cross-review rounds), since review landed code changes after the prior gate run. All 19 non-e2e gates green; isolated test-api-e2e.sh run twice (63/65 both times, same pre-existing launcher-lifecycle pair, unrelated to PR-C). Ticks T067-T095 in tasks.md; ROADMAP.md regenerated. * docs(roadmap): link PR-C * fix(spec-107): cross-review round 3 for PR-C * docs(spec-107): record PR-C cross-review round 3 * fix(spec-107): wait for PhaseReady before seeding the group-scope fixture (PR-C CI) TestMCPGroupScope_TwoFixtureParity_GroupScopedToken failed in CI (not locally) because newGroupScopeFixture wrote directly to storage/upstream manager right after NewServer(), racing StartBackgroundInitialization's async LoadConfiguredServers — which treats cfg.Servers as the source of truth and unconditionally re-saves every entry (including the disabled shared-server placeholders this fixture registers) before flipping the runtime to PhaseReady. When that reconciliation write lands after the fixture's own Enabled:true write, the server reverts to Enabled:false in storage and isExactToolCallable drops its indexed tool from retrieve_tools — exactly the observed empty-vs-populated mismatch. Same race already fixed once in this codebase (see newLogsTestServer/server_logs_missing_file_test.go); this applies the same require.Eventually(PhaseReady) guard here. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Dumbris
added a commit
that referenced
this pull request
Sep 18, 2026
… tool-call funnel (Spec 107 PR-D) (#1296) * docs(specs): Spec 107 — server edition SSO front door hardened for real IdPs Generic OIDC provider, IdP-group → server allowlist, attributable JSONL audit line, and a freeze/cut of the latent multiuser/credential-injection code, split into four PRs (A freeze/cut, B OIDC + front door + telemetry, C entitlement predicate + tenant session, D audit line). Judge-panel drafted; cross-model reviewed over nine rounds (codex gpt-5.6-sol), ~130 findings resolved. Un-parks roadmap epic sso. * test(spec-107): PR-A red tests, server-tag lint job and release notice Related #1177 * refactor(serveredition): freeze the latent credential-injection chain and normalise removed config keys (Spec 107 PR-A) Related #1177 * fix(storage): enforce the agent-token cap per owner, not per deployment (Spec 107 PR-A) Closes #1177 * fix(spec-107): address adversarial review of PR-A * fix(spec-107): codex round 1 for PR-A * fix(spec-107): codex round 2 for PR-A * fix(spec-107): codex round 3 for PR-A * fix(spec-107): codex round 4 for PR-A * docs(spec-107): PR-A verification and task ticks * docs(roadmap): link PR-A * test(oauthserver): fake OIDC identity provider and server-edition dev rig (Spec 107 PR-B) * test(serveredition): red tests for the generic OIDC provider, subject binding and refusals (Spec 107 PR-B) * feat(serveredition): generic OIDC provider with JWKS-verified ID tokens, groups capture and subject binding (Spec 107 PR-B) Related #1169 * test(serveredition): red tests for trusted proxies, public URL, cookie security and forced MCP auth (Spec 107 PR-B) * feat(serveredition): front door behind an ingress — public URL, trusted proxies, cookie security policy, forced MCP auth, provider probe (Spec 107 PR-B) * feat(config,telemetry): hot-reload clauses, docs and OAS for the PR-B keys; telemetry v13 server-edition fields (Spec 107 PR-B) * fix(spec-107): gate fixes for PR-B * docs(spec-107): PR-B automated-check evidence * fix(spec-107): cross-review round 1 for PR-B * fix(spec-107): cross-review round 2 for PR-B * fix(spec-107): cross-review round 3 for PR-B * fix(spec-107): cross-review round 3 for PR-B * fix(spec-107): cross-review round 4 for PR-B * fix(spec-107): cross-review round 5 for PR-B * fix(spec-107): cross-review round 6 for PR-B * docs(spec-107): PR-B real-instance verification, gate re-run, task ticks T062/T063: real-instance verification of the OIDC front door (happy-path login, groups, /auth/me, agent-token mint + /mcp initialize/tools-list, ErrorMode tamper matrix, cookie Secure matrix behind a simulated trusted vs untrusted proxy) and the telemetry v13 payload (server vs personal). T064: full gate set re-run on HEAD after cross-review round 6 touched the config load path, oauth_handler.go and telemetry.go — all green. T066: tick completed PR-B tasks (T032-T066); ROADMAP.md regenerated. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(roadmap): link PR-B Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): pull forward the Windows heap-corruption + Pass-2 race fix (#1288) This branch diverged from main before #1288 landed, so PR-B's CI hit the exact regression it fixed: handleAddFromRegistry's success path dereferenced a nil cfg, chi's recoverer turned the panic into a bare 500, and the recovered hardware fault corrupted the Go heap on windows/amd64 under Go 1.26 (golang/go#81238) — the internal/httpapi test binary crashed with "found pointer to free object" and took the whole Windows unit-test job (and the binaries job's httpapi/scanner test bundle) down with it. Cherry-picked the same fix main already carries: the nil-config guard with a JSON 500 (and its pinning test), plus the unrelated Pass-2 temp-dir teardown race fix in the scanner test this same upstream commit bundled. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(spec-107): record the CI red/fix cycle for PR-B Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(spec-107): record PR-B CI green Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(spec-107): cross-review round 7 for PR-B * fix(spec-107): cross-review round 8 for PR-B * test(serveredition): two-fixture non-disclosure harness and single-predicate guard (Spec 107 PR-C) * test(scope): red tests for group grants, owner resolution, scoped index search and minting doors (Spec 107 PR-C) * feat(scope): one entitlement predicate with IdP-group grants, owner resolution on every authentication, scoped index search (Spec 107 PR-C) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * test(httpapi,frontend): red tests for the tenant session principal, route allowlist and Web UI gating (Spec 107 PR-C) * feat(httpapi,web): tenant session principal on core REST, profiles/SSE/activity projections, Web UI on the SSO session alone (Spec 107 PR-C) T083: SessionPrincipalResolver type in internal/httpapi/session_principal.go; rewrote apiKeyAuthMiddleware with FR-001 precedence (X-API-Key > Authorization > ?apikey > session cookie). Auth context now excludes session principals from CanRevealSecrets. T084: Session principal resolver built in setup.go via userHandlers.ResolveAgentTokenOwner, installed via Dependencies and wired in serveredition_wire.go. T085: GET /profiles omits profiles whose entitlements don't intersect session scope; GET /profiles/active returns empty when active profile is hidden. SSE and activity paths re-resolve session principals before every frame for live-narrowing. T086: Activity filtering and masking for tenant sessions; GET /user/activity added; admin_user principals answer unconditionally with empty list (SC-006). T088–T089: Frontend principal-kind gating for tenant sessions (onboarded to /login, hidden config/routing/servers/stats/security/activity surfaces). AdminUsers adds Groups column; AdminServers shows server-scoped group/grant chips. Playwright e2e test added (quickstart.md spec). Both editions build clean; all unit-tests.yml -tags server race suites pass. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * docs(spec-107): access block reference, agent-token invariant, hot-reload and OAS wiring for PR-C * fix(spec-107): gate fixes for PR-C Full plan.md gate set run against PR-C HEAD (1e379ff): all gates pass, no PR-C code required fixing. golangci-lint's pinned v2 binary refuses the go1.26 module (pre-existing tooling gap); the @latest fallback found 16 issues, all pre-existing and outside the PR-C diff. The isolated test-api-e2e.sh run's two failures (launcher-lifecycle reconnect/log capture) are spec-046 fixture flakes unrelated to any PR-C file, reproduced identically on two runs. Records the full pass/fail table under '## PR-C / ### Automated checks' in verification.md. * fix(spec-107): cross-review round 1 for PR-C Four genuine defects found and fixed across three opencode gpt-5.6-sol/terra review chunks (a fourth chunk was clean plus two low-value findings rejected as pinned-by-design or non-security): - tenantSessionAllowlist matched on the DECODED r.URL.Path, but chi routes on RawPath — a server name containing a literal "/" (nothing forbids one; only ":" is refused) reaches the real handler via its percent-encoded id while the allowlist Cuts the decoded path on the wrong boundary, bypassing the named must-refuse for /servers/{id}/tool-calls. Fixed by matching on RawPath when set. - visibleSharedServers/visibleSharedServer independently re-read the live admin-config snapshot after the entitlement predicate had already read its own, opening a hot-reload race where a server could be entitled against one snapshot and disclosed from another. Fixed with tenantEntitledSnapshot, which fetches the snapshot once and threads it through both the predicate and the disclosure lookup. - GET /user/activity read AllowedServers off the AuthContext, but the ServerEditionAuthMiddleware this door is actually mounted behind never populates that field (only the separate SessionPrincipalResolver path does) — so every tenant on this door saw every user's activity for every server, entitled or not. Fixed by resolving the entitlement set live through the one predicate instead. - Dashboard.vue's loadActivitySummary was the one dashboard loader missing the tenant guard its four siblings already carry, drawing a spurious 403 against the now-allowlist-refused /activity/summary on every tenant page load and 30s refresh. Also corrected RELEASE_NOTICE.md's inverted claim about the access block's default (absent = Shared-only, NOT deny-all; only a present- but-empty block is deny-all) and widened the Playwright refused-route smoke list with several named must-refuse examples. Two findings verified and rejected: the nil-vs-empty access-block collapse in ServerEditionAccessProjection is deliberate, pinned by an existing config_hotreload_serveredition_test.go case; rejecting an empty group_servers key is defensive config validation, not a spec violation. * docs(spec-107): record PR-C cross-review round 1 Appends the round 1 entry (4 genuine findings fixed, 2 rejected with reasons, full verification command list) under PR-C. * fix(spec-107): cross-review round 2 for PR-C Cross-model review (opencode gpt-5.6-sol/terra) round 2 of PR-C: - entitledServerNamesFor read the admin-config servers once (round 1's fix) but the access block through a separate, independent live call, so a hot reload landing between the two reads could splice a servers snapshot from one configuration version to an access snapshot from another. A new EntitlementSnapshotProvider derives both values from one liveConfig() read, wired in setup.go. - The /events heartbeat branch never re-resolved the session principal, unlike its status/runtime-event siblings, so a disabled tenant's otherwise-idle stream never closed (FR-005). - The tenant profile projection only omitted a profile when scoping narrowed a non-empty effective set to empty, missing the case where the effective set was already empty. - Three tenant-dashboard call sites (Usage.vue's activity/usage poll, refreshSecurityScannerStatus, the ModeSwitcher control) and one Activity.vue fallback path issued or exposed admin-only core requests to a tenant session, contradicting FR-041; a doc paragraph misattributed an administrator-only capability to tenants. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(spec-107): record PR-C cross-review round 2 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(spec-107): PR-C real-instance verification, gate re-run, task ticks T079/T090: real-instance verification against the dev-server-edition rig (quickstart §5-§7) - entitlement-filtered REST, non-disclosing by-name refusals matching a nonexistent server, "*" token materialisation to the entitled set, hot reload narrowing a live session/token with no restart or rotation, JWT self-renewal refused, live administrator parity. Web UI/Playwright run against the rig with a properly embedded frontend (make build-server, since the rig script's bare go build does not embed it): 4/5 pass; the 5th hit a pre-existing router deep-link/reload bug (title updates, body does not) unrelated to the PR-C diff, flagged as a follow-up rather than fixed inline. T093: full gate set re-run against HEAD (aac5053, after both cross-review rounds), since review landed code changes after the prior gate run. All 19 non-e2e gates green; isolated test-api-e2e.sh run twice (63/65 both times, same pre-existing launcher-lifecycle pair, unrelated to PR-C). Ticks T067-T095 in tasks.md; ROADMAP.md regenerated. * docs(roadmap): link PR-C * fix(spec-107): cross-review round 3 for PR-C * docs(spec-107): record PR-C cross-review round 3 * fix(spec-107): wait for PhaseReady before seeding the group-scope fixture (PR-C CI) TestMCPGroupScope_TwoFixtureParity_GroupScopedToken failed in CI (not locally) because newGroupScopeFixture wrote directly to storage/upstream manager right after NewServer(), racing StartBackgroundInitialization's async LoadConfiguredServers — which treats cfg.Servers as the source of truth and unconditionally re-saves every entry (including the disabled shared-server placeholders this fixture registers) before flipping the runtime to PhaseReady. When that reconciliation write lands after the fixture's own Enabled:true write, the server reverts to Enabled:false in storage and isExactToolCallable drops its indexed tool from retrieve_tools — exactly the observed empty-vs-populated mismatch. Same race already fixed once in this codebase (see newLogsTestServer/server_logs_missing_file_test.go); this applies the same require.Eventually(PhaseReady) guard here. * test(audit): red tests for JCS canonicalisation, line schema, redaction and the synchronous sink (Spec 107 PR-D) * feat(audit): edition-neutral JSONL audit line — JCS args hash, schema-validated builder, synchronous sink (Spec 107 PR-D) * test(server,jsruntime): red tests for the audit funnels, caller derivation, nested observer and error classes (Spec 107 PR-D) * feat(server): audit lines at the authorization and tool-call funnels, nested code_execution observer, error classes (Spec 107 PR-D) T096-T119: Edition-neutral audit line emission at authorization and tool-call decision points. - internal/audit/error_class.go: ErrorClass enum (upstream_error, upstream_unavailable, validation, sanitisation, cancelled, internal) with errors.Is/As routing and ErrorClassOf() type switch - internal/jsruntime/runtime.go: AuthzObserver interface (report cached decision), AuthzGateReport with ParentID, ExecutionOptions.{AuthzObserver, ParentID} for nested tracking, nestedAuthzObserver implementation - internal/server/audit_funnel.go: auditDispatch (attempt wrapper), installAuditAttempt (RFC 8785 JCS arg hash, mount/source/origin/profile derivation), auditCallerFromContext (full table: stdio, socket, agent_token, session_admin, anonymous), auditAuthz (deny reason from telemetry.BlockReason, disclosed:false for scopes), auditToolCall (pairs authz, error_class routing), auditToolCallShed (rejected outcome) - internal/server/mcp.go, mcp_routing.go, mcp_code_execution.go: ctx as first param to emit* activity functions; handleCallToolVariant surface/intent gates; code_execution bridge with parent_id + nested observer - internal/server/server.go: ServerOption pattern, WithAuditSink, stdioAuthContext tags - internal/server/serveredition_wire.go: AuditSink dependency wiring - internal/server/audit_funnel_test.go: 11 tests covering allow/deny, hidden-server scope, intent rejection, nested calls, limiter shed, stdio/socket callers, schema validation - activity_result_status_test.go, code_exec_activity_test.go, preflight_telemetry_test.go: statusArgIndex 5→6, ctx passed to emit functions All audit lines validated against contracts/audit-line.schema.json; no secrets/tokens in clear; one authz per attempt, one tool_call per call. * feat(audit): auth_event lines, audit_log config with per-edition defaults, doctor finding, write-failure metric, settings wiring (Spec 107 PR-D) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * docs(audit): audit-log reference, team deployment guide, e2e audit assertion, release notice (Spec 107 PR-D) * fix(spec-107): gate fixes for PR-D * fix(spec-107): address adversarial review of PR-D * docs(spec-107): PR-D real-instance verification, gate re-run, task ticks T116: ran scripts/dev-server-edition.sh --phase d end to end, plus manual extensions for cases the script doesn't cover (sentinel in a caller-supplied tool name, native stdio transport rules, unwritable path). Fixed two rig bugs found along the way (not in the audited feature code): the scratch config left fixture servers quarantined, and the isError=null vs "false" assertion didn't accept the omitempty case. Documented a pre-existing, out-of-scope discrepancy: `--listen ""` doesn't actually reach native stdio through `mcpproxy serve` (Validate() resets it to the HTTP default); `--listen ":0"` does. T117: re-ran the full gate set since the adversarial-review commit changed code after the prior recording. All green; two transient flakes under heavy concurrent load (a Docker-status test, a launcher-lifecycle respawn test) confirmed non-reproducing / out of this PR's scope. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(roadmap): link PR-D, epic in_review Spec 107's four PRs (A, B, C, D) are all open; PR-D is #1296. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(spec-107): cross-review round 1 for PR-D opencode CLI (gpt-5.6-sol / gpt-5.6-terra) reviewed the PR-D diff in 5 chunks; all 5 returned findings (16 total). 8 verified genuine and fixed: NaN/Infinity silently hashed as JSON null instead of refused (canonical.go), client.version unmasked in the audit line, a short sink write not counted as a failure, a malformed args_json dispatch producing no audit line at all, work_session_id never stamped on the attempt, auth_event lines never carrying client.ip, two missing FR-014 startup notices, and a missing sidebars.js entry for the new audit-log doc. 3 rejected as false positives with evidence (redact.go patterns are a superset of the log sanitizer's, not narrower; no request_id==transport_request_id binding rule exists in the contracts doc; the unknown-server fail-open in mcp_code_execution.go is a documented deliberate design choice). 5 more confirmed genuine but deferred to a later round given their blast radius (JCS float formatting, per-kind caller-identity validation, auth_event flags vocabulary, Attempt.Operation sourced from the caller's variant instead of the resolved target tier, and a batch-cancellation gap in the tool_call/authz pairing) — all documented in verification.md. Full round detail, rejection evidence and verification commands in specs/107-server-edition-sso-hardening/verification.md under PR-D / Cross-review / Round 1. * fix(spec-107): cross-review round 2 for PR-D Fixes 8 findings from round 2 (opencode gpt-5.6-sol/terra, 5 chunks), three of them round-1's own deferred list independently re-confirmed: - ES6 Number::toString fixed/exponential threshold in formatNumberJCS (args_sha256 could diverge from a real JCS reference implementation) - length-cap the per-field audit redaction pass (unbounded client.name could drop a required line at the sink's record-size limit) - Server.ReplayToolCall wrote zero audit lines (bypassed the funnel entirely via runtime.ReplayToolCall's direct managed-client call) - malformed args_json recorded authz allow before the remaining gates ran; now authz deny (reason: other) - audit line operation now reflects the target tool's real tier, not the caller's chosen call_tool_* door - batch dispatch no longer skips the audit-attempt bridge on an already-cancelled execution context - UpdateUserLogin returns the record a subject_mismatch/user_disabled refusal was decided against, closing a race that could silently drop auth_event's required user_id - HandleLogin now flags redirect_rejected on a pre-redirect failure, not only via the callback's stored pending state - EffectiveAuditLog now honours an explicit audit_log block on the personal edition; only the absent-block default is edition-keyed See specs/107-server-edition-sso-hardening/verification.md for the full per-finding table and verification commands. * fix(spec-107): cross-review round 3 for PR-D * fix(spec-107): cross-review round 4 for PR-D * fix(test): loosen TestUpstreamServersListOperation's CI-flaky 100ms budget macos-latest and windows-latest Unit Tests jobs on PR #1296 failed with "list took too long: 157.821625ms (should be < 100ms)" while ubuntu-latest passed on the identical commit. handleUpstreamServers/handleListUpstreams and emitActivityInternalToolCall are untouched by this PR (the new audit funnel in audit_funnel.go only wires into makeDirectModeHandler and the call_tool_* funnels), and this test calls handleUpstreamServers in-process with no HTTP layer at all, so the audit-line feature cannot be the cause. The 100ms wall-clock bound had no slack for shared/loaded CI runners, matching this repo's documented history of macOS/Windows timing flakes. Replace it with a 500ms ceiling (upstreamServersListCeiling) that still catches an architectural regression but tolerates runner noise, following the same pattern already used by preflightBenchPerOpCeiling in internal/httpapi/preflight_bench_test.go. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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.
Why
Two CI failures that hit Spec 105 PRs (#1282, #1284, #1285) without those PRs touching the code that failed:
internal/httpapi(fatal error: found pointer to free object,unexpected fault address 0xffffffffffffffffwith a garbage stack). Root cause:TestMutatingServerRoutes_AdminAllowed/registry-add-serverdriveshandleAddFromRegistrywith a controller stub that returnsnil, nil, nil; the handler dereferencedcfg.Name, chi's recoverer swallowed the nil-pointer panic and the test passed on every OS. Under Go 1.26 on windows/amd64 the recovered hardware exception corrupts the Go heap (golang/go#81238), so the test binary dies at a later GC — intermittently, on whichever PR runs next.TestServiceStartScanDeepOnRunsSourceResolutionAndPass2:TempDir RemoveAll cleanup: directory not empty. The Pass-2 goroutine writestools.jsoninto the server working dir afterResolveFullSourcereturns and before it registers its engine job, sowaitForScanIdlecan return in the "Pass 1 cleared, Pass 2 not yet started" window and teardown races the write (known rerun-clearable flake since 2026-08-31).What
handleAddFromRegistry: a nil config on the success branch is a JSON 500 (registry returned no server configuration), mirroring the nil-toleranceredactedRegistrySummaryalready has;TestAddFromRegistry_NilConfigIsAnErrorpins it (fails onmainwith the recovered panic). A fullinternal/httpapirun now logs zero recovered panics.waitForScanIdle.-race -count=20, also underGOMAXPROCS=1: green.🤖 Generated with Claude Code