fix: owner-gate remaining MCP/config mutations

[atomsbaza]

Problem / Motivation

The dashboard still exposes MCP and configuration mutation routes that can write machine-global state, restart sessions, install or remove capabilities, or change gateway behavior. The routes this PR targets did not share one owner authorization boundary, so the protection could drift between handlers. Several owner-denial helpers also duplicated the same identity rule in different modules.

Why it matters

MCP and dashboard configuration writes affect the host and future sessions, not only the requesting tab. An app-scoped token or an authenticated non-owner must not be able to trigger those side effects. Authorization must happen before JSON parsing, locks, resolver work, process spawning, or any other mutation side effect.

What changed

• One shared owner boundary. Centralize the owner predicate and denial response in the shared dashboard handler module; the response is 403 with machine-readable code owner_only.
• Enumerate the invariant. Gate all 20 targeted mutation routes across MCP sync/apply/toggle/custom/discovery/server/probe/measure/quarantine, MCP gateway control, config, and theme operations. Six further mutating routes in the same registrar (dashboard config write, onboarding import, OAuth relay, connections mint/cancel) remain outside this boundary and are tracked in migrate the 7 remaining inline owner-denial sequences to require_owner_dashboard_request #6224.
• Preserve the internal-auth contract. X-Internal-Secret remains an explicit opt-in exception only for MCP server registration (PUT/DELETE /api/mcp/servers/{name}); it does not bypass the other mutation routes.
• Remove drift. Replace duplicated owner-denial helpers with the shared boundary while preserving compatibility exports used by existing callers and tests.
• Test the ordering guarantee. Walk the real route registrar and send requests without JSON bodies, proving non-owner and app-token requests receive 403 owner_only before a handler can return a body-parse error or perform a side effect.

Tests

• Targeted authorization/config suite: 702 passed, 6 deselected.
• Route invariant test: 4 passed.
• Fixture regression group: 146 passed.
• Additional probe/theme/gateway route tests: 65 passed.
• Compatibility regression for MCP sync handlers: 66 passed.
• py_compile passed for all changed Python files.
• git diff --check passed.

The final targeted run deselected 6 environment-only test nodes. The broader relevant run had 3 environment-only failures: two pip-channel checks because the uv-created environment has no pip module, and one optional STT capability check because voice/Transcribe dependencies were not installed. Formatting/lint executables (black, isort, flake8, and ruff) were unavailable in the isolated worktree environment.

Review and CI status

The local diff was reviewed and the available targeted validation passed. No separate model-pinned review is claimed here. Upstream automated checks for this fork PR are currently held by GitHub action_required approval for the fork workflows; this is an execution-permission blocker, not a source-test failure. After an upstream maintainer approves and runs the workflows, CI and review results should be evaluated for the current head revision.

Scope

Backend-only authorization and regression coverage. No frontend behavior changes and no screenshot evidence is applicable.

Closes #5014

[bolichen97]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[bolichen97]

🤖 Kiro Crew [operator: bolichen97#66809557]: This PR has been inactive with a blocked CI state. I've assessed the blockers and they appear resolvable — I'll push fixes directly to this branch as a co-author.

Assessment: The branch is behind main with merge conflicts (mergeable_state=dirty), and Backend Lint fails on one mechanical F401 — source_providers.py still imports is_owner_dashboard_request from _shared after the local copy was removed, but no longer references it (the centralization is correct; the import is just orphaned). Plan: rebase onto latest main, resolve conflicts guided by the PR's intent (one shared owner-gate boundary in _shared, taking main's handler/test structure + this PR's gating logic), drop the unused import, and run local gates before force-pushing. All other CI (Backend/Frontend/Coverage/Build tests) passed on the pre-conflict head. No design or scope change.

If you'd prefer I don't touch this PR, add the pr-no-autofix label.

[bolichen97]

🤖 Kiro Crew [operator: bolichen97#66809557]

Rebased this PR onto current main (was CONFLICTING) and pushed 6adfb57d3 (was b824cffa6). What changed and why:

Conflict resolution — main's structure + this PR's consolidation, guard semantics untouched:

• Main added stale_owner_session_response / STALE_OWNER_SESSION_CODE (401 stale_session_reauth relabel for signed pre-owner bootstrap sessions) after this PR's base, and every owner-gate deny site on main runs it. To keep that behavior at the call sites this PR migrates, require_owner_dashboard_request now runs the same relabel on its deny path (lazy import from source_providers to avoid the import cycle). Relabel-only: it runs strictly after the deny decision and never grants — the fail-closed semantics of the guard are unchanged.
• source_providers.py: kept main's stale_owner_session_response machinery in place; removed the moved is_owner_dashboard_request / _LOCAL_DASHBOARD_OWNER_SUBJECTS definitions per this PR, importing them from _shared instead. is_owner_dashboard_request is kept as a # noqa: F401 compatibility re-export because 10+ untouched modules (chat_handlers, prompts, secrets, tailnet_mobile, handlers_cloud, …) import it from here. _LOCAL_DASHBOARD_OWNER_SUBJECTS is genuinely used in-file (stale relabel + source-owner gate) — this resolves the F401 blocker flagged pre-rebase.
• agents.py / mcp_apps.py: took this PR's side (delete _require_owner / the inline gate in favor of the shared guard); the stale relabel those main-side versions carried is preserved via the shared guard above.

Rebase-integration fixes:

• test_dashboard_handlers_core_coverage.py: TestFallbackModelPatch was added on main after this PR's base and built a bare app for the now-owner-gated PATCH /api/config/kirocrew; gave it the same _owner_identity middleware + state pattern this PR established for the sibling classes.
• test_stale_owner_session_signal.py: migrated test_agents_gate_labels_stale_bootstrap_denial from the deleted agents._require_owner to the shared guard (same contract, same assertions).
• error-code-baseline.json: regenerated with python test/test_error_code_contract.py --update — missing_code 1357 → 1355 (this PR's guard adds code fields where the old inline gates had none).

Local gates: isort / flake8 / black-baseline clean; mypy clean on the touched files; 1090 targeted tests pass (all 24 test files this PR touches + the stale-signal and error-code contract suites).

Single commit preserved (original authorship kept, Kiro Crew co-author trailer added).

[bolichen97]

🤖 Kiro Crew [operator: bolichen97#66809557]

Review-ready ✅ at head 810560d02 (rebased onto main 4b682353c, single commit, original authorship preserved).

• CI: 69 checks green, 10 skipped, 0 failures. PR Readiness: success. MERGEABLE.
• AI lanes: GPT ✅ · Opus ✅ · Design PASS · UX ✅ · First Principles 🟡 CONCERNS (advisory) — dispositioned above: PR body corrected (20 routes, no oversell), the 6 ungated sibling routes and 7 inline denial copies tracked in migrate the 7 remaining inline owner-denial sequences to require_owner_dashboard_request #6224, and the error=-param subtraction rebutted with the two tests that pin that string.

Drive summary (from CONFLICTING + F401 to green):

1. Cherry-picked the original commit onto current main (branch was 3300+ commits stale); resolved 4 conflicts keeping main's newer stale_owner_session_response machinery — the shared guard now runs the stale-session 401 relabel on its deny path, so no main behavior regressed.
2. Fixed the F401s: _LOCAL_DASHBOARD_OWNER_SUBJECTS is genuinely used; is_owner_dashboard_request kept as a compat re-export for its 10+ importers; later dropped the zero-consumer copy in agents.py per First Principles.
3. Repaired 3 rebase-seam test files that main added after this PR's base (TestFallbackModelPatch, test_mcp_mutation_identifiers.py, TestResetEndpoint) — each needed the owner-identity fixture the PR established.
4. Mid-drive, main merged a new ungated mutating route (POST /api/mcp/quarantine/clear) and this PR's own route-walk invariant caught it — gated it and pinned it in the set (validation the design works).

Not merged — that decision stays with maintainers. Thanks @atomsbaza for the original consolidation design; the fail-closed guard semantics are exactly as you wrote them.

[bolichen97]

I re-read the complete current 26-file diff after the head changed from 48111dd9 to 9a0d9ca3. Please rebase this PR onto current main and reduce it to residual-only scope.

Already merged coverage:

• fix: gate agent-spec mutating endpoints on owner auth (#4944) #5011 (ab437bcb) owner-gates the agent-spec mutators.
• refactor: migrate inline owner-denial sequences to shared helper #6245 (f38562c9) provides the shared require_owner_dashboard_request guard and migrates many inline owner-denial sequences. The current branch still carries helper relocation/compatibility work that now overlaps that mainline implementation.

Keep the distinct residual: owner gates for the remaining MCP/custom/discovery/gateway/config mutations (including probe/measure/quarantine/sync/toggle/remove/apply, custom add/update, discovery install, gateway enable/stub/resolve refresh, and config/theme writes), plus the route-walk and route-specific authorization tests. Dropping the already-merged helper and agent-spec portions will make the security delta reviewable without losing those uncovered routes.

[bolichen97]

Audit note — part of this has already landed; the rest has not

This PR is not a duplicate and is not finished by anything on main. The audit checked it part by part against main, and some of what it does is already there. Flagging it so a reviewer does not have to rediscover the overlap, and so the PR is not mistaken for fully-covered work.

Already landed

Only ONE claimed reference really landed and touches this work: #6245 -- the issue/PR reference check row 6245 PR closed 2026-08-28T06:10:12Z, and the landed-commit index for main line 848 f38562c96b659ba0975d5b1bb3f86596992f1e64 2026-08-27 refactor(dashboard): migrate inline owner-denial sequences to shared helper (#6245). Confirmed merged. #5011 also landed (the issue/PR reference check: PR closed 2026-08-22T07:39:58Z; landed as ab437bc) but the initial scan correctly scored it ADJACENT/coverage NONE and I confirm it covers none of this PR's routes. the initial scan claims that COLLAPSE now that the issue/PR reference check is populated (it was 0 bytes in the initial scan): #5014 (the PR body's 'Closes #5014') is an ISSUE, open -- 5014 ISSUE open -- so it cannot cover code; #6224 (the body's deferral target) is an ISSUE, closed -- 6224 ISSUE closed -- also not a PR. Neither was ever asserted as a coverage source, but the initial scan left both 'UNVERIFIED as to kind' and now they are settled. The three remaining the initial scan relations -- #7308, #6307, #7393 -- are all still-OPEN PRs (7308 PR open, 6307 PR open, 7393 PR open), so none of them covers anything; the initial scan had already scored all three coverage NONE, so there is no merged-vs-open error to correct here.

Which parts main already has

Only the shared boundary this PR builds ON, not any route it gates. #6245 (f38562c) landed: (1) require_owner_dashboard_request and _owner_denial_response in src/kiro_crew/dashboard/handlers/_shared.py (main lines 1890 and 1937; git log origin/main -S'async def require_owner_dashboard_request' -- .../_shared.py names f38562c as the sole commit) -- so the body's bullet 'One shared owner boundary. Centralize the owner predicate and denial response' and the 403 + code: owner_only shape are already on main; (2) test/dashboard_owner_helpers.py (added by f38562c), the as_owner/NoConfiguredOwner fixtures that make this PR's 13 hand-rolled _owner_identity middlewares and MagicMock claim stubs redundant; (3) test/test_agent_config_owner_gate_invariant.py (added by f38562c), the registrar-wide walk whose _KNOWN_UNGATED_ROUTES debt list enumerates EXACTLY this PR's 20 routes plus 4 others, with _MAX_KNOWN_UNGATED_ROUTES = 24; (4) the six sibling mutations the PR body still defers to #6224 -- require_owner_dashboard_request(request, ...) is on main at connections.py:185 (mcp_oauth_relay), :336 (connections_mint), :485 (connections_cancel), :668 (connections_premint) and files.py:3682 (dashboard_config.write), with the two onboarding-import routes recorded as _PRE_OWNER_EXCLUSIONS; that paragraph of the body is stale; (5) handlers/agents.py:150 _require_owner already delegates to the shared guard on main, which makes this PR's test/test_stale_owner_session_signal.py retarget cosmetic rather than load-bearing.

What is still genuinely yours

The entire security payload -- 20 route gates, the guard extension, the kiro_prerequisite migration, and the new invariant test -- is absent from main. (a) ZERO of the 20 routes is gated: git grep 'require_owner_dashboard_request|is_owner_dashboard_request' origin/main -- handlers/mcp.py handlers/mcp_custom.py handlers/mcp_discover.py handlers/core.py returns NOTHING, and main's api_mcp_probe / api_mcp_sync / api_mcp_toggle / api_theme_config(PUT) / api_kirocrew_config(PUT) / api_kirocrew_config_patch all fall straight through to body = await request.json() or their first side effect. Every one of the 19 new operation labels is absent from src/ (mcp.probe, mcp.measure, mcp.quarantine_clear, mcp.sync, mcp.apply, mcp.toggle, mcp.toggle_tool, mcp.toggle_all, mcp.remove, mcp.server_detail, mcp.custom_add, mcp.custom_update, mcp.discover_install, mcp_gateway.enable, mcp_gateway.set_stub, mcp_gateway.resolve_refresh, config.theme.write -> 0 files each; config.update and config.patch DO appear in core.py at 1259/1396/1413/1441 and 1859 but only as pre-existing SEL operation= labels inside the handlers' own _deny/_log_sel, never as an owner gate). main's OWN test file names these 20 as debt: _KNOWN_UNGATED_ROUTES documents them as 'candidates for future owner-gating work'. (b) The guard's three new keyword params are unlanded -- main's signature is exactly async def require_owner_dashboard_request(request: web.Request, operation: str); allow_internal_auth -> 0 hits in src/, and main's guard hardcodes resources="non_owner_block" and passes no error= to the SEL record or to _owner_denial_response. So the X-Internal-Secret opt-in for PUT/DELETE /api/mcp/servers/{name} has no mechanism on main. (c) handlers/kiro_prerequisite.py on main still carries its module-local _LOCAL_DASHBOARD_OWNER_SUBJECTS (line 23), _is_dashboard_owner (line 66) and _dashboard_owner_only (line 78, with operation="kiro_prerequisite_access"), called at lines 117/124/147/209 -- the migration is unlanded. (d) test/test_mcp_config_endpoints_owner_auth.py does not exist on main (git ls-tree origin/main <path> -> empty), so the 4 tests pinning the 20-route set, the auth-before-body-parse ordering, app-token refusal, and the internal-auth exception being limited to server registration are all new. (e) Three residual redundancies against #6245 that a rebase must resolve rather than land: the PR adds a byte-identical second copy of is_owner_dashboard_request + _LOCAL_DASHBOARD_OWNER_SUBJECTS into _shared.py while source_providers.py:6048/:5993 keeps the original AND stays the guard's own lazy import (the PR does not touch source_providers.py); it hand-rolls owner-identity scaffolding in 13 test files instead of using the landed test/dashboard_owner_helpers.py; and it never prunes its 20 now-gated routes from main's _KNOWN_UNGATED_ROUTES nor lowers _MAX_KNOWN_UNGATED_ROUTES from 24 to 4, which that test's own docstring requires ('lower _MAX_KNOWN_UNGATED_ROUTES to the new size to keep the ratchet tight').

Suggested action: REBASE — the remainder is real work; rebase onto the landed part rather than closing.

---

_{From a repository-wide duplicate/overlap audit of every pull request open against main (2026-09-02, 330 PRs, one reviewer per PR). Each PR was read as its full merge-base diff plus its description and every comment and review, then compared against each candidate PR's own diff and against origin/main at 1a765b88ceb7. This PR is not being closed — the note is informational. If the reading is wrong, please correct the reasoning rather than just the conclusion.}

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #7308 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7308: CONTINUE_DEVELOPMENT. Same nine handlers, same lines, complementary goals. Neither should be closed; sequence them and have the second author rebase onto the first. Files: src/kiro_crew/dashboard/handlers/mcp.py.
• PR #7393 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #7393: REBASE. Same campaign, no code conflict, but they entrench two different owner-gate spellings and error codes. Worth one maintainer decision on whether the skills family should land on the shared _shared.py boundary; that decision should not block 7393, since 5161 gives the skills routes no coverage at all. Files: src/kiro_crew/dashboard/handlers/_shared.py.
• This PR is PARTIALLY_COVERED with PR #6245. Coverage is explicitly incomplete; this finding is not a completion or closure claim. Recommended action for PR #5161: REBASE. Main landed the boundary this PR builds on, not any route it gates. Rebasing onto f38562c lets the PR drop the duplicated predicate, reuse test/dashboard_owner_helpers.py, and prune its 20 entries from _KNOWN_UNGATED_ROUTES while keeping the whole security payload. Files: src/kiro_crew/dashboard/handlers/_shared.py, src/kiro_crew/dashboard/handlers/mcp.py, test/test_mcp_config_endpoints_owner_auth.py.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.

[bolichen97]

Rebased onto main 9af9543b0 by a maintainer as part of the 2026-09-08 open-PR audit (was 880 commits behind).

Conflict resolutions:

• handlers/mcp.py: main (refactor(dashboard): sweep tranche 3 onto read_bounded_json (#5587) #7308) moved 5 of these handlers onto read_bounded_json. Kept main's bounded read and put your owner gate above it, so auth still runs before any parse.
• handlers/_shared.py: kept main's synchronous SEL enqueue (Offload SEL first-touch init at gateway startup, then delete the 18 per-site to_thread audit wrappers #8608 removed the thread hop) and kept your resources=/error= parameters and the allow_internal_auth opt-in.
• test/test_mcp_apply.py, test/test_mcp_mutation_identifiers.py: kept main's real body streams (attach_body / BodyStreamPayload) plus your owner-claim stubs.

Rebase follow-ups: kiro_prerequisite.api_kiro_prerequisite_update_cli is new on main and called the _dashboard_owner_only helper you removed, so it now uses the shared guard; test_stt_config_field_types.py and test_config_agent_fields_redaction.py are new on main and hit now-gated routes, so they get owner claims via main's dashboard_owner_helpers.

Gates run locally on changed files: black (8 files remain in .github/black-baseline.txt, unchanged), isort, flake8, and pytest across the 25 affected test files (all green).

Please review the resolutions. A maintainer push makes the maintainer the last pusher, so a second approver is needed under the repo's last-push rule. Reply if anything looks wrong.

Note, not changed here: main's test_agent_config_owner_gate_invariant.py still lists all 20 of these routes in _KNOWN_UNGATED_ROUTES with the ceiling at 20. Now that they are gated, that debt list and its ceiling can be shrunk in this PR.