Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ carries the product now.
and posts `corral/gate = pass|fail` — a status branch protection **requires**, so a
red or missing verdict blocks the merge. Fail-closed (a `success` is only ever posted
on a real exit-0), and the gated SHA is provably the merged SHA. v1 is GitHub +
opt-in (`CORRALAI_GATE_POLICIES`).
opt-in (one `CORRALAI_GATE_POLICY_<NAME>` per repo).
- **The control gate.** The merge gate runs the *repo's own* check; the control gate
runs the **control owner's independently-vetted tests** against each PR head — the
person accountable for code they didn't write sets the bar, and the author can't
Expand Down
11 changes: 7 additions & 4 deletions cmd/corral-wrangler/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,14 @@
// from CORRALAI_BRAIN_KEY above (that's an Ed25519 IDENTITY SEED, not a bearer token — do not reuse it)
// CORRALAI_REKOR_URL Sigstore Rekor instance report_build anchors signed build attestations to (default https://rekor.sigstore.dev);
// `corral certify verify` checks the same default unless --rekor-url overrides it
// CORRALAI_GATE_POLICIES repo merge gate: ";"-separated policies "repo=owner/name,base=main,net=false,timeout=600,cmd=go test ./...";
// cmd= MUST be the last field — everything after it is the command verbatim (commas
// allowed, never split) so "cmd=go test -run A,B ./..." isn't silently truncated;
// CORRALAI_GATE_POLICY_<NAME> repo merge gate: ONE policy per variable, "repo=owner/name,base=main,net=false,timeout=600,cmd=go test ./...";
// cmd= MUST be last — everything after it is the command VERBATIM to the end of the
// value: commas, semicolons, quotes and newlines included, so nothing can truncate it
// into a weaker command that exits 0 and posts a wrongful success;
// timeout= is seconds, defaults to gate.DefaultGateTimeout (600s) when omitted;
// empty => the repo gate is OFF (no poller starts); GitHub-only for v1
// no such variable => the repo gate is OFF (no poller starts); GitHub-only for v1.
// Replaces CORRALAI_GATE_POLICIES, which is REFUSED: its ";" separator collided with
// commands containing ";"
// CORRALAI_GATE_DB repo gate dedupe/index store DuckDB path (default ~/.claude/corralai_gate.duckdb)
// CORRALAI_GATE_POLL_SECONDS how often (seconds) the repo gate polls covered repos for new PR heads (default 120)
// CORRALAI_GATE_EXEC_BACKEND / _EXEC_UNSAFE_HOST same jail backend used by the independent verify-gate (see below);
Expand Down
11 changes: 7 additions & 4 deletions cmd/corral/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,14 @@
// from CORRALAI_BRAIN_KEY above (that's an Ed25519 IDENTITY SEED, not a bearer token — do not reuse it)
// CORRALAI_REKOR_URL Sigstore Rekor instance report_build anchors signed build attestations to (default https://rekor.sigstore.dev);
// `corral certify verify` checks the same default unless --rekor-url overrides it
// CORRALAI_GATE_POLICIES repo merge gate: ";"-separated policies "repo=owner/name,base=main,net=false,timeout=600,cmd=go test ./...";
// cmd= MUST be the last field — everything after it is the command verbatim (commas
// allowed, never split) so "cmd=go test -run A,B ./..." isn't silently truncated;
// CORRALAI_GATE_POLICY_<NAME> repo merge gate: ONE policy per variable, "repo=owner/name,base=main,net=false,timeout=600,cmd=go test ./...";
// cmd= MUST be last — everything after it is the command VERBATIM to the end of the
// value: commas, semicolons, quotes and newlines included, so nothing can truncate it
// into a weaker command that exits 0 and posts a wrongful success;
// timeout= is seconds, defaults to gate.DefaultGateTimeout (600s) when omitted;
// empty => the repo gate is OFF (no poller starts); GitHub-only for v1
// no such variable => the repo gate is OFF (no poller starts); GitHub-only for v1.
// Replaces CORRALAI_GATE_POLICIES, which is REFUSED: its ";" separator collided with
// commands containing ";"
// CORRALAI_GATE_DB repo gate dedupe/index store DuckDB path (default ~/.claude/corralai_gate.duckdb)
// CORRALAI_GATE_POLL_SECONDS how often (seconds) the repo gate polls covered repos for new PR heads (default 120)
// CORRALAI_GATE_EXEC_BACKEND / _EXEC_UNSAFE_HOST same jail backend used by the independent verify-gate (see below);
Expand Down
11 changes: 7 additions & 4 deletions docs/cli/corral-wrangler.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,14 @@ CORRALAI_BRAIN_TOKEN `corral certify`'s bearer token to authenticate to a
from CORRALAI_BRAIN_KEY above (that's an Ed25519 IDENTITY SEED, not a bearer token — do not reuse it)
CORRALAI_REKOR_URL Sigstore Rekor instance report_build anchors signed build attestations to (default https://rekor.sigstore.dev);
`corral certify verify` checks the same default unless --rekor-url overrides it
CORRALAI_GATE_POLICIES repo merge gate: ";"-separated policies "repo=owner/name,base=main,net=false,timeout=600,cmd=go test ./...";
cmd= MUST be the last field — everything after it is the command verbatim (commas
allowed, never split) so "cmd=go test -run A,B ./..." isn't silently truncated;
CORRALAI_GATE_POLICY_<NAME> repo merge gate: ONE policy per variable, "repo=owner/name,base=main,net=false,timeout=600,cmd=go test ./...";
cmd= MUST be last — everything after it is the command VERBATIM to the end of the
value: commas, semicolons, quotes and newlines included, so nothing can truncate it
into a weaker command that exits 0 and posts a wrongful success;
timeout= is seconds, defaults to gate.DefaultGateTimeout (600s) when omitted;
empty => the repo gate is OFF (no poller starts); GitHub-only for v1
no such variable => the repo gate is OFF (no poller starts); GitHub-only for v1.
Replaces CORRALAI_GATE_POLICIES, which is REFUSED: its ";" separator collided with
commands containing ";"
CORRALAI_GATE_DB repo gate dedupe/index store DuckDB path (default ~/.claude/corralai_gate.duckdb)
CORRALAI_GATE_POLL_SECONDS how often (seconds) the repo gate polls covered repos for new PR heads (default 120)
CORRALAI_GATE_EXEC_BACKEND / _EXEC_UNSAFE_HOST same jail backend used by the independent verify-gate (see below);
Expand Down
11 changes: 7 additions & 4 deletions docs/cli/corral.md
Original file line number Diff line number Diff line change
Expand Up @@ -999,11 +999,14 @@ CORRALAI_BRAIN_TOKEN `corral certify`'s bearer token to authenticate to a
from CORRALAI_BRAIN_KEY above (that's an Ed25519 IDENTITY SEED, not a bearer token — do not reuse it)
CORRALAI_REKOR_URL Sigstore Rekor instance report_build anchors signed build attestations to (default https://rekor.sigstore.dev);
`corral certify verify` checks the same default unless --rekor-url overrides it
CORRALAI_GATE_POLICIES repo merge gate: ";"-separated policies "repo=owner/name,base=main,net=false,timeout=600,cmd=go test ./...";
cmd= MUST be the last field — everything after it is the command verbatim (commas
allowed, never split) so "cmd=go test -run A,B ./..." isn't silently truncated;
CORRALAI_GATE_POLICY_<NAME> repo merge gate: ONE policy per variable, "repo=owner/name,base=main,net=false,timeout=600,cmd=go test ./...";
cmd= MUST be last — everything after it is the command VERBATIM to the end of the
value: commas, semicolons, quotes and newlines included, so nothing can truncate it
into a weaker command that exits 0 and posts a wrongful success;
timeout= is seconds, defaults to gate.DefaultGateTimeout (600s) when omitted;
empty => the repo gate is OFF (no poller starts); GitHub-only for v1
no such variable => the repo gate is OFF (no poller starts); GitHub-only for v1.
Replaces CORRALAI_GATE_POLICIES, which is REFUSED: its ";" separator collided with
commands containing ";"
CORRALAI_GATE_DB repo gate dedupe/index store DuckDB path (default ~/.claude/corralai_gate.duckdb)
CORRALAI_GATE_POLL_SECONDS how often (seconds) the repo gate polls covered repos for new PR heads (default 120)
CORRALAI_GATE_EXEC_BACKEND / _EXEC_UNSAFE_HOST same jail backend used by the independent verify-gate (see below);
Expand Down
6 changes: 3 additions & 3 deletions internal/brain/gate.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (j jailAdapter) Run(ctx context.Context, command, workspace string, network
//
// opts.GatePolicies == nil/empty is the feature's OFF switch: StartGate is
// a complete no-op (nil, nil) — zero behavior change for a brain that
// doesn't set CORRALAI_GATE_POLICIES.
// sets no CORRALAI_GATE_POLICY_<NAME>.
//
// opts.GateBackend == nil DISABLES gating even when policies ARE
// configured: this is the fail-closed contract carried up from
Expand All @@ -83,11 +83,11 @@ func StartGate(ctx context.Context, opts Options) (*gate.Store, error) {
return nil, nil
}
if opts.GateBackend == nil {
log.Printf("gate: DISABLED — CORRALAI_GATE_POLICIES is set (%d polic(ies)) but no sandbox isolation backend is available; refusing to run PR checks unsandboxed (set CORRALAI_GATE_EXEC_BACKEND)", len(opts.GatePolicies))
log.Printf("gate: DISABLED — %d gate polic(ies) are configured but no sandbox isolation backend is available; refusing to run PR checks unsandboxed (set CORRALAI_GATE_EXEC_BACKEND)", len(opts.GatePolicies))
return nil, nil
}
if opts.Repo == nil {
log.Printf("gate: DISABLED — CORRALAI_GATE_POLICIES is set but no repo.Engine is configured (Options.Repo is nil)")
log.Printf("gate: DISABLED — a gate policy is configured but no repo.Engine is configured (Options.Repo is nil)")
return nil, nil
}

Expand Down
4 changes: 2 additions & 2 deletions internal/brain/gate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func TestGateRunHandlerUnknownSHAIs404(t *testing.T) {
// startup failure).
func TestStartGateNilBackendDisablesGating(t *testing.T) {
store, err := StartGate(context.Background(), Options{
GatePolicies: []gate.Policy{{Repo: "o/r", CheckCmd: []string{"true"}}},
GatePolicies: []gate.Policy{{Repo: "o/r", CheckCmd: "true"}},
GateBackend: nil,
GateDB: filepath.Join(t.TempDir(), "gate.db"),
})
Expand All @@ -198,7 +198,7 @@ func TestStartGateNilBackendDisablesGating(t *testing.T) {
}
}

// TestStartGateEmptyPoliciesIsOff: no CORRALAI_GATE_POLICIES configured
// TestStartGateEmptyPoliciesIsOff: no CORRALAI_GATE_POLICY_<NAME> configured
// means the feature is off — StartGate must be a complete no-op.
func TestStartGateEmptyPoliciesIsOff(t *testing.T) {
store, err := StartGate(context.Background(), Options{})
Expand Down
2 changes: 1 addition & 1 deletion internal/brain/identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ type Options struct {
Witness transparency.Witness

// GatePolicies declares which repos/branches the merge gate (repo-gate
// control plane) covers — parsed from CORRALAI_GATE_POLICIES. Empty/nil
// control plane) covers — parsed from CORRALAI_GATE_POLICY_<NAME>. Empty/nil
// is the feature's off switch: StartGate is then a complete no-op and
// no poller ever starts (zero behavior change for existing brains).
GatePolicies []gate.Policy
Expand Down
92 changes: 2 additions & 90 deletions internal/gate/coldreview3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"database/sql"
"path/filepath"
"slices"
"strings"
"testing"
"time"
)
Expand All @@ -22,93 +21,6 @@ import (
// where it supplied them, because a test written from my paraphrase of a
// finding is a test written by the person who missed it.

// TestSemicolonGuardIsNotDefeatedByAnEqualsSign is round three's R1.
//
// MY BUG: the round-two guard asked `!strings.Contains(entry, "=")` to decide
// whether a fragment was a continuation of a truncated command. Any ordinary
// second command defeats that — "go test -tags=integration ./..." contains an
// '=' — so the fragment read as a policy entry, the truncation went unnoticed
// again, and the weaker command was accepted. I guarded a real case and left
// the common one open.
//
// The rule is now keyed on repo=, the field a policy REQUIRES.
func TestSemicolonGuardIsNotDefeatedByAnEqualsSign(t *testing.T) {
for _, tail := range []string{
"go test -tags=integration ./...", // the reviewer's own input
"GOFLAGS=-mod=mod go test ./...",
"make check VAR=1",
"go test ./...", // no '=' at all: the case round two did catch
} {
t.Run(tail, func(t *testing.T) {
pol, bad := ParsePolicies("repo=o/r,cmd=go vet ./... ; " + tail)
if len(pol) != 0 {
t.Errorf("accepted %d policy/policies with command %q — the operator wrote two steps and this gate runs one, then posts success",
len(pol), strings.Join(pol[0].CheckCmd, " "))
}
if len(bad) == 0 {
t.Fatal("the truncation was silent")
}
})
}
}

// TestTwoRealEntriesSharingARepoStillParse is the control for R1's fix. The
// guard drops the PRECEDING policy when it sees a continuation, and it used to
// identify that policy with `strings.Contains(frags[i-1], policies[n-1].Repo)`
// — another guess, which misfires when two entries share a repo. It is keyed
// on the fragment's index now, and legitimate multi-entry values must survive.
func TestTwoRealEntriesSharingARepoStillParse(t *testing.T) {
pol, bad := ParsePolicies("repo=o/r,context=corral/lint,cmd=golangci-lint run;repo=o/r,context=corral/test,cmd=go test ./...")
if len(pol) != 2 {
t.Fatalf("policies = %d, want 2 (bad=%v) — two contexts on one repo is the documented multi-policy shape", len(pol), bad)
}
if pol[0].Context == pol[1].Context {
t.Errorf("both policies came back with context %q", pol[0].Context)
}
if got := strings.Join(pol[1].CheckCmd, " "); got != "go test ./..." {
t.Errorf("second command = %q, want it intact", got)
}
}

// TestStrayFieldAfterCmdToleratesWhitespace is round three's R2.
//
// MY BUG: strayFieldAfterCmd matched the single spelling ","+f+"=" exactly. An
// operator writing the spaced form — "cmd=make test, base=release", which is at
// least as natural — slipped through, the field was absorbed into the command,
// and Base stayed nil: a policy gating EVERY base branch rather than the one
// named. One spelling guarded, the other left open, in the fix for a finding
// about exactly that.
func TestStrayFieldAfterCmdToleratesWhitespace(t *testing.T) {
for _, entry := range []string{
"repo=o/r,cmd=make test, base=release", // the reviewer's own input
"repo=o/r,cmd=make test ,base=release",
"repo=o/r,cmd=make test , base = release",
"repo=o/r,cmd=make test,\tcontext=corral/other",
"repo=o/r,cmd=make test, timeout=30",
} {
t.Run(entry, func(t *testing.T) {
pol, bad := ParsePolicies(entry)
if len(pol) != 0 {
t.Errorf("accepted a policy whose Base is %v and command is %q — a WIDER policy than was written, silently",
pol[0].Base, strings.Join(pol[0].CheckCmd, " "))
}
if len(bad) == 0 {
t.Fatal("a policy field after cmd= was swallowed with nothing reported")
}
})
}
}

// TestACommandContainingACommaEqualsPairStillParses is the control: the fix
// must not start refusing commands that legitimately contain ",<word>=" where
// the word is not a policy field.
func TestACommandContainingACommaEqualsPairStillParses(t *testing.T) {
pol, bad := ParsePolicies("repo=o/r,cmd=go test -ldflags=-X main.v=1,other=2 ./...")
if len(pol) != 1 {
t.Fatalf("policies = %d, want 1 (bad=%v) — 'other' is not a policy field and must not trip the guard", len(pol), bad)
}
}

// TestLegacyKeyIsWidenedNotLeftAlone is round three's R3, and the one I am
// least comfortable about, because I shipped it on the strength of a sentence
// I wrote asserting it was safe.
Expand Down Expand Up @@ -212,7 +124,7 @@ func TestRunnerPostsAndRecordsTheSameContext(t *testing.T) {
}
// A Policy with NO Context, as brain Options.GatePolicies may build it.
if err := r.Run(context.Background(), "http://forge/o/r",
Policy{Repo: "o/r", CheckCmd: []string{"true"}}, PRRef{Number: 1, HeadSHA: "abc"}); err != nil {
Policy{Repo: "o/r", CheckCmd: "true"}, PRRef{Number: 1, HeadSHA: "abc"}); err != nil {
t.Fatal(err)
}

Expand Down Expand Up @@ -273,7 +185,7 @@ func TestTheRunnerActuallyUsesTheBoundedTimeout(t *testing.T) {
Now: func() time.Time { return time.Unix(0, 0) },
}
_ = r.Run(context.Background(), "http://forge/o/r",
Policy{Repo: "o/r", Context: "corral/gate", CheckCmd: []string{"true"}, TimeoutS: 9223372036854775807},
Policy{Repo: "o/r", Context: "corral/gate", CheckCmd: "true", TimeoutS: 9223372036854775807},
PRRef{Number: 1, HeadSHA: "abc"})

if jail.lastTimeout <= 0 {
Expand Down
Loading
Loading