sixfold-space/madtea

SHA256

Fork 0

feat(guard): enforce the ADR 0046 write guard on every forge write surface #790

Merged

hexajon merged 4 commits from issue-660-client-guard into main

2026-10-16 02:41:40 +00:00

hexajon commented

2026-10-16 02:41:35 +00:00

Owner

Summary

ADR 0046 decides one write guard that refuses a forge write by its declared effect, on every surface, with no override. This wires that guard into the forge client so every write passes it.

The client calls the guard before every write: the typed JSON path, multipart uploads, and the raw api passthrough. A read is never guarded. A client built with no guard refuses every write, so a path that skips install fails closed (#660).
The raw api passthrough routes through the guard in all three modes. It receives the /api/v1-relative path, so the effect matcher can classify it; a path with no v1 form stays unmatched and the write is refused as an undeclared endpoint (#784).
A parity test locks "no write around the guard" for the passthrough: a write calls the guard, a refusal stops the forge call, and a read never calls it (#785).
The diffpatch endpoint is classified as a content write, so a diffpatch to the default branch is guarded (#786).
The delete-tagging completeness test covers every subpackage (#787).

This satisfies the passthrough-limit and parity-test items of the epic (#668, #669).

One follow-up stays open: the parity test does not yet cover a full-URL-mode write or assert the normalized endpoint value (#789). The guard refuses any endpoint it cannot match, so this is test coverage, not an open hole.

Part of the write-guard epic (#670).

Closes #660
Closes #784
Closes #785
Closes #786
Closes #787

## Summary ADR 0046 decides one write guard that refuses a forge write by its declared effect, on every surface, with no override. This wires that guard into the forge client so every write passes it. - The client calls the guard before every write: the typed JSON path, multipart uploads, and the raw api passthrough. A read is never guarded. A client built with no guard refuses every write, so a path that skips install fails closed (#660). - The raw api passthrough routes through the guard in all three modes. It receives the /api/v1-relative path, so the effect matcher can classify it; a path with no v1 form stays unmatched and the write is refused as an undeclared endpoint (#784). - A parity test locks "no write around the guard" for the passthrough: a write calls the guard, a refusal stops the forge call, and a read never calls it (#785). - The diffpatch endpoint is classified as a content write, so a diffpatch to the default branch is guarded (#786). - The delete-tagging completeness test covers every subpackage (#787). This satisfies the passthrough-limit and parity-test items of the epic (#668, #669). One follow-up stays open: the parity test does not yet cover a full-URL-mode write or assert the normalized endpoint value (#789). The guard refuses any endpoint it cannot match, so this is test coverage, not an open hole. Part of the write-guard epic (#670). Closes #660 Closes #784 Closes #785 Closes #786 Closes #787

hexajon added 4 commits

2026-10-16 02:41:35 +00:00

feat(guard): #660 enforce the ADR 0046 forge write guard on the first-class CLI and MCP surfaces 999b838b31

Wire the forge write guard into the gitea client so the recorded write limits
hold on the first-class CLI and MCP write paths. The client calls a WriteGuard
interface (internal/gitea) before every write; a client with no installed guard
refuses every write (fail closed). The service adapter (internal/service/
forgeguard) resolves each write's effects (the operation's declared effects when
a first-class operation tags the context, else the endpoint's effects) and
applies the default-branch, out-of-scope, and destroy decisions from
internal/service/guard. A concrete-to-template matcher (internal/opspec
MatchEndpoint) maps the client's concrete paths to the effect registry. Main
installs the adapter at startup.

Every first-class forge write tags its operation: the eight content and branch
operations, and the recoverable deletes, so prune's proven-safe remote-branch
delete and the ordinary comment, label, milestone, and similar deletes keep
working. A source-level completeness test fails the gate on any client Delete*
call site with no tag. The two gated unprovable destroys, packages/delete and
admin/unadopted-delete, are intentionally left untagged, so the client refuses
them; the refusal names raw git and the forge web UI, and the test records them
as a documented exception.

The per-handler out-of-scope checks move off the MCP handlers, because the
client now enforces the out-of-scope rule on the CLI too, not only MCP.

The raw api passthrough is NOT yet routed through this guard; that work and its
parity test are tracked as #784 and #785.

feat(guard): #784 #785 route the raw api passthrough through the write guard cb566d0fcd

The raw api passthrough reached the forge without the ADR 0046 write guard:
rawAPI dispatched to doRequest, doRequestAbsolute, and doRequestURL, none of
which call the guard. So a passthrough DELETE hard-deleted, and a passthrough
content write reached the default branch or an out-of-scope repo. Route rawAPI
through guardWrite before it dispatches (#784). The guard receives the
/api/v1-relative path, normalized from the absolute and full-URL modes, so the
effect matcher can classify it; a path with no v1 form stays unmatched and a
write to it is refused as an endpoint no vendored spec declares. Reads and
pagination are never guarded.

Add the "no write around the guard" parity test (#785): a write through every
raw passthrough mode calls the guard, and a read never does. The two api test
packages do passthrough writes, so they carry the allow-all test install.

fix(guard): #786 classify the diffpatch endpoint as a content write 6a4bd2b88d

The diffpatch endpoint was effNone while its first-class operations apply-diff
and commit-patch are default-branch, so a passthrough diffpatch to the default
branch stayed allowed even after the passthrough is guarded. diffpatch applies a
patch and commits it, so it writes repo content on a branch: classify it
effContentWrite.

Add a consistency test: for every content-or-branch first-class operation, the
endpoint it maps to must declare at least the operation's effects, so the
passthrough view (keyed by endpoint) is never weaker than the first-class view
(keyed by operation). It caught the diffpatch divergence and is self-validating
for any sibling.

test(guard): #787 broaden the delete-tagging completeness test to every subpackage

drift / govulncheck (pull_request) Has been skipped

Details

drift / surface-diff (pull_request) Successful in 52s

Details

drift / forgejo-smoke (pull_request) Has been skipped

Details

ci / mcp-conformance (pull_request) Successful in 1m33s

Details

ci / mcp-conformance (push) Successful in -28s

Details

drift / govulncheck (push) Has been skipped

Details

drift / surface-diff (push) Successful in -29s

Details

drift / forgejo-smoke (push) Has been skipped

Details

ci / gate (push) Successful in 11m37s

Details

ci / gate (pull_request) Successful in 20m27s

Details

56ee3f9916

The delete-tagging completeness test scanned only internal/service top-level and
matched only the s.client.Delete* shape, so an untagged remote-branch delete in
a subpackage (the workflow finish cleanup, which calls DeleteBranch on a gitea
client held in a variable) would not be caught and would be silently refused in
production. Broaden the scan to the whole internal/service tree with WalkDir, and
match a client.Delete* call on a client variable too. A destroy that does not
start with Delete is a known residual gap, noted in the test.

hexajon merged commit 56ee3f9916 into main

2026-10-16 02:41:40 +00:00

No reviewers

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

sixfold-space/madtea!790

No description provided.

Rows
Columns