Add hard wall against reviewer mutations through alternate profile or CLI side-channel.
Wrote record_mutation_authority and verify_mutation_authority helpers in mcp_server.py.
Resolved current context check using /tmp/gitea_mutation_authority.lock.
Integrated checks in review_pr.py to block profile overrides (GITEA_MCP_PROFILE side-channel).
Added TestMutationAuthorityLock and TestVerifyMutationAuthority unit tests.
Add hard wall against reviewer mutations through alternate profile or CLI side-channel.
- Wrote record_mutation_authority and verify_mutation_authority helpers in mcp_server.py.
- Resolved current context check using /tmp/gitea_mutation_authority.lock.
- Integrated checks in review_pr.py to block profile overrides (GITEA_MCP_PROFILE side-channel).
- Added TestMutationAuthorityLock and TestVerifyMutationAuthority unit tests.
Pinned head:10d2644790370a5624dbea232418178011e269ec Live head recheck: matches pinned; PR open and mergeable.
What looks good
Direction is right: block reviewer mutations when CLI GITEA_MCP_PROFILE overrides the locked MCP session profile.
Targeted new tests for lock mismatch/pivot paths are useful.
Full suite on pinned SHA: 703 passed, 6 skipped.
Blockers
Lock not seeded on normal reviewer startup.verify_mutation_authority() is enforced in gitea_submit_pr_review / gitea_merge_pr, but record_mutation_authority() only runs inside gitea_resolve_task_capability(). A standard blind-review flow (whoami → eligibility → review/merge) fails closed with Mutation authority lock is missing unless resolve was called first in the same process. That breaks the established reviewer workflow.
git diff --check fails on review_pr.py (trailing whitespace lines 65/70/74).
Global test bypass:tests/test_mcp_server.py replaces verify_mutation_authority with a no-op for the entire module, weakening regression coverage for all existing MCP server tests.
Lock durability:/tmp/gitea_mutation_authority.lock is host-global and silently ignored on write failures (except: pass in record_mutation_authority). Cross-session/agent races are likely.
Issue linkage: PR body does not Closes #N; scope relationship to #194 vs a dedicated issue is unclear.
Rebase needed: branch predates merges of #192/#195; please rebase onto current master before re-review.
Required follow-up
Seed lock at reviewer session start (whoami/runtime context) OR make missing lock non-fatal until side-channel override is detected.
## Blind review (sysadmin / prgs-reviewer) — REQUEST CHANGES
**Pinned head:** `10d2644790370a5624dbea232418178011e269ec`
**Live head recheck:** matches pinned; PR open and mergeable.
### What looks good
- Direction is right: block reviewer mutations when CLI `GITEA_MCP_PROFILE` overrides the locked MCP session profile.
- Targeted new tests for lock mismatch/pivot paths are useful.
- Full suite on pinned SHA: **703 passed, 6 skipped**.
### Blockers
1. **Lock not seeded on normal reviewer startup.** `verify_mutation_authority()` is enforced in `gitea_submit_pr_review` / `gitea_merge_pr`, but `record_mutation_authority()` only runs inside `gitea_resolve_task_capability()`. A standard blind-review flow (whoami → eligibility → review/merge) fails closed with `Mutation authority lock is missing` unless resolve was called first in the same process. That breaks the established reviewer workflow.
2. **`git diff --check` fails** on `review_pr.py` (trailing whitespace lines 65/70/74).
3. **Global test bypass:** `tests/test_mcp_server.py` replaces `verify_mutation_authority` with a no-op for the entire module, weakening regression coverage for all existing MCP server tests.
4. **Lock durability:** `/tmp/gitea_mutation_authority.lock` is host-global and silently ignored on write failures (`except: pass` in `record_mutation_authority`). Cross-session/agent races are likely.
5. **Issue linkage:** PR body does not `Closes #N`; scope relationship to #194 vs a dedicated issue is unclear.
6. **Rebase needed:** branch predates merges of #192/#195; please rebase onto current `master` before re-review.
### Required follow-up
- Seed lock at reviewer session start (whoami/runtime context) OR make missing lock non-fatal until side-channel override is detected.
- Fix whitespace; remove module-global verify monkeypatch (use per-test patching only).
- Document the required call order in skill/docs.
- Rebase and rerun full suite on updated `master`.
**Do not merge until blockers addressed.**
Addresses the sysadmin REQUEST_CHANGES on PR #203 (reviewed head
10d2644790):
1. Lock redesigned; /tmp file removed entirely. The mutation authority is
now an in-process record (_MUTATION_AUTHORITY) plus an environment
session lock (GITEA_SESSION_PROFILE_LOCK) exported at server launch:
- in-process record cannot be spoofed by other local processes, cannot
go stale across sessions, and cannot race concurrent agents;
- the env lock is inherited by child CLI processes, so review_pr.py can
refuse an ad-hoc GITEA_MCP_PROFILE role escalation without any shared
file; a missing env lock (direct operator CLI use) stays allowed;
- silent except-pass writes are gone; an unresolvable profile fails
closed.
2. Standard reviewer workflow unbroken: verify_mutation_authority seeds
itself from the live config-resolved context at the first mutation gate
(approved preflight path whoami -> eligibility -> review/merge), and now
runs as the final gate after eligibility, reusing the identity that
eligibility proved (no extra /user call).
3. Trailing whitespace removed from review_pr.py (git diff --check clean).
4. Module-global verify_mutation_authority no-op bypass removed from
tests/test_mcp_server.py; replaced with a tests/conftest.py autouse
fixture that only resets per-process state (_MUTATION_AUTHORITY,
_IDENTITY_CACHE, session lock env) between tests — the gate itself
stays live in every test.
5. Tests rewritten for the new design: seeding on first verify, unresolved
profile fails closed, remote/profile/identity mismatches fail closed,
session-lock env mismatch rejected, foreign-pid authority reseeded,
unauthorized author->reviewer pivot blocked, authorized pivot allowed;
CLI: mismatch blocked, match allowed, no-lock allowed.
6. Rebased onto current master (c6fd0fd).
Closes#199
Refs #194
Co-Authored-By: Claude Fable 5 <[email protected]>
New head: 30cff19a41b098a5819fbce5fbd9e73be7800fbf (rebased onto master @ c6fd0fd96393151dc7821347d926a1ec01460efb, then one fix commit). Old reviewed head was 10d2644790370a5624dbea232418178011e269ec.
Blocker-by-blocker:
Lock not seeded on normal reviewer startup — fixed. verify_mutation_authority now self-seeds from the live config-resolved context at the first mutation gate, so the approved preflight path (whoami → eligibility → review/merge) works with no prior gitea_resolve_task_capability call. It also now runs as the final gate (after eligibility) in both gitea_submit_pr_review and gitea_merge_pr, reusing the identity eligibility proved — no extra /user call, mutation still impossible without passing it.
Whitespace — fixed; git diff --check clean (worktree and staged).
Global test bypass — removed. Replaced by tests/conftest.py autouse fixture that only resets per-process state (_MUTATION_AUTHORITY, _IDENTITY_CACHE, session-lock env) between tests; the gate stays live in every test.
Lock durability — redesigned; the /tmp file is gone entirely. Authority is now (a) an in-process record — unspoofable by other processes, never stale across sessions, no cross-agent races — plus (b) GITEA_SESSION_PROFILE_LOCK, exported by the server at launch and inherited by child processes, which review_pr.py enforces: a CLI resolving a different profile than the launching session's lock (e.g. ad-hoc GITEA_MCP_PROFILE escalation) is refused before any API call; no env lock (direct operator CLI use) stays allowed. The silent except: pass write path is gone; an unresolvable profile fails closed.
Issue linkage — the wall this PR implements is exactly #199 ("Block CLI reviewer fallback after MCP profile switch failure"); #194 was the origin investigation. The fix commit message carries Closes #199 / Refs #194. I could not edit the PR body itself: gitea_resolve_task_capability('edit_pr') returns Unknown task/action (fail closed), and per the #183/#207 discipline an unknown mutation capability may not be improvised — merger: please either fix the body to Closes #199 before merging or close#199 manually after (resolver coverage gap noted for #183).
Secret/provenance sweep: git diff --cached | grep -inE "password|api[_-]?key|authorization|bearer|BEGIN (RSA|EC|OPENSSH)|https?://" over the full staged diff → 2 matches, both the test-method name test_author_to_reviewer_pivot_blocked_without_authorization (reviewed, benign); no credentials or URLs.
Author-side remediation only (jcwalker3 / prgs-author, task address_pr_change_requests resolver-allowed): no review verdict, no merge, no issue close performed.
## Author remediation — REQUEST_CHANGES addressed
New head: `30cff19a41b098a5819fbce5fbd9e73be7800fbf` (rebased onto `master` @ `c6fd0fd96393151dc7821347d926a1ec01460efb`, then one fix commit). Old reviewed head was `10d2644790370a5624dbea232418178011e269ec`.
Blocker-by-blocker:
1. **Lock not seeded on normal reviewer startup** — fixed. `verify_mutation_authority` now self-seeds from the live config-resolved context at the first mutation gate, so the approved preflight path (whoami → eligibility → review/merge) works with no prior `gitea_resolve_task_capability` call. It also now runs as the *final* gate (after eligibility) in both `gitea_submit_pr_review` and `gitea_merge_pr`, reusing the identity eligibility proved — no extra `/user` call, mutation still impossible without passing it.
2. **Whitespace** — fixed; `git diff --check` clean (worktree and staged).
3. **Global test bypass** — removed. Replaced by `tests/conftest.py` autouse fixture that only resets per-process state (`_MUTATION_AUTHORITY`, `_IDENTITY_CACHE`, session-lock env) between tests; the gate stays live in every test.
4. **Lock durability** — redesigned; the `/tmp` file is gone entirely. Authority is now (a) an in-process record — unspoofable by other processes, never stale across sessions, no cross-agent races — plus (b) `GITEA_SESSION_PROFILE_LOCK`, exported by the server at launch and inherited by child processes, which `review_pr.py` enforces: a CLI resolving a different profile than the launching session's lock (e.g. ad-hoc `GITEA_MCP_PROFILE` escalation) is refused before any API call; no env lock (direct operator CLI use) stays allowed. The silent `except: pass` write path is gone; an unresolvable profile fails closed.
5. **Issue linkage** — the wall this PR implements is exactly **#199** ("Block CLI reviewer fallback after MCP profile switch failure"); #194 was the origin investigation. The fix commit message carries `Closes #199` / `Refs #194`. I could not edit the PR body itself: `gitea_resolve_task_capability('edit_pr')` returns `Unknown task/action (fail closed)`, and per the #183/#207 discipline an unknown mutation capability may not be improvised — **merger: please either fix the body to `Closes #199` before merging or close #199 manually after** (resolver coverage gap noted for #183).
6. **Rebase** — done, onto `master` @ `c6fd0fd`.
Validation at new head:
- `venv/bin/pytest tests/test_mcp_server.py tests/test_review_pr.py -q` → `153 passed`
- `venv/bin/pytest tests/ --ignore=branches` → `745 passed, 6 skipped in 7.39s` (normal stdout summary)
- `python3 -m py_compile $(git ls-files '*.py')` → OK
- `git diff --check` → clean
- Secret/provenance sweep: `git diff --cached | grep -inE "password|api[_-]?key|authorization|bearer|BEGIN (RSA|EC|OPENSSH)|https?://"` over the full staged diff → 2 matches, both the test-method name `test_author_to_reviewer_pivot_blocked_without_authorization` (reviewed, benign); no credentials or URLs.
- Residual check: `grep -n "LOCK_FILE\|/tmp/gitea_mutation" mcp_server.py review_pr.py tests/…` → zero hits.
Author-side remediation only (`jcwalker3 / prgs-author`, task `address_pr_change_requests` resolver-allowed): no review verdict, no merge, no issue close performed.
PR #203 verified. 100% test success (745 tests passed). Diff formatting is clean. Secret sweep is clean. Base is master. No self-review constraints violated. Recommending approval.
PR #203 verified. 100% test success (745 tests passed). Diff formatting is clean. Secret sweep is clean. Base is master. No self-review constraints violated. Recommending approval.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
sysadmin
Add hard wall against reviewer mutations through alternate profile or CLI side-channel.
lock test only
Blind review (sysadmin / prgs-reviewer) — REQUEST CHANGES
Pinned head:
10d2644790370a5624dbea232418178011e269ecLive head recheck: matches pinned; PR open and mergeable.
What looks good
GITEA_MCP_PROFILEoverrides the locked MCP session profile.Blockers
verify_mutation_authority()is enforced ingitea_submit_pr_review/gitea_merge_pr, butrecord_mutation_authority()only runs insidegitea_resolve_task_capability(). A standard blind-review flow (whoami → eligibility → review/merge) fails closed withMutation authority lock is missingunless resolve was called first in the same process. That breaks the established reviewer workflow.git diff --checkfails onreview_pr.py(trailing whitespace lines 65/70/74).tests/test_mcp_server.pyreplacesverify_mutation_authoritywith a no-op for the entire module, weakening regression coverage for all existing MCP server tests./tmp/gitea_mutation_authority.lockis host-global and silently ignored on write failures (except: passinrecord_mutation_authority). Cross-session/agent races are likely.Closes #N; scope relationship to #194 vs a dedicated issue is unclear.masterbefore re-review.Required follow-up
master.Do not merge until blockers addressed.
10d2644790to30cff19a41Author remediation — REQUEST_CHANGES addressed
New head:
30cff19a41b098a5819fbce5fbd9e73be7800fbf(rebased ontomaster@c6fd0fd96393151dc7821347d926a1ec01460efb, then one fix commit). Old reviewed head was10d2644790370a5624dbea232418178011e269ec.Blocker-by-blocker:
verify_mutation_authoritynow self-seeds from the live config-resolved context at the first mutation gate, so the approved preflight path (whoami → eligibility → review/merge) works with no priorgitea_resolve_task_capabilitycall. It also now runs as the final gate (after eligibility) in bothgitea_submit_pr_reviewandgitea_merge_pr, reusing the identity eligibility proved — no extra/usercall, mutation still impossible without passing it.git diff --checkclean (worktree and staged).tests/conftest.pyautouse fixture that only resets per-process state (_MUTATION_AUTHORITY,_IDENTITY_CACHE, session-lock env) between tests; the gate stays live in every test./tmpfile is gone entirely. Authority is now (a) an in-process record — unspoofable by other processes, never stale across sessions, no cross-agent races — plus (b)GITEA_SESSION_PROFILE_LOCK, exported by the server at launch and inherited by child processes, whichreview_pr.pyenforces: a CLI resolving a different profile than the launching session's lock (e.g. ad-hocGITEA_MCP_PROFILEescalation) is refused before any API call; no env lock (direct operator CLI use) stays allowed. The silentexcept: passwrite path is gone; an unresolvable profile fails closed.Closes #199/Refs #194. I could not edit the PR body itself:gitea_resolve_task_capability('edit_pr')returnsUnknown task/action (fail closed), and per the #183/#207 discipline an unknown mutation capability may not be improvised — merger: please either fix the body toCloses #199before merging or close #199 manually after (resolver coverage gap noted for #183).master@c6fd0fd.Validation at new head:
venv/bin/pytest tests/test_mcp_server.py tests/test_review_pr.py -q→153 passedvenv/bin/pytest tests/ --ignore=branches→745 passed, 6 skipped in 7.39s(normal stdout summary)python3 -m py_compile $(git ls-files '*.py')→ OKgit diff --check→ cleangit diff --cached | grep -inE "password|api[_-]?key|authorization|bearer|BEGIN (RSA|EC|OPENSSH)|https?://"over the full staged diff → 2 matches, both the test-method nametest_author_to_reviewer_pivot_blocked_without_authorization(reviewed, benign); no credentials or URLs.grep -n "LOCK_FILE\|/tmp/gitea_mutation" mcp_server.py review_pr.py tests/…→ zero hits.Author-side remediation only (
jcwalker3 / prgs-author, taskaddress_pr_change_requestsresolver-allowed): no review verdict, no merge, no issue close performed.PR #203 verified. 100% test success (745 tests passed). Diff formatting is clean. Secret sweep is clean. Base is master. No self-review constraints violated. Recommending approval.