From 4f131cd58b451176b58872d942f4b60ad53362b9 Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 28 Aug 2026 16:22:45 +0800 Subject: [PATCH 1/2] Fix Docker Sandboxes empty policy attribution Accept Docker Sandboxes v0.38 blank relay-rule telemetry only after an authenticated pre-rule proof, exact scoped policy delta, and stable inventory fingerprint. Preserve fail-closed handling for missing, wrong, stale, blocked, or drifted evidence. --- .../provider/dockersandboxes/egress_relay.go | 281 +++++++++++++++--- .../dockersandboxes/egress_relay_test.go | 155 +++++++++- .../dockersandboxes/network_policy.go | 2 +- 3 files changed, 402 insertions(+), 36 deletions(-) diff --git a/internal/provider/dockersandboxes/egress_relay.go b/internal/provider/dockersandboxes/egress_relay.go index 0fb775c..97b1121 100644 --- a/internal/provider/dockersandboxes/egress_relay.go +++ b/internal/provider/dockersandboxes/egress_relay.go @@ -8,12 +8,14 @@ import ( "crypto/subtle" "encoding/base64" "encoding/binary" + "encoding/hex" "encoding/json" "errors" "fmt" "io" "net" "net/netip" + "sort" "strconv" "strings" "sync" @@ -49,21 +51,41 @@ type guestRelayConfiguration struct { } type relayTokenBinding struct { - ProviderID string - Token string - Epoch uint64 - PolicyRules map[string]struct{} + ProviderID string + Token string + Epoch uint64 + PolicyRules map[string]struct{} + PolicyInventoryDigest string + PolicyPreAllowProof string } type relayBindingSnapshot struct { - Instance provider.Instance - Token string - Epoch uint64 - Relay *egressRelay - Port int - PolicyRules map[string]struct{} + Instance provider.Instance + Token string + Epoch uint64 + Relay *egressRelay + Port int + PolicyRules map[string]struct{} + PolicyInventoryDigest string + PolicyPreAllowProof string } +type relayPolicyProof struct { + RuleNames []string + InventoryDigest string + PreAllowProof string +} + +type relayPolicyPrecondition struct { + InventoryDigest string + Proof string +} + +const ( + relayPolicyPreAllowBlocked = "blocked" + relayPolicyPreAllowAuthenticatedOpen = "authenticated-open" +) + const hostTrustRelayVerificationScript = `set -euo pipefail test -f /run/epar/egress-relay-active test ! -L /run/epar/egress-relay-active @@ -72,6 +94,10 @@ test "$(curl --silent --show-error --output /dev/null --write-out '%{http_code}' registry_status="$(curl --silent --show-error --output /dev/null --write-out '%{http_code}' --connect-timeout 2 --max-time 5 --proxy http://127.0.0.1:3129 --noproxy '' --cacert /usr/local/share/ca-certificates/epar/epar-egress-relay.crt https://registry-1.docker.io/v2/)" test "${registry_status}" = "401"` +func hostTrustRelayBlockedProbeScript(port int) string { + return fmt.Sprintf(`output="$(timeout 3 bash -c 'IFS= read -r token; response=""; if exec 3<>/dev/tcp/host.docker.internal/%d 2>/dev/null; then printf "EPAR1 %%s PING\n" "${token}" >&3; IFS= read -r response <&3 || true; fi; printf "%%s" "${response}"' || true)"; printf '%%s' "${output}"`, port) +} + func (p *Provider) ActivateHostTrustRuntime(ctx context.Context, instance provider.Instance) (activationErr error) { if !p.hostTrustRelayEnabled { if p.logger != nil { @@ -126,16 +152,20 @@ func (p *Provider) ActivateHostTrustRuntime(ctx context.Context, instance provid }() resource := net.JoinHostPort("host.docker.internal", strconv.Itoa(relay.port)) - var policyRuleNames []string - addedPolicyRules, policyRuleNames, err = p.applyHostTrustRelayPolicy(ctx, instance, provider.NetworkPolicyRule{ + precondition, err := p.verifyHostTrustRelayBeforeAllow(ctx, binding) + if err != nil { + return err + } + var policyProof relayPolicyProof + addedPolicyRules, policyProof, err = p.applyHostTrustRelayPolicy(ctx, instance, provider.NetworkPolicyRule{ Name: "epar-host-trust-relay", Decision: provider.NetworkPolicyAllow, Resources: []string{resource}, - }) + }, precondition) if err != nil { return fmt.Errorf("allow exact Docker Sandboxes host-trust relay endpoint: %w", err) } - binding, err = p.bindRelayPolicyRules(binding, policyRuleNames) + binding, err = p.bindRelayPolicyProof(binding, policyProof) if err != nil { return err } @@ -244,10 +274,13 @@ func (p *Provider) finalizeGuestRelay(ctx context.Context, instance provider.Ins return err } -func (p *Provider) applyHostTrustRelayPolicy(ctx context.Context, instance provider.Instance, rule provider.NetworkPolicyRule) ([]provider.NetworkPolicyRule, []string, error) { +func (p *Provider) applyHostTrustRelayPolicy(ctx context.Context, instance provider.Instance, rule provider.NetworkPolicyRule, precondition relayPolicyPrecondition) ([]provider.NetworkPolicyRule, relayPolicyProof, error) { before, err := p.ReadNetworkPolicy(ctx, instance) if err != nil { - return nil, nil, err + return nil, relayPolicyProof{}, err + } + if relayPolicyInventoryDigest(before) != precondition.InventoryDigest { + return nil, relayPolicyProof{}, fmt.Errorf("Docker Sandboxes policy changed between blocked relay proof and exact allow application") } beforeIDs := make(map[string]struct{}, len(before)) for _, existing := range before { @@ -258,7 +291,7 @@ func (p *Provider) applyHostTrustRelayPolicy(ctx context.Context, instance provi after, readErr := p.ReadNetworkPolicy(readbackCtx, instance) cancel() if readErr != nil { - return nil, nil, errors.Join(applyErr, fmt.Errorf("read back relay policy delta: %w", readErr)) + return nil, relayPolicyProof{}, errors.Join(applyErr, fmt.Errorf("read back relay policy delta: %w", readErr)) } added := make([]provider.NetworkPolicyRule, 0, 1) policyRuleNames := make([]string, 0, 1) @@ -272,9 +305,20 @@ func (p *Provider) applyHostTrustRelayPolicy(ctx context.Context, instance provi added = append(added, candidate) } if len(policyRuleNames) == 0 { - return added, nil, errors.Join(applyErr, fmt.Errorf("Docker Sandboxes policy readback did not identify the exact active relay allow rule")) + return added, relayPolicyProof{}, errors.Join(applyErr, fmt.Errorf("Docker Sandboxes policy readback did not identify the exact active relay allow rule")) } - return added, policyRuleNames, applyErr + if len(added) != 1 || len(policyRuleNames) != 1 { + return added, relayPolicyProof{}, errors.Join(applyErr, fmt.Errorf("Docker Sandboxes policy readback did not identify one newly added exact relay allow rule")) + } + expectedAfter := append(append([]provider.NetworkPolicyRule(nil), before...), added[0]) + if relayPolicyInventoryDigest(expectedAfter) != relayPolicyInventoryDigest(after) { + return added, relayPolicyProof{}, errors.Join(applyErr, fmt.Errorf("Docker Sandboxes policy changed outside the exact relay allow application")) + } + return added, relayPolicyProof{ + RuleNames: policyRuleNames, + InventoryDigest: relayPolicyInventoryDigest(after), + PreAllowProof: precondition.Proof, + }, applyErr } func hostTrustRelayPolicyProbeStart() time.Time { @@ -283,29 +327,114 @@ func hostTrustRelayPolicyProbeStart() time.Time { return time.Now().UTC().Truncate(time.Second) } -func (p *Provider) verifyBoundHostTrustRelayPolicy(ctx context.Context, binding relayBindingSnapshot, startedAt time.Time) error { - if binding.Port <= 0 || len(binding.PolicyRules) == 0 { - return fmt.Errorf("Docker Sandboxes host-trust relay policy proof is not bound to the exact instance") +func (p *Provider) verifyHostTrustRelayBeforeAllow(ctx context.Context, binding relayBindingSnapshot) (relayPolicyPrecondition, error) { + if binding.Port <= 0 { + return relayPolicyPrecondition{}, fmt.Errorf("Docker Sandboxes host-trust relay precondition is not bound to the exact instance") + } + if err := p.verifyRelayBinding(binding); err != nil { + return relayPolicyPrecondition{}, err + } + rules, err := p.ReadNetworkPolicy(ctx, binding.Instance) + if err != nil { + return relayPolicyPrecondition{}, fmt.Errorf("read Docker Sandboxes policy before relay proof: %w", err) + } + resource := net.JoinHostPort("host.docker.internal", strconv.Itoa(binding.Port)) + for _, rule := range rules { + if rule.Active && rule.Decision == provider.NetworkPolicyAllow && rule.ResourceType == "network" && isSandboxPolicyTarget(rule.Scope, rule.AppliesTo, binding.Instance.Name) { + for _, candidate := range rule.Resources { + if candidate == resource { + return relayPolicyPrecondition{}, fmt.Errorf("Docker Sandboxes host-trust relay endpoint already had an exact allow rule before activation") + } + } + } + } + inventoryDigest := relayPolicyInventoryDigest(rules) + startedAt := hostTrustRelayPolicyProbeStart() + probeCtx, cancelProbe := context.WithTimeout(ctx, guestRelayProbeTimeout) + result, probeErr := p.Exec(probeCtx, binding.Instance, provider.ShellCommand(hostTrustRelayBlockedProbeScript(binding.Port)), provider.ExecOptions{ + Stdin: binding.Token + "\n", + SensitiveValues: []string{binding.Token}, + SuppressTranscript: true, + }) + cancelProbe() + if probeErr != nil { + return relayPolicyPrecondition{}, fmt.Errorf("probe Docker Sandboxes host-trust relay before exact allow: %w", probeErr) + } + document, err := p.readHostTrustRelayPolicyLog(ctx, binding.Instance) + if err != nil { + return relayPolicyPrecondition{}, err + } + relayHosts := hostTrustRelayPolicyHosts(binding.Port) + foundBlocked := false + foundAllowed := false + for _, record := range document.Allowed { + if record.VMName != binding.Instance.Name || record.LastSeen.Before(startedAt) { + continue + } + if _, exactRelay := relayHosts[record.Host]; !exactRelay { + continue + } + if record.ProxyType != "transparent" || record.Count <= 0 || record.Rule == nil { + return relayPolicyPrecondition{}, fmt.Errorf("Docker Sandboxes returned invalid allowed evidence for the pre-rule relay proof") + } + foundAllowed = true + } + for _, record := range document.Blocked { + if record.VMName != binding.Instance.Name || record.LastSeen.Before(startedAt) || record.Count <= 0 { + continue + } + if _, exactRelay := relayHosts[record.Host]; exactRelay { + foundBlocked = true + } + } + probeOutput := strings.TrimSpace(result.Stdout) + switch { + case probeOutput == "PONG" && foundAllowed && !foundBlocked: + return relayPolicyPrecondition{InventoryDigest: inventoryDigest, Proof: relayPolicyPreAllowAuthenticatedOpen}, nil + case probeOutput == "" && foundBlocked && !foundAllowed: + return relayPolicyPrecondition{InventoryDigest: inventoryDigest, Proof: relayPolicyPreAllowBlocked}, nil + case probeOutput != "" && probeOutput != "PONG": + return relayPolicyPrecondition{}, fmt.Errorf("Docker Sandboxes host-trust relay returned an unexpected pre-rule authenticated response") + default: + return relayPolicyPrecondition{}, fmt.Errorf("Docker Sandboxes policy log did not confirm a consistent pre-rule relay result") } +} + +func (p *Provider) readHostTrustRelayPolicyLog(ctx context.Context, instance provider.Instance) (policyLogDocument, error) { result, err := p.run(ctx, commandRequest{ - args: []string{"policy", "log", binding.Instance.Name, "--json"}, + args: []string{"policy", "log", instance.Name, "--json"}, operation: "verify Docker Sandboxes host-trust relay route", outputLimit: diagnosticOutputLimit, timeout: providerReadbackTimeout, }) if err != nil { - return err + return policyLogDocument{}, err } decoder := json.NewDecoder(strings.NewReader(result.Stdout)) decoder.DisallowUnknownFields() var document policyLogDocument if err := decoder.Decode(&document); err != nil || requireJSONEOF(decoder) != nil { - return fmt.Errorf("Docker Sandboxes policy log returned an unsupported json schema") + return policyLogDocument{}, fmt.Errorf("Docker Sandboxes policy log returned an unsupported json schema") + } + return document, nil +} + +func hostTrustRelayPolicyHosts(port int) map[string]struct{} { + return map[string]struct{}{ + net.JoinHostPort("localhost", strconv.Itoa(port)): {}, + net.JoinHostPort("host.docker.internal", strconv.Itoa(port)): {}, } - relayHosts := map[string]struct{}{ - net.JoinHostPort("localhost", strconv.Itoa(binding.Port)): {}, - net.JoinHostPort("host.docker.internal", strconv.Itoa(binding.Port)): {}, +} + +func (p *Provider) verifyBoundHostTrustRelayPolicy(ctx context.Context, binding relayBindingSnapshot, startedAt time.Time) error { + if binding.Port <= 0 || len(binding.PolicyRules) == 0 { + return fmt.Errorf("Docker Sandboxes host-trust relay policy proof is not bound to the exact instance") } + document, err := p.readHostTrustRelayPolicyLog(ctx, binding.Instance) + if err != nil { + return err + } + relayHosts := hostTrustRelayPolicyHosts(binding.Port) for _, record := range document.Blocked { if record.VMName != binding.Instance.Name || record.LastSeen.Before(startedAt) { continue @@ -315,6 +444,7 @@ func (p *Provider) verifyBoundHostTrustRelayPolicy(ctx context.Context, binding } } foundTransparentRelay := false + foundUnattributedRelay := false for _, record := range document.Allowed { if record.VMName != binding.Instance.Name || record.LastSeen.Before(startedAt) { continue @@ -328,17 +458,91 @@ func (p *Provider) verifyBoundHostTrustRelayPolicy(ctx context.Context, binding if record.ProxyType != "transparent" { return fmt.Errorf("Docker Sandboxes host-trust relay used unexpected %q routing", record.ProxyType) } - if _, expectedRule := binding.PolicyRules[record.Rule]; !expectedRule { - return fmt.Errorf("Docker Sandboxes host-trust relay matched unexpected policy rule %q", record.Rule) + if record.Count <= 0 { + return fmt.Errorf("Docker Sandboxes host-trust relay policy record had an invalid request count") + } + if record.Rule == nil { + return fmt.Errorf("Docker Sandboxes host-trust relay policy record omitted the matched rule identity") + } + if *record.Rule == "" { + foundUnattributedRelay = true + } else if _, expectedRule := binding.PolicyRules[*record.Rule]; !expectedRule { + return fmt.Errorf("Docker Sandboxes host-trust relay matched unexpected policy rule %q", *record.Rule) } foundTransparentRelay = true } if !foundTransparentRelay { return fmt.Errorf("Docker Sandboxes policy log did not confirm fresh transparent routing for the exact EPAR host-trust relay endpoint and bound policy rule") } + if foundUnattributedRelay { + if err := p.verifyUnattributedRelayPolicyProof(ctx, binding); err != nil { + return err + } + } + return nil +} + +func (p *Provider) verifyUnattributedRelayPolicyProof(ctx context.Context, binding relayBindingSnapshot) error { + if binding.PolicyInventoryDigest == "" || binding.PolicyPreAllowProof != relayPolicyPreAllowBlocked && binding.PolicyPreAllowProof != relayPolicyPreAllowAuthenticatedOpen { + return fmt.Errorf("Docker Sandboxes omitted the matched relay rule identity without a bound pre-rule policy proof") + } + if err := p.verifyRelayBinding(binding); err != nil { + return err + } + rules, err := p.ReadNetworkPolicy(ctx, binding.Instance) + if err != nil { + return fmt.Errorf("read Docker Sandboxes policy for unattributed relay proof: %w", err) + } + if relayPolicyInventoryDigest(rules) != binding.PolicyInventoryDigest { + return fmt.Errorf("Docker Sandboxes policy changed after the unattributed relay proof was bound") + } + resource := net.JoinHostPort("host.docker.internal", strconv.Itoa(binding.Port)) + matched := 0 + for _, rule := range rules { + if !rule.Active || rule.Decision != provider.NetworkPolicyAllow || rule.ResourceType != "network" || len(rule.Resources) != 1 || rule.Resources[0] != resource || !isRemovableSandboxPolicyRule(rule, binding.Instance.Name) { + continue + } + if _, expected := binding.PolicyRules[rule.Name]; expected { + matched++ + } + } + if matched != 1 { + return fmt.Errorf("Docker Sandboxes unattributed relay proof no longer has one exact bound allow rule") + } return nil } +func relayPolicyInventoryDigest(rules []provider.NetworkPolicyRule) string { + type digestRule struct { + ID string + Name string + PolicyID string + Scope string + AppliesTo string + ResourceType string + Resources []string + Decision provider.NetworkPolicyDecision + Origin string + Status string + Editable bool + Active bool + } + normalized := make([]digestRule, 0, len(rules)) + for _, rule := range rules { + resources := append([]string(nil), rule.Resources...) + sort.Strings(resources) + normalized = append(normalized, digestRule{ + ID: rule.ID, Name: rule.Name, PolicyID: rule.PolicyID, Scope: rule.Scope, AppliesTo: rule.AppliesTo, + ResourceType: rule.ResourceType, Resources: resources, Decision: rule.Decision, Origin: rule.Origin, + Status: rule.Status, Editable: rule.Editable, Active: rule.Active, + }) + } + sort.Slice(normalized, func(i, j int) bool { return normalized[i].ID < normalized[j].ID }) + payload, _ := json.Marshal(normalized) + digest := sha256.Sum256(payload) + return hex.EncodeToString(digest[:]) +} + func (p *Provider) ensureRelayToken(instance provider.Instance) (relayBindingSnapshot, error) { if err := validateInstance(instance, true); err != nil { return relayBindingSnapshot{}, err @@ -398,7 +602,10 @@ func relayBindingSnapshotLocked(instance provider.Instance, binding relayTokenBi if relay != nil { port = relay.port } - return relayBindingSnapshot{Instance: instance, Token: binding.Token, Epoch: binding.Epoch, Relay: relay, Port: port, PolicyRules: policyRules} + return relayBindingSnapshot{ + Instance: instance, Token: binding.Token, Epoch: binding.Epoch, Relay: relay, Port: port, PolicyRules: policyRules, + PolicyInventoryDigest: binding.PolicyInventoryDigest, PolicyPreAllowProof: binding.PolicyPreAllowProof, + } } func (p *Provider) currentRelayBinding(instance provider.Instance) (relayBindingSnapshot, error) { @@ -412,8 +619,12 @@ func (p *Provider) currentRelayBinding(instance provider.Instance) (relayBinding } func (p *Provider) bindRelayPolicyRules(snapshot relayBindingSnapshot, ruleNames []string) (relayBindingSnapshot, error) { - policyRules := make(map[string]struct{}, len(ruleNames)) - for _, ruleName := range ruleNames { + return p.bindRelayPolicyProof(snapshot, relayPolicyProof{RuleNames: ruleNames}) +} + +func (p *Provider) bindRelayPolicyProof(snapshot relayBindingSnapshot, proof relayPolicyProof) (relayBindingSnapshot, error) { + policyRules := make(map[string]struct{}, len(proof.RuleNames)) + for _, ruleName := range proof.RuleNames { if ruleName == "" { return relayBindingSnapshot{}, fmt.Errorf("Docker Sandboxes host-trust relay policy readback omitted the matched rule identity") } @@ -429,6 +640,8 @@ func (p *Provider) bindRelayPolicyRules(snapshot relayBindingSnapshot, ruleNames return relayBindingSnapshot{}, fmt.Errorf("Docker Sandboxes host-trust relay binding changed during activation") } binding.PolicyRules = policyRules + binding.PolicyInventoryDigest = proof.InventoryDigest + binding.PolicyPreAllowProof = proof.PreAllowProof p.relayTokens[snapshot.Instance.Name] = binding return relayBindingSnapshotLocked(snapshot.Instance, binding, p.relay), nil } @@ -444,7 +657,7 @@ func (p *Provider) verifyRelayBinding(snapshot relayBindingSnapshot) error { } func relayBindingMatchesSnapshot(binding relayTokenBinding, relay *egressRelay, snapshot relayBindingSnapshot) bool { - return relay != nil && relay == snapshot.Relay && relay.port == snapshot.Port && binding.ProviderID == snapshot.Instance.ProviderID && binding.Token == snapshot.Token && binding.Epoch == snapshot.Epoch + return relay != nil && relay == snapshot.Relay && relay.port == snapshot.Port && binding.ProviderID == snapshot.Instance.ProviderID && binding.Token == snapshot.Token && binding.Epoch == snapshot.Epoch && binding.PolicyInventoryDigest == snapshot.PolicyInventoryDigest && binding.PolicyPreAllowProof == snapshot.PolicyPreAllowProof } func (p *Provider) verifyExactRelayInstance(ctx context.Context, snapshot relayBindingSnapshot) error { diff --git a/internal/provider/dockersandboxes/egress_relay_test.go b/internal/provider/dockersandboxes/egress_relay_test.go index 82369da..83d75ef 100644 --- a/internal/provider/dockersandboxes/egress_relay_test.go +++ b/internal/provider/dockersandboxes/egress_relay_test.go @@ -149,6 +149,12 @@ func TestHostTrustRelayActivationFailureRollsBackExactAddedPolicy(t *testing.T) case strings.HasPrefix(args, "policy allow network --sandbox "+testName+" host.docker.internal:"): rulePresent = true return provider.ExecResult{}, nil + case strings.HasPrefix(args, "exec -i "+testName+" -- bash -lc ") && strings.Contains(args, "/dev/tcp/host.docker.internal/"): + return provider.ExecResult{}, nil + case args == "policy log "+testName+" --json" && !rulePresent: + now := time.Now().UTC() + entry := policyLogEntry(net.JoinHostPort("localhost", fmt.Sprint(p.hostTrustRelayPort)), testName, "transparent", now) + return provider.ExecResult{Stdout: fmt.Sprintf(`{"blocked_hosts":[%s],"allowed_hosts":[]}`, entry)}, nil case args == "policy rm network --sandbox "+testName+" --id "+ruleID: rulePresent = false removed = true @@ -212,14 +218,29 @@ func TestHostTrustRelayActivationCommitsOnlyAfterFreshPolicyProof(t *testing.T) case args == "policy allow network --sandbox "+testName+" "+resource: rulePresent = true return provider.ExecResult{}, nil + case strings.HasPrefix(args, "exec -i "+testName+" -- bash -lc ") && strings.Contains(args, "/dev/tcp/host.docker.internal/"): + stdin, readErr := io.ReadAll(request.stdin) + if readErr != nil || len(request.sensitiveValues) != 1 || string(stdin) != request.sensitiveValues[0]+"\n" { + t.Fatal("authenticated pre-rule probe did not pass its token only through redacted stdin") + } + if strings.Contains(args, request.sensitiveValues[0]) { + t.Fatal("authenticated pre-rule probe exposed its token in the command") + } + return provider.ExecResult{Stdout: "PONG"}, nil case strings.HasPrefix(args, "exec -i "+testName+" -- bash -lc ") && strings.Contains(args, "/opt/epar/configure-egress-relay.sh --commit"): committed = true return provider.ExecResult{}, nil case strings.HasPrefix(args, "exec -i "+testName+" -- bash -lc ") && strings.Contains(args, "/opt/epar/configure-egress-relay.sh"): return provider.ExecResult{}, nil + case strings.HasPrefix(args, "exec -i "+testName+" -- bash -lc "): + return provider.ExecResult{}, nil case args == "policy log "+testName+" --json": now := time.Now().UTC() - return provider.ExecResult{Stdout: fmt.Sprintf(`{"blocked_hosts":[],"allowed_hosts":[%s]}`, policyLogEntry(net.JoinHostPort("localhost", fmt.Sprint(p.hostTrustRelayPort)), testName, "transparent", now))}, nil + entry := policyLogEntryWithRule(net.JoinHostPort("localhost", fmt.Sprint(p.hostTrustRelayPort)), testName, "transparent", "", now) + if !rulePresent { + return provider.ExecResult{Stdout: fmt.Sprintf(`{"blocked_hosts":[],"allowed_hosts":[%s]}`, entry)}, nil + } + return provider.ExecResult{Stdout: fmt.Sprintf(`{"blocked_hosts":[],"allowed_hosts":[%s]}`, entry)}, nil default: t.Fatalf("unexpected command: %v", request.args) return provider.ExecResult{}, nil @@ -230,6 +251,9 @@ func TestHostTrustRelayActivationCommitsOnlyAfterFreshPolicyProof(t *testing.T) t.Fatal(err) } defer p.releaseRelayTokenForInstance(testInstance) + if err := p.VerifyHostTrustRuntime(context.Background(), testInstance); err != nil { + t.Fatalf("maintenance verification after unattributed activation: %v", err) + } if !committed || !rulePresent || len(p.relayTokens) != 1 || p.relay == nil { t.Fatalf("committed activation state = commit %t policy %t tokens %d relay %v", committed, rulePresent, len(p.relayTokens), p.relay) } @@ -238,6 +262,82 @@ func TestHostTrustRelayActivationCommitsOnlyAfterFreshPolicyProof(t *testing.T) } } +func TestHostTrustRelayBeforeAllowRequiresConsistentEvidence(t *testing.T) { + for _, test := range []struct { + name string + probeOutput string + policyLog func(string, int) string + wantErr string + }{ + { + name: "accepted blocked transition", + policyLog: func(name string, port int) string { + entry := policyLogEntry(net.JoinHostPort("localhost", fmt.Sprint(port)), name, "transparent", time.Now().UTC()) + return fmt.Sprintf(`{"blocked_hosts":[%s],"allowed_hosts":[]}`, entry) + }, + }, + { + name: "accepted authenticated open baseline", + probeOutput: "PONG", + policyLog: func(name string, port int) string { + entry := policyLogEntryWithRule(net.JoinHostPort("localhost", fmt.Sprint(port)), name, "transparent", "", time.Now().UTC()) + return fmt.Sprintf(`{"blocked_hosts":[],"allowed_hosts":[%s]}`, entry) + }, + }, + {name: "unexpected relay response", probeOutput: "connected", policyLog: func(string, int) string { return `{"blocked_hosts":[],"allowed_hosts":[]}` }, wantErr: "unexpected pre-rule authenticated response"}, + {name: "missing blocked record", policyLog: func(string, int) string { return `{"blocked_hosts":[],"allowed_hosts":[]}` }, wantErr: "did not confirm"}, + { + name: "endpoint already allowed", + policyLog: func(name string, port int) string { + entry := policyLogEntry(net.JoinHostPort("localhost", fmt.Sprint(port)), name, "transparent", time.Now().UTC()) + return fmt.Sprintf(`{"blocked_hosts":[],"allowed_hosts":[%s]}`, entry) + }, + wantErr: "consistent pre-rule relay result", + }, + { + name: "stale blocked record", + policyLog: func(name string, port int) string { + entry := policyLogEntry(net.JoinHostPort("localhost", fmt.Sprint(port)), name, "transparent", time.Now().UTC().Add(-time.Minute)) + return fmt.Sprintf(`{"blocked_hosts":[%s],"allowed_hosts":[]}`, entry) + }, + wantErr: "did not confirm", + }, + } { + t.Run(test.name, func(t *testing.T) { + p := NewWithDryRun("sbx", false) + p.ConfigureHostTrustRelay(true, "blocked-proof-"+test.name) + binding, err := p.ensureRelayToken(testInstance) + if err != nil { + t.Fatal(err) + } + defer p.releaseRelayToken(binding) + p.runCommand = func(_ context.Context, request commandRequest) (provider.ExecResult, error) { + args := strings.Join(request.args, " ") + switch { + case args == "ls --json": + return provider.ExecResult{Stdout: readyListJSON}, nil + case args == "policy ls "+testName+" --include-inactive --json": + return provider.ExecResult{Stdout: policyFixture("[]")}, nil + case strings.HasPrefix(args, "exec -i "+testName+" -- bash -lc ") && strings.Contains(args, "/dev/tcp/host.docker.internal/"): + return provider.ExecResult{Stdout: test.probeOutput}, nil + case args == "policy log "+testName+" --json" && test.policyLog != nil: + return provider.ExecResult{Stdout: test.policyLog(testName, binding.Port)}, nil + default: + t.Fatalf("unexpected command: %v", request.args) + return provider.ExecResult{}, nil + } + } + _, err = p.verifyHostTrustRelayBeforeAllow(context.Background(), binding) + if test.wantErr == "" && err != nil { + t.Fatal(err) + } + if test.wantErr != "" && (err == nil || !strings.Contains(err.Error(), test.wantErr)) { + t.Fatalf("error = %v, want substring %q", err, test.wantErr) + } + }) + } +} + func TestHostTrustRelayDebugDiagnosticsCanBeEnabled(t *testing.T) { p := NewWithDryRun("sbx", false) var logOutput bytes.Buffer @@ -327,6 +427,46 @@ func TestHostTrustRelayVerificationFailsClosedWithoutExactControllerBinding(t *t } } +func TestUnattributedRelayPolicyProofRejectsInventoryDrift(t *testing.T) { + const ruleID = "33333333-3333-3333-3333-333333333333" + p := NewWithDryRun("sbx", false) + p.ConfigureHostTrustRelay(true, "inventory-drift-test") + binding, err := p.ensureRelayToken(testInstance) + if err != nil { + t.Fatal(err) + } + resource := net.JoinHostPort("host.docker.internal", fmt.Sprint(binding.Port)) + rule := provider.NetworkPolicyRule{ + ID: ruleID, Name: testRelayPolicyRule, PolicyID: "local", Scope: "sandbox:" + testName, AppliesTo: "sandbox:" + testName, + ResourceType: "network", Resources: []string{resource}, Decision: provider.NetworkPolicyAllow, Origin: "scoped", Status: "active", Editable: true, Active: true, + } + binding, err = p.bindRelayPolicyProof(binding, relayPolicyProof{ + RuleNames: []string{testRelayPolicyRule}, InventoryDigest: relayPolicyInventoryDigest([]provider.NetworkPolicyRule{rule}), PreAllowProof: relayPolicyPreAllowBlocked, + }) + if err != nil { + t.Fatal(err) + } + defer p.releaseRelayToken(binding) + p.runCommand = func(_ context.Context, request commandRequest) (provider.ExecResult, error) { + args := strings.Join(request.args, " ") + switch args { + case "ls --json": + return provider.ExecResult{Stdout: readyListJSON}, nil + case "policy ls " + testName + " --include-inactive --json": + drifted := fmt.Sprintf(`[{"id":%q,"name":"changed relay rule","policy_id":"local","scope":%q,"applies_to":%q,"resource_type":"network","decision":"allow","resources":[%q],"origin":"scoped","status":"active","editable":true,"sandbox_id":%q}]`, ruleID, "sandbox:"+testName, "sandbox:"+testName, resource, testName) + return provider.ExecResult{Stdout: policyFixture(drifted)}, nil + default: + t.Fatalf("unexpected command: %v", request.args) + return provider.ExecResult{}, nil + } + } + + err = p.verifyUnattributedRelayPolicyProof(context.Background(), binding) + if err == nil || !strings.Contains(err.Error(), "policy changed") { + t.Fatalf("error = %v, want inventory drift failure", err) + } +} + func TestHostTrustRelayVerificationFailsWhenBindingRebindsDuringGuestProbe(t *testing.T) { p := NewWithDryRun("sbx", false) p.ConfigureHostTrustRelay(true, "rebind-test") @@ -467,6 +607,15 @@ func TestVerifyHostTrustRelayPolicyRequiresFreshTransparentExactPort(t *testing. {name: "wrong port", allowed: policyLogEntry("localhost:43124", "sandbox-one", "transparent", started), wantErr: "did not confirm"}, {name: "wrong route", allowed: policyLogEntry("localhost:43123", "sandbox-one", "forward", started), wantErr: "unexpected"}, {name: "wrong rule", allowed: policyLogEntryWithRule("localhost:43123", "sandbox-one", "transparent", "other-rule", started), wantErr: "unexpected policy rule"}, + {name: "empty rule without causal proof", allowed: policyLogEntryWithRule("localhost:43123", "sandbox-one", "transparent", "", started), wantErr: "pre-rule policy proof"}, + {name: "missing rule", allowed: policyLogEntryWithoutRule("localhost:43123", "sandbox-one", "transparent", started), wantErr: "omitted the matched rule identity"}, + {name: "null rule", allowed: strings.Replace(policyLogEntry("localhost:43123", "sandbox-one", "transparent", started), `"rule":"host relay"`, `"rule":null`, 1), wantErr: "omitted the matched rule identity"}, + {name: "whitespace rule", allowed: policyLogEntryWithRule("localhost:43123", "sandbox-one", "transparent", " ", started), wantErr: "unexpected policy rule"}, + {name: "zero count", allowed: strings.Replace(policyLogEntry("localhost:43123", "sandbox-one", "transparent", started), `"count_since":1`, `"count_since":0`, 1), wantErr: "invalid request count"}, + {name: "empty then wrong rule", allowed: policyLogEntryWithRule("localhost:43123", "sandbox-one", "transparent", "", started) + "," + policyLogEntryWithRule("localhost:43123", "sandbox-one", "transparent", "other-rule", started), wantErr: "unexpected policy rule"}, + {name: "wrong then empty rule", allowed: policyLogEntryWithRule("localhost:43123", "sandbox-one", "transparent", "other-rule", started) + "," + policyLogEntryWithRule("localhost:43123", "sandbox-one", "transparent", "", started), wantErr: "unexpected policy rule"}, + {name: "expected then wrong rule", allowed: policyLogEntry("localhost:43123", "sandbox-one", "transparent", started) + "," + policyLogEntryWithRule("localhost:43123", "sandbox-one", "transparent", "other-rule", started), wantErr: "unexpected policy rule"}, + {name: "wrong then expected rule", allowed: policyLogEntryWithRule("localhost:43123", "sandbox-one", "transparent", "other-rule", started) + "," + policyLogEntry("localhost:43123", "sandbox-one", "transparent", started), wantErr: "unexpected policy rule"}, {name: "blocked", allowed: policyLogEntry("localhost:43123", "sandbox-one", "transparent", started), blocked: policyLogEntry("localhost:43123", "sandbox-one", "transparent", started), wantErr: "blocked"}, {name: "credential forward", allowed: policyLogEntry("registry-1.docker.io:443", "sandbox-one", "forward", started) + "," + policyLogEntry("localhost:43123", "sandbox-one", "transparent", started), wantErr: "credential-bearing"}, } { @@ -498,6 +647,10 @@ func policyLogEntryWithRule(host, vmName, proxyType, rule string, lastSeen time. return fmt.Sprintf(`{"host":%q,"vm_name":%q,"proxy_type":%q,"rule":%q,"last_seen":%q,"since":%q,"count_since":1}`, host, vmName, proxyType, rule, lastSeen.Format(time.RFC3339Nano), lastSeen.Format(time.RFC3339Nano)) } +func policyLogEntryWithoutRule(host, vmName, proxyType string, lastSeen time.Time) string { + return fmt.Sprintf(`{"host":%q,"vm_name":%q,"proxy_type":%q,"last_seen":%q,"since":%q,"count_since":1}`, host, vmName, proxyType, lastSeen.Format(time.RFC3339Nano), lastSeen.Format(time.RFC3339Nano)) +} + func TestValidatePolicyCommandRejectsBroadPolicyAccess(t *testing.T) { accepted := [][]string{ {"policy", "log", "sandbox-one", "--json"}, diff --git a/internal/provider/dockersandboxes/network_policy.go b/internal/provider/dockersandboxes/network_policy.go index 537980f..09d85f9 100644 --- a/internal/provider/dockersandboxes/network_policy.go +++ b/internal/provider/dockersandboxes/network_policy.go @@ -18,7 +18,7 @@ type policyLogRecord struct { Host string `json:"host"` VMName string `json:"vm_name"` ProxyType string `json:"proxy_type"` - Rule string `json:"rule"` + Rule *string `json:"rule"` LastSeen time.Time `json:"last_seen"` Since time.Time `json:"since"` Count int `json:"count_since"` From fac5047f5cdae6c9989a6fc59d3a3ed45c871f8e Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 28 Aug 2026 18:30:22 +0800 Subject: [PATCH 2/2] Fix busy Docker Sandboxes host-trust reconciliation Use the dedicated read-only trust transport verifier during steady-state reconciliation so normal job-created runtime state does not trigger false quarantine. Preserve durable quarantine while a job is busy, avoid repeated registration fences, and resume exact cleanup once idle. --- internal/pool/host_trust_test.go | 249 +++++++++++++++++++++++++++- internal/pool/manager.go | 28 ++++ internal/pool/provider_lifecycle.go | 24 ++- internal/provider/provider.go | 13 +- 4 files changed, 290 insertions(+), 24 deletions(-) diff --git a/internal/pool/host_trust_test.go b/internal/pool/host_trust_test.go index 49c1f63..53cc588 100644 --- a/internal/pool/host_trust_test.go +++ b/internal/pool/host_trust_test.go @@ -22,6 +22,8 @@ import ( "github.com/solutionforest/ephemeral-action-runner/internal/config" gh "github.com/solutionforest/ephemeral-action-runner/internal/github" "github.com/solutionforest/ephemeral-action-runner/internal/hosttrust" + "github.com/solutionforest/ephemeral-action-runner/internal/logging" + poolstate "github.com/solutionforest/ephemeral-action-runner/internal/pool/state" "github.com/solutionforest/ephemeral-action-runner/internal/provider" ) @@ -396,8 +398,8 @@ func TestHostTrustReconciliationFencesWhenTransportVerificationFails(t *testing. if activator.calls != 0 { t.Fatalf("activation calls = %d, want zero while fencing the unhealthy registered runner", activator.calls) } - if activator.verifyCalls != 1 { - t.Fatalf("runtime verification calls = %d, want one before fencing the unhealthy registered runner", activator.verifyCalls) + if activator.verifyCalls != 0 { + t.Fatalf("common runtime verification calls = %d, want zero when a dedicated host-trust verifier exists", activator.verifyCalls) } if activator.verifyHostTrustCalls != 1 { t.Fatalf("provider host-trust verification calls = %d, want one before fencing the unhealthy registered runner", activator.verifyHostTrustCalls) @@ -438,8 +440,8 @@ func TestHostTrustReconciliationDoesNotReactivateHealthyTransport(t *testing.T) if activator.calls != 0 { t.Fatalf("host trust transport activations = %d, want zero for healthy current-generation transport", activator.calls) } - if activator.verifyCalls != 1 { - t.Fatalf("runtime verification calls = %d, want one for healthy current-generation transport", activator.verifyCalls) + if activator.verifyCalls != 0 { + t.Fatalf("common runtime verification calls = %d, want zero when a dedicated host-trust verifier exists", activator.verifyCalls) } if activator.verifyHostTrustCalls != 1 { t.Fatalf("provider host-trust verification calls = %d, want one for healthy current-generation transport", activator.verifyHostTrustCalls) @@ -478,8 +480,8 @@ func TestHostTrustReconciliationFencesBusyRunnerWhenTransportVerificationFails(t if activator.calls != 0 { t.Fatalf("host trust transport activations = %d, want zero for failed verification", activator.calls) } - if activator.verifyCalls != 1 { - t.Fatalf("runtime verification calls = %d, want one before fencing the busy runner", activator.verifyCalls) + if activator.verifyCalls != 0 { + t.Fatalf("common runtime verification calls = %d, want zero when a dedicated host-trust verifier exists", activator.verifyCalls) } if activator.verifyHostTrustCalls != 1 { t.Fatalf("provider host-trust verification calls = %d, want one before fencing the busy runner", activator.verifyHostTrustCalls) @@ -492,6 +494,241 @@ func TestHostTrustReconciliationFencesBusyRunnerWhenTransportVerificationFails(t } } +func TestHostTrustReconciliationBusyRunnerIgnoresJobMutableGeneralRuntimeState(t *testing.T) { + fake := &fakeProvider{instances: []provider.Instance{{Name: "runner-1", ProviderID: "fake:runner-1", State: "running"}}} + activator := &hostTrustVerifyingLifecycle{ + activatingLifecycle: &activatingLifecycle{ + Lifecycle: provider.AdaptLegacy(fake, false), + verifyErr: errors.New("workflow-created Docker client configuration is present"), + }, + } + github := &fakeGitHub{runner: gh.Runner{Name: "runner-1", ID: 42, Status: "online", Busy: true}, found: true} + manager := Manager{ + Config: config.Config{ + Provider: config.ProviderConfig{Type: "docker-sandboxes"}, + Image: config.ImageConfig{HostTrustMode: config.HostTrustModeOverlay, HostTrustScopes: []string{"system"}}, + }, + Provider: fake, + Lifecycle: activator, + GitHub: github, + } + var console bytes.Buffer + logDirectory := t.TempDir() + manager.Config.Logging.Directory = logDirectory + runtime, err := logging.NewRuntime(logging.Options{Directory: logDirectory, ManagerSinks: logging.SinkConsole, Stdout: &console, Stderr: &console}) + if err != nil { + t.Fatal(err) + } + defer runtime.Close() + manager.Logging = runtime + current := hosttrust.Snapshot{Generation: "g1", HostOS: "windows", Scopes: []string{"system"}, Certificates: []hosttrust.Certificate{{Name: "root.crt", PEM: []byte("pem")}}, CollectedAt: time.Now().UTC()} + active := map[string]ProvisionedInstance{"runner-1": {Name: "runner-1", ProviderID: "fake:runner-1", RunnerID: 42, HostTrustGeneration: "g1", ProviderOwned: true, Phase: LifecycleReady}} + busyHandoff := make(map[string]bool) + + manager.reconcileHostTrustRunners(context.Background(), active, current, busyHandoff) + manager.reconcileHostTrustRunners(context.Background(), active, current, busyHandoff) + + if got := active["runner-1"].Phase; got != LifecycleReady { + t.Fatalf("runner phase = %s, want %s", got, LifecycleReady) + } + if activator.verifyCalls != 0 { + t.Fatalf("common runtime verification calls = %d, want zero for job-mutable runtime state", activator.verifyCalls) + } + if activator.verifyHostTrustCalls != 2 { + t.Fatalf("provider host-trust verification calls = %d, want one read-only transport check per reconciliation", activator.verifyHostTrustCalls) + } + if got := len(hostTrustLeaseInputs(fake)); got != 1 { + t.Fatalf("busy handoff lease writes = %d, want one bounded lease", got) + } + if got := atomic.LoadInt32(&github.deleteCalls); got != 0 { + t.Fatalf("GitHub registration fence calls = %d, want zero for healthy busy transport", got) + } + if output := console.String(); strings.Contains(output, "host trust transport verification warning") || strings.Contains(output, "host trust registration fencing warning") { + t.Fatalf("healthy busy reconciliation emitted transport or fencing warning: %q", output) + } +} + +func TestHostTrustReconciliationActivatorOnlyFallsBackToCommonRuntimeVerification(t *testing.T) { + fake := &fakeProvider{instances: []provider.Instance{{Name: "runner-1", ProviderID: "fake:runner-1", State: "running"}}} + activator := &activatingLifecycle{ + Lifecycle: provider.AdaptLegacy(fake, false), + verifyErr: errors.New("runtime unavailable"), + } + github := &fakeGitHub{runner: gh.Runner{Name: "runner-1", ID: 42, Status: "online"}, found: true} + manager := Manager{ + Config: config.Config{ + Image: config.ImageConfig{HostTrustMode: config.HostTrustModeOverlay, HostTrustScopes: []string{"system"}}, + }, + Provider: fake, + Lifecycle: activator, + GitHub: github, + } + current := hosttrust.Snapshot{Generation: "g1", HostOS: "linux", Scopes: []string{"system"}, Certificates: []hosttrust.Certificate{{Name: "root.crt", PEM: []byte("pem")}}, CollectedAt: time.Now().UTC()} + active := map[string]ProvisionedInstance{"runner-1": {Name: "runner-1", ProviderID: "fake:runner-1", RunnerID: 42, HostTrustGeneration: "g1", ProviderOwned: true, Phase: LifecycleReady}} + + manager.reconcileHostTrustRunners(context.Background(), active, current, make(map[string]bool)) + + if activator.verifyCalls != 1 { + t.Fatalf("common runtime verification calls = %d, want one fallback verification", activator.verifyCalls) + } + if got := active["runner-1"].Phase; got != LifecycleQuarantined { + t.Fatalf("runner phase = %s, want %s", got, LifecycleQuarantined) + } + if got := atomic.LoadInt32(&github.deleteCalls); got != 1 { + t.Fatalf("GitHub registration fence calls = %d, want one after fallback verification failure", got) + } +} + +func TestDurableHostTrustQuarantineDoesNotRepeatBusyFenceAndRetiresWhenIdle(t *testing.T) { + manager, store, name := readyLifecycleManager(t) + fake := &fakeProvider{instances: []provider.Instance{{Name: name, ProviderID: "docker:ready-id", State: "running"}}} + activator := &hostTrustVerifyingLifecycle{ + activatingLifecycle: &activatingLifecycle{Lifecycle: provider.AdaptLegacy(fake, false)}, + verifyHostTrustErr: errors.New("relay marker unavailable"), + } + busyRunner := gh.Runner{Name: name, ID: 42, Status: "online", Busy: true} + github := &fakeGitHub{ + runner: busyRunner, + found: true, + listRunners: []gh.Runner{busyRunner}, + deleteErr: errors.New("runner is currently running a job"), + } + manager.Config.Image = config.ImageConfig{HostTrustMode: config.HostTrustModeOverlay, HostTrustScopes: []string{"system"}} + manager.Provider = fake + manager.Lifecycle = activator + manager.GitHub = github + current := hosttrust.Snapshot{Generation: "g1", HostOS: "windows", Scopes: []string{"system"}, Certificates: []hosttrust.Certificate{{Name: "root.crt", PEM: []byte("pem")}}, CollectedAt: time.Now().UTC()} + active := map[string]ProvisionedInstance{name: {Name: name, ProviderID: "docker:ready-id", RunnerID: 42, HostTrustGeneration: "g1", ProviderOwned: true, Phase: LifecycleReady}} + + manager.reconcileHostTrustRunners(context.Background(), active, current, make(map[string]bool)) + if got := atomic.LoadInt32(&github.deleteCalls); got != 1 { + t.Fatalf("initial GitHub registration fence calls = %d, want 1", got) + } + if got := active[name].Phase; got != LifecycleQuarantined { + t.Fatalf("phase after failed busy fence = %s, want %s", got, LifecycleQuarantined) + } + + var err error + active, err = manager.reconcilePhysicalPool(context.Background(), active, true) + if err != nil { + t.Fatal(err) + } + manager.reconcileHostTrustRunners(context.Background(), active, current, make(map[string]bool)) + if got := active[name].Phase; got != LifecycleQuarantined { + t.Fatalf("phase after busy reconciliation = %s, want durable quarantine", got) + } + if got := atomic.LoadInt32(&github.deleteCalls); got != 1 { + t.Fatalf("GitHub registration fence calls after busy reconciliation = %d, want no repeat", got) + } + if activator.verifyHostTrustCalls != 1 { + t.Fatalf("provider host-trust verification calls = %d, want no repeat after durable quarantine", activator.verifyHostTrustCalls) + } + record, err := store.Read(context.Background(), name) + if err != nil { + t.Fatal(err) + } + if record.Phase != poolstate.PhaseQuarantined { + t.Fatalf("durable phase = %s, want %s", record.Phase, poolstate.PhaseQuarantined) + } + + idleRunner := busyRunner + idleRunner.Busy = false + github.runner = idleRunner + github.listRunners = []gh.Runner{idleRunner} + github.deleteErr = nil + github.deleteFunc = func(context.Context, int64) error { + github.found = false + github.listRunners = nil + return nil + } + active, err = manager.reconcilePhysicalPool(context.Background(), active, true) + if err != nil { + t.Fatal(err) + } + if _, found := active[name]; found { + t.Fatalf("durably quarantined idle runner remains active: %#v", active[name]) + } + if got := atomic.LoadInt32(&github.deleteCalls); got != 2 { + t.Fatalf("total GitHub deletion calls = %d, want initial busy fence plus idle cleanup", got) + } + if got := atomic.LoadInt32(&fake.deleteCalls); got != 1 { + t.Fatalf("provider deletion calls = %d, want one exact idle cleanup", got) + } + record, err = store.Read(context.Background(), name) + if err != nil { + t.Fatal(err) + } + if record.Phase != poolstate.PhaseTombstoned { + t.Fatalf("durable phase after idle cleanup = %s, want %s", record.Phase, poolstate.PhaseTombstoned) + } +} + +func TestReconciliationHydratesDurableBusyQuarantineWithoutReAdoption(t *testing.T) { + manager, store, name := readyLifecycleManager(t) + if _, err := store.Transition(context.Background(), name, poolstate.Transition{Action: poolstate.ActionQuarantine, Reason: "host trust transport unavailable"}); err != nil { + t.Fatal(err) + } + fake := &fakeProvider{instances: []provider.Instance{{Name: name, ProviderID: "docker:ready-id", State: "running"}}} + busyRunner := gh.Runner{Name: name, ID: 42, Status: "online", Busy: true} + github := &fakeGitHub{runner: busyRunner, found: true, listRunners: []gh.Runner{busyRunner}} + manager.Provider = fake + manager.Lifecycle = provider.AdaptLegacy(fake, false) + manager.GitHub = github + + active, err := manager.reconcilePhysicalPool(context.Background(), nil, true) + if err != nil { + t.Fatal(err) + } + if got := active[name].Phase; got != LifecycleQuarantined { + t.Fatalf("hydrated phase = %s, want %s", got, LifecycleQuarantined) + } + if got := atomic.LoadInt32(&github.deleteCalls); got != 0 { + t.Fatalf("GitHub deletion calls = %d, want zero while hydrated quarantine is busy", got) + } + if got := atomic.LoadInt32(&fake.deleteCalls); got != 0 { + t.Fatalf("provider deletion calls = %d, want zero while hydrated quarantine is busy", got) + } + record, err := store.Read(context.Background(), name) + if err != nil { + t.Fatal(err) + } + if record.Phase != poolstate.PhaseQuarantined { + t.Fatalf("durable phase = %s, want %s", record.Phase, poolstate.PhaseQuarantined) + } +} + +func TestReconciliationRecoversTransientQuarantineWhenDurableStateIsReady(t *testing.T) { + manager, store, name := readyLifecycleManager(t) + fake := &fakeProvider{instances: []provider.Instance{{Name: name, ProviderID: "docker:ready-id", State: "running"}}} + idleRunner := gh.Runner{Name: name, ID: 42, Status: "online"} + github := &fakeGitHub{runner: idleRunner, found: true, listRunners: []gh.Runner{idleRunner}} + manager.Provider = fake + manager.Lifecycle = provider.AdaptLegacy(fake, false) + manager.GitHub = github + known := map[string]ProvisionedInstance{name: {Name: name, ProviderID: "docker:ready-id", RunnerID: 42, ProviderOwned: true, Phase: LifecycleQuarantined}} + + active, err := manager.reconcilePhysicalPool(context.Background(), known, true) + if err != nil { + t.Fatal(err) + } + if got := active[name].Phase; got != LifecycleReady { + t.Fatalf("recovered phase = %s, want %s", got, LifecycleReady) + } + if got := atomic.LoadInt32(&github.deleteCalls); got != 0 { + t.Fatalf("GitHub deletion calls = %d, want zero for transient quarantine recovery", got) + } + if got := atomic.LoadInt32(&fake.deleteCalls); got != 0 { + t.Fatalf("provider deletion calls = %d, want zero for transient quarantine recovery", got) + } + record, err := store.Read(context.Background(), name) + if err != nil { + t.Fatal(err) + } + if record.Phase != poolstate.PhaseReady { + t.Fatalf("durable phase = %s, want unchanged %s", record.Phase, poolstate.PhaseReady) + } +} + func TestHostTrustReconciliationFencesRegistrationWhenIdleLeaseRefreshFails(t *testing.T) { fake := &fakeProvider{execErrs: []error{errors.New("lease transport unavailable"), nil}} github := &fakeGitHub{runner: gh.Runner{Name: "runner-1", ID: 42, Status: "online"}, found: true} diff --git a/internal/pool/manager.go b/internal/pool/manager.go index d87c595..fad376b 100644 --- a/internal/pool/manager.go +++ b/internal/pool/manager.go @@ -1049,6 +1049,34 @@ func (m *Manager) reconcilePhysicalPool(ctx context.Context, known map[string]Pr reconciled[name] = vm return reconciled, fmt.Errorf("record GitHub job phase for %s: %w", name, err) } + if vm.Phase == LifecycleQuarantined && m.LifecycleState != nil { + record, recordErr := m.LifecycleState.Read(ctx, name) + if recordErr != nil { + reconciled[name] = vm + return reconciled, fmt.Errorf("read durable quarantine for %s: %w", name, recordErr) + } + if record.Phase == poolstate.PhaseQuarantined { + if runner.Busy { + // A busy GitHub runner cannot be deleted. Keep the exact + // capacity quarantined and let its unrefreshed host-trust + // lease expire closed instead of re-adopting it and + // repeating the same fence on every reconciliation. + reconciled[name] = vm + continue + } + if err := m.retireInstance(ctx, vm, "durably quarantined runner became idle"); err != nil { + if errors.Is(err, provider.ErrControlPlaneFailure) { + return reconciled, err + } + vm.Phase = LifecycleCleanupPending + reconciled[name] = vm + m.warnf("[%s] durably quarantined retirement pending: %v\n", name, err) + } else { + delete(reconciled, name) + } + continue + } + } if runner.Status == "online" { vm.Phase = LifecycleReady reconciled[name] = vm diff --git a/internal/pool/provider_lifecycle.go b/internal/pool/provider_lifecycle.go index 2ce7f69..006f8b8 100644 --- a/internal/pool/provider_lifecycle.go +++ b/internal/pool/provider_lifecycle.go @@ -76,23 +76,21 @@ func (m *Manager) verifyProviderRuntime(ctx context.Context, instance provider.I return nil } -// verifyProviderHostTrustRuntime preserves the common runtime check and adds -// an optional provider-specific read-only trust-transport check. Providers -// that only implement HostTrustRuntimeActivator therefore retain the common -// VerifyRuntime fallback, while providers with a transport that needs a -// stronger proof can implement HostTrustRuntimeVerifier. +// verifyProviderHostTrustRuntime prefers the provider-specific read-only +// trust-transport check when one exists. General VerifyRuntime implementations +// may validate pristine pre-job state that a running workflow is allowed to +// change, so they are not composed with the dedicated steady-state verifier. +// Providers that only implement HostTrustRuntimeActivator retain the common +// VerifyRuntime fallback. func (m *Manager) verifyProviderHostTrustRuntime(ctx context.Context, instance provider.Instance) error { - if err := m.verifyProviderRuntime(ctx, instance); err != nil { - return err - } verifier, ok := m.providerLifecycle().(provider.HostTrustRuntimeVerifier) - if !ok { + if ok { + if err := verifier.VerifyHostTrustRuntime(ctx, instance); err != nil { + return fmt.Errorf("verify provider host-trust runtime: %w", err) + } return nil } - if err := verifier.VerifyHostTrustRuntime(ctx, instance); err != nil { - return fmt.Errorf("verify provider host-trust runtime: %w", err) - } - return nil + return m.verifyProviderRuntime(ctx, instance) } func (m *Manager) verifyProviderAdmission(ctx context.Context, instance provider.Instance) error { diff --git a/internal/provider/provider.go b/internal/provider/provider.go index d3c4460..800ac33 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -332,11 +332,14 @@ type HostTrustRuntimeActivator interface { // HostTrustRuntimeVerifier is an optional provider capability for read-only // verification of transport state needed by an already registered runtime. -// The pool invokes it during steady-state reconciliation after the common -// VerifyRuntime check. Implementations must fail closed and must not mutate -// network policy, restart a daemon, reconfigure the guest, or expose -// credentials; returning nil means the exact instance's provider-owned trust -// transport is active and verified. +// The pool invokes it instead of the common VerifyRuntime check during +// steady-state host-trust reconciliation because general runtime verification +// may include pristine pre-job assertions that are invalid after assignment. +// Implementations must include every transport invariant needed for that +// steady-state decision, fail closed, and must not mutate network policy, +// restart a daemon, reconfigure the guest, or expose credentials; returning +// nil means the exact instance's provider-owned trust transport is active and +// verified. type HostTrustRuntimeVerifier interface { VerifyHostTrustRuntime(ctx context.Context, instance Instance) error }