policy/v2: bound the tests and sshTests failure report

The report printed every failure with the full labels, so its size was
labels times failures. Print 100 lines of 256-byte labels, then a count.
This commit is contained in:
Kristoffer Dalby committed 2026-10-30 14:55:36 +00:00
1 parent 5a316e83d0
commit 19d8960f10
3 files changed
+151 -18

No files matched your search

+24 -14
View File
@@ -43,53 +43,63 @@ type SSHPolicyTestResults struct {
Results []SSHPolicyTestResult `json:"results"`
}
// Errors renders the per-test failure breakdown joined by newlines.
// Errors renders the per-test failure breakdown joined by newlines,
// bounded by [testFailures].
func (r SSHPolicyTestResults) Errors() string {
if r.AllPassed {
return ""
}
var lines []string
var failures testFailures
for _, res := range r.Results {
if res.Passed {
continue
}
src := clipTestLabel(res.Src)
for _, e := range res.Errors {
lines = append(lines, fmt.Sprintf("%s: %s", res.Src, e))
failures.addf("%s: %s", src, clipTestLabel(e))
}
for _, user := range sortedUsers(res.AcceptFail) {
for _, dst := range res.AcceptFail[user] {
lines = append(lines, fmt.Sprintf(
failures.addf(
"%s/%s -> %s: expected ALLOWED, got DENIED",
res.Src, displayUser(user), dst,
))
src, clipTestLabel(displayUser(user)), clipTestLabel(dst),
)
}
}
for _, user := range sortedUsers(res.DenyFail) {
for _, dst := range res.DenyFail[user] {
lines = append(lines, fmt.Sprintf(
failures.addf(
"%s/%s -> %s: expected DENIED, got ALLOWED",
res.Src, displayUser(user), dst,
))
src, clipTestLabel(displayUser(user)), clipTestLabel(dst),
)
}
}
for _, user := range sortedUsers(res.CheckFail) {
for _, dst := range res.CheckFail[user] {
lines = append(lines, fmt.Sprintf(
// The reason scans AcceptOK; skip it for lines that
// are only counted.
var reason string
if !failures.full() {
reason = checkFailReason(res, user, dst)
}
failures.addf(
"%s/%s -> %s: expected ALLOWED via check, got %s",
res.Src, displayUser(user), dst,
checkFailReason(res, user, dst),
))
src, clipTestLabel(displayUser(user)), clipTestLabel(dst),
reason,
)
}
}
}
return strings.Join(lines, "\n")
return failures.String()
}
// failures is the breakdown apart per failing entry, so an API answer can
+63 -4
View File
@@ -163,39 +163,98 @@ type PolicyTestResults struct {
// keep the per-test detail because it is significantly more useful in
// CLI / config-reload paths where the user does not have a separate
// audit endpoint to consult.
//
// The breakdown is bounded by [testFailures].
func (r PolicyTestResults) Errors() string {
if r.AllPassed {
return ""
}
var lines []string
var failures testFailures
for _, res := range r.Results {
if res.Passed {
continue
}
src := clipTestLabel(res.Src)
protoSuffix := ""
if res.Proto != "" {
protoSuffix = fmt.Sprintf(" (%s)", res.Proto)
}
for _, e := range res.Errors {
lines = append(lines, fmt.Sprintf("%s%s: %s", res.Src, protoSuffix, e))
failures.addf("%s%s: %s", src, protoSuffix, clipTestLabel(e))
}
for _, dst := range res.AcceptFail {
lines = append(lines, fmt.Sprintf("%s -> %s%s: expected ALLOWED, got DENIED", res.Src, dst, protoSuffix))
failures.addf(
"%s -> %s%s: expected ALLOWED, got DENIED",
src, clipTestLabel(dst), protoSuffix,
)
}
for _, dst := range res.DenyFail {
lines = append(lines, fmt.Sprintf("%s -> %s%s: expected DENIED, got ALLOWED", res.Src, dst, protoSuffix))
failures.addf(
"%s -> %s%s: expected DENIED, got ALLOWED",
src, clipTestLabel(dst), protoSuffix,
)
}
}
return failures.String()
}
// The policy author sizes both the labels on a failure line and the
// number of failures, so a breakdown rendered in full grows with their
// product. It is rendered on the write path and, for a stored policy
// whose tests stopped passing, at every boot, so both factors are capped.
const (
maxTestFailureLines = 100
maxTestLabelBytes = 256
)
// testFailures collects the lines of a `tests` or `sshTests` failure
// breakdown. Failures past [maxTestFailureLines] are counted, not
// rendered.
type testFailures struct {
lines []string
omitted int
}
func (f *testFailures) full() bool {
return len(f.lines) >= maxTestFailureLines
}
func (f *testFailures) addf(format string, args ...any) {
if f.full() {
f.omitted++
return
}
f.lines = append(f.lines, fmt.Sprintf(format, args...))
}
func (f *testFailures) String() string {
lines := f.lines
if f.omitted > 0 {
lines = append(lines, fmt.Sprintf("... and %d more", f.omitted))
}
return strings.Join(lines, "\n")
}
// clipTestLabel shortens s to [maxTestLabelBytes] for a failure line.
func clipTestLabel(s string) string {
if len(s) <= maxTestLabelBytes {
return s
}
return strings.ToValidUTF8(s[:maxTestLabelBytes], "") + "..."
}
// TestFailure is one failing tests or sshTests entry: its src as written and
// its lines of the failure breakdown.
type TestFailure struct {
+64
View File
@@ -1,6 +1,7 @@
package v2
import (
"fmt"
"strings"
"testing"
@@ -439,3 +440,66 @@ func TestPolicyTestResultsErrorsRendering(t *testing.T) {
// Lines should be newline-separated, not space-joined.
assert.Equal(t, 2, strings.Count(rendered, "\n")+1, "expected one line per failing assertion")
}
// TestFailureBreakdownIsBounded pins that the rendered breakdown of
// failing tests and sshTests does not grow with label length times
// failures: the policy author sizes both, and the breakdown is also
// rendered at boot.
func TestFailureBreakdownIsBounded(t *testing.T) {
users := sshTestUsers()
nodes := sshTestNodes(users)
const failures = 2000
name := strings.Repeat("a", 64<<10)
hosts := `"hosts": {"` + name + `": "100.64.0.1", "gone": "100.64.0.99"},`
quoted := make([]string, failures)
for i := range failures {
quoted[i] = fmt.Sprintf(`"x%d"`, i)
}
names := strings.Join(quoted, ",")
for _, tt := range []struct {
name, block string
sentinel error
}{
{
name: "sshTests dst without node",
block: `"sshTests": [{"src": "` + name + `", "dst": [` +
strings.Repeat(`"gone",`, failures) + `], "deny": ["root"]}]`,
sentinel: errSSHPolicyTestsFailed,
},
{
name: "sshTests accept",
block: `"sshTests": [{"src": "` + name +
`", "dst": ["100.64.0.2"], "accept": [` + names + `]}]`,
sentinel: errSSHPolicyTestsFailed,
},
{
name: "sshTests check",
block: `"sshTests": [{"src": "` + name +
`", "dst": ["100.64.0.2"], "check": [` + names + `]}]`,
sentinel: errSSHPolicyTestsFailed,
},
{
name: "tests accept",
block: `"tests": [{"src": "` + name + `", "accept": [` +
strings.Repeat(`"gone:22",`, failures) + `]}]`,
sentinel: errPolicyTestsFailed,
},
} {
t.Run(tt.name, func(t *testing.T) {
pm, err := NewPolicyManager([]byte(`{}`), users, nodes.ViewSlice())
require.NoError(t, err)
_, err = pm.SetPolicy([]byte(`{"acls": [], ` + hosts + tt.block + `}`))
require.ErrorIs(t, err, tt.sentinel)
require.Less(t, len(err.Error()), 100<<10)
require.True(t, strings.HasSuffix(err.Error(), fmt.Sprintf(
"... and %d more", failures-maxTestFailureLines,
)), "error ends with the count")
})
}
}