Emit mesh/check as the bridge's own check.status, never through a tool (K1)
Review finding K1 of R4e, issue 1301 S4 (ADR 0503 §3, ADR 0513 §4: the forge requires mesh/check alone). gitea_check_status rested on "only the bridge invokes it", but node-tools invokes every tool and a tool is told no caller, so any session could set mesh/check and merge past the checker. The bridge now emits check.status on mesh.mod.mesh-delivery-forge-bridge.event. check.status, which the bus grants to its own account alone (emits), with the owed status, its key and its revision; the event id check.<key>.<rev> is the bus's dedup id. A status the bus did not take is not kept as done. OwedAct refuses mesh/check, and gitea.gitea_check_status leaves the invokes. The integration test shows the tool runner's account refused on that subject and gitea's durable consumer handed statuses emitted before it fetched.
This commit is contained in:
@@ -1,7 +1,9 @@
|
||||
package bridge
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"regexp"
|
||||
"slices"
|
||||
"strings"
|
||||
|
||||
@@ -31,15 +33,60 @@ type Act struct {
|
||||
// number, body), merge.ts gitea_merge_pull_request (owner, repo, number, head, approved_heads).
|
||||
const (
|
||||
commitStatusTool = "gitea.gitea_commit_status"
|
||||
// checkStatusTool is the one tool that writes mesh/check (R4e, issue 1301's option A): gitea_commit_status refuses that
|
||||
// context, and only this bridge invokes it. Its arguments: owner, repo, sha, state, description, target_url, context.
|
||||
checkStatusTool = "gitea.gitea_check_status"
|
||||
deliveryViewTool = "gitea.gitea_delivery_view"
|
||||
noteAppendTool = "gitea.gitea_note_append"
|
||||
addCommentTool = "gitea.gitea_add_comment"
|
||||
mergePullTool = "gitea.gitea_merge_pull_request"
|
||||
)
|
||||
|
||||
// CheckStatusEvent is the event the bridge writes mesh/check with (R4e finding K1; issue 1301 S4; ADR 0503 §3): no tool
|
||||
// writes mesh/check, since a tool is told no caller and node-tools invokes every tool. The bus grants its subject,
|
||||
// mesh.mod.mesh-delivery-forge-bridge.event.check.status, to this module's own account alone (its manifest's emits), and
|
||||
// the forge module consumes it and writes the status (mesh-catalog modules/gitea check.ts).
|
||||
const CheckStatusEvent = "check.status"
|
||||
|
||||
// CheckStatus is the body of a check.status: the status the checker owes on a head, the owed key and the revision it was
|
||||
// owed at, by which the forge module drops a stale one delivered after a newer.
|
||||
type CheckStatus struct {
|
||||
Owner string `json:"owner"`
|
||||
Repo string `json:"repo"`
|
||||
Sha string `json:"sha"`
|
||||
State string `json:"state"`
|
||||
Description string `json:"description"`
|
||||
TargetURL string `json:"target_url,omitempty"`
|
||||
Key string `json:"key"`
|
||||
Revision uint64 `json:"revision"`
|
||||
}
|
||||
|
||||
var wholeSha = regexp.MustCompile(`^([0-9a-f]{40}|[0-9a-f]{64})$`)
|
||||
|
||||
// CheckStatusMessage turns the checker's owed status (mesh-sdk go/delivery CheckerOwedKey, EncodeCheckerOwed) at revision rev
|
||||
// into the check.status it is emitted as: its id, `check.<key>.<rev>`, the bus's dedup id too, and its body. A key that is
|
||||
// not the checker's, a value that is not its status, and a head that is not a whole sha are errors, each saying why.
|
||||
func CheckStatusMessage(key string, rev uint64, value []byte) (id string, body []byte, err error) {
|
||||
repository, _, head, err := delivery.ParseCheckerOwedKey(key)
|
||||
if err != nil {
|
||||
return "", nil, err
|
||||
}
|
||||
o, err := delivery.DecodeCheckerOwed(value)
|
||||
if err != nil {
|
||||
return "", nil, fmt.Errorf("the checker's owed status %s: %w", key, err)
|
||||
}
|
||||
if o.Act != delivery.OwedStatus || o.Context != delivery.StatusContextCheck {
|
||||
return "", nil, fmt.Errorf("the checker's owed %s is %s %s, not the status %s", key, o.Act, o.Context, delivery.StatusContextCheck)
|
||||
}
|
||||
if !wholeSha.MatchString(head) {
|
||||
return "", nil, fmt.Errorf("the checker's owed %s names the head %q, not a whole sha", key, head)
|
||||
}
|
||||
owner, repo := ownerRepo(repository)
|
||||
body, err = json.Marshal(CheckStatus{Owner: owner, Repo: repo, Sha: head, State: o.State, Description: o.Text,
|
||||
TargetURL: o.Target, Key: key, Revision: rev})
|
||||
if err != nil {
|
||||
return "", nil, err
|
||||
}
|
||||
return fmt.Sprintf("check.%s.%d", key, rev), body, nil
|
||||
}
|
||||
|
||||
// MergeAct is the name of the act of merging a pull request.
|
||||
const MergeAct = "merge"
|
||||
|
||||
@@ -80,11 +127,11 @@ func OwedAct(key string, value []byte) (Act, error) {
|
||||
if o.Context != context {
|
||||
return Act{}, fmt.Errorf("the owed status %s says the context %q, not the %q its key names", key, o.Context, context)
|
||||
}
|
||||
tool := commitStatusTool
|
||||
if o.Context == delivery.StatusContextCheck {
|
||||
tool = checkStatusTool
|
||||
return Act{}, fmt.Errorf("the owed status %s is %s, which no tool writes: the bridge emits it as %s (CheckStatusMessage)",
|
||||
key, delivery.StatusContextCheck, CheckStatusEvent)
|
||||
}
|
||||
a.Call = Call{Tool: tool, Args: map[string]any{"owner": owner, "repo": repo, "sha": head, "context": o.Context,
|
||||
a.Call = Call{Tool: commitStatusTool, Args: map[string]any{"owner": owner, "repo": repo, "sha": head, "context": o.Context,
|
||||
"state": o.State, "description": o.Text, "target_url": o.Target}}
|
||||
case delivery.OwedView:
|
||||
if err := needsPull(); err != nil {
|
||||
|
||||
@@ -2,6 +2,7 @@ package bridge
|
||||
|
||||
import (
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"git.novox.be/novox/mesh-sdk/go/delivery"
|
||||
@@ -43,23 +44,60 @@ func TestOwedStatusIsACommitStatusCall(t *testing.T) { // 1341/AC1
|
||||
}
|
||||
}
|
||||
|
||||
func TestOwedMeshCheckGoesThroughTheCheckStatusToolAlone(t *testing.T) { // R4e 1301-S4 option A: only gitea_check_status writes mesh/check
|
||||
// checkerKV is the status the checker owes, as it writes it (mesh-sdk go/delivery CheckerOwedKey, EncodeCheckerOwed).
|
||||
func checkerKV(t *testing.T, o delivery.Owed) (string, []byte) {
|
||||
t.Helper()
|
||||
key, err := delivery.CheckerOwedKey("novox/mesh-delivery", 12, headA)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
v, err := delivery.EncodeCheckerOwed(delivery.Owed{Act: delivery.OwedStatus, Context: delivery.StatusContextCheck, State: "success", Text: "pass", Target: "https://view.example/checks/g.1"})
|
||||
v, err := delivery.EncodeCheckerOwed(o)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return key, v
|
||||
}
|
||||
|
||||
// R4e finding K1 (issue 1301 S4; ADR 0503 §3): no tool writes mesh/check, since a tool is told no caller and node-tools
|
||||
// invokes every tool. The bridge emits check.status, a subject only its own account may publish, and gitea writes it.
|
||||
func TestOwedMeshCheckIsNoToolCall(t *testing.T) { // K1
|
||||
key, v := checkerKV(t, delivery.Owed{Act: delivery.OwedStatus, Context: delivery.StatusContextCheck, State: "success", Text: "pass", Target: "https://view.example/checks/g.1"})
|
||||
a, err := OwedAct(key, v)
|
||||
if err == nil || !strings.Contains(err.Error(), "check.status") {
|
||||
t.Fatalf("OwedAct(mesh/check) = %+v, %v; want refused, naming check.status", a, err)
|
||||
}
|
||||
// Nor through the gate's own owed bucket: the context is refused whichever key carries it.
|
||||
gk, gv := owedValue(t, "novox/mesh-delivery", 12, "status=mesh/check",
|
||||
delivery.Owed{Act: delivery.OwedStatus, Context: delivery.StatusContextCheck, State: "success", Text: "pass", Target: "https://x.example"})
|
||||
if a, err := OwedAct(gk, gv); err == nil || !strings.Contains(err.Error(), "check.status") {
|
||||
t.Fatalf("OwedAct(gate's mesh/check) = %+v, %v; want refused", a, err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckStatusMessageCarriesTheOwedStatusAndItsRevision(t *testing.T) { // K1
|
||||
key, v := checkerKV(t, delivery.Owed{Act: delivery.OwedStatus, Context: delivery.StatusContextCheck, State: "success", Text: "pass", Target: "https://view.example/checks/g.1"})
|
||||
id, body, err := CheckStatusMessage(key, 42, v)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
want := Call{Tool: "gitea.gitea_check_status", Args: map[string]any{"owner": "novox", "repo": "mesh-delivery", "sha": headA,
|
||||
"context": "mesh/check", "state": "success", "description": "pass", "target_url": "https://view.example/checks/g.1"}}
|
||||
if !reflect.DeepEqual(a.Call, want) {
|
||||
t.Errorf("call = %+v, want %+v", a.Call, want)
|
||||
if CheckStatusEvent != "check.status" {
|
||||
t.Errorf("event = %q", CheckStatusEvent)
|
||||
}
|
||||
if id != "check."+key+".42" {
|
||||
t.Errorf("id = %q", id)
|
||||
}
|
||||
want := `{"owner":"novox","repo":"mesh-delivery","sha":"` + headA + `","state":"success","description":"pass","target_url":"https://view.example/checks/g.1","key":"` + key + `","revision":42}`
|
||||
if string(body) != want {
|
||||
t.Errorf("body = %s\nwant %s", body, want)
|
||||
}
|
||||
// A key that is not the checker's, and a value that is not its status, are no message.
|
||||
gk, gv := owedValue(t, "novox/mesh-delivery", 12, "status=mesh/merge-gate",
|
||||
delivery.Owed{Act: delivery.OwedStatus, Context: "mesh/merge-gate", State: "success", Text: "x", Target: "https://x.example"})
|
||||
if _, _, err := CheckStatusMessage(gk, 1, gv); err == nil {
|
||||
t.Error("a key of the gate made a check.status")
|
||||
}
|
||||
if _, _, err := CheckStatusMessage(key, 1, []byte("{")); err == nil {
|
||||
t.Error("a value that is no owed status made a check.status")
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user