Skip to content

Commit 0697ab3

Browse files
dilyevskyclaude
dilyevsky
and
claude
committed
[apiserver] reject status.type tampering via ValidateUpdate
The statusSubResourceStrategy does not call PrepareForUpdate, so controllers updating the /status subresource could overwrite status.type with arbitrary values. Add a ValidateUpdate check that rejects any update where status.type doesn't match deriveRecordType(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c30c045 commit 0697ab3

File tree

2 files changed

+215
-0
lines changed

2 files changed

+215
-0
lines changed

‎api/core/v1alpha3/domainrecord_validate.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,15 @@ func (r *DomainRecord) ValidateUpdate(ctx context.Context, obj runtime.Object) f
7777
))
7878
}
7979

80+
// status.type is server-authoritative; reject updates that tamper with it.
81+
if expected := deriveRecordType(r); r.Status.Type != expected {
82+
errs = append(errs, field.Invalid(
83+
field.NewPath("status", "type"),
84+
r.Status.Type,
85+
fmt.Sprintf("must be %q (derived from spec)", expected),
86+
))
87+
}
88+
8089
return errs
8190
}
8291

‎api/core/v1alpha3/domainrecord_validate_test.go‎

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
package v1alpha3
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
8+
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
func int32Ptr(v int32) *int32 { return &v }
14+
15+
func TestValidateUpdate_StatusTypeServerAuthoritative(t *testing.T) {
16+
tests := []struct {
17+
name string
18+
old *DomainRecord
19+
updated *DomainRecord
20+
wantErr bool
21+
errField string
22+
errMessage string
23+
}{
24+
{
25+
name: "correct status.type on A record passes",
26+
old: &DomainRecord{
27+
ObjectMeta: metav1.ObjectMeta{Name: "example--a"},
28+
Spec: DomainRecordSpec{
29+
Name: "example",
30+
TTL: int32Ptr(60),
31+
Target: DomainRecordTarget{
32+
DNS: &DomainRecordTargetDNS{
33+
A: []string{"1.2.3.4"},
34+
},
35+
},
36+
},
37+
Status: DomainRecordStatus{Type: "A"},
38+
},
39+
updated: &DomainRecord{
40+
ObjectMeta: metav1.ObjectMeta{Name: "example--a"},
41+
Spec: DomainRecordSpec{
42+
Name: "example",
43+
TTL: int32Ptr(60),
44+
Target: DomainRecordTarget{
45+
DNS: &DomainRecordTargetDNS{
46+
A: []string{"5.6.7.8"},
47+
},
48+
},
49+
},
50+
Status: DomainRecordStatus{Type: "A"},
51+
},
52+
wantErr: false,
53+
},
54+
{
55+
name: "correct status.type on Ref record passes",
56+
old: &DomainRecord{
57+
ObjectMeta: metav1.ObjectMeta{Name: "example--ref"},
58+
Spec: DomainRecordSpec{
59+
Name: "example",
60+
TTL: int32Ptr(60),
61+
Target: DomainRecordTarget{
62+
Ref: &LocalObjectReference{
63+
Group: "core.apoxy.dev",
64+
Kind: "Proxy",
65+
Name: "my-proxy",
66+
},
67+
},
68+
},
69+
Status: DomainRecordStatus{Type: "Ref"},
70+
},
71+
updated: &DomainRecord{
72+
ObjectMeta: metav1.ObjectMeta{Name: "example--ref"},
73+
Spec: DomainRecordSpec{
74+
Name: "example",
75+
TTL: int32Ptr(60),
76+
Target: DomainRecordTarget{
77+
Ref: &LocalObjectReference{
78+
Group: "core.apoxy.dev",
79+
Kind: "Proxy",
80+
Name: "other-proxy",
81+
},
82+
},
83+
},
84+
Status: DomainRecordStatus{Type: "Ref"},
85+
},
86+
wantErr: false,
87+
},
88+
{
89+
name: "tampered status.type on A record is rejected",
90+
old: &DomainRecord{
91+
ObjectMeta: metav1.ObjectMeta{Name: "example--a"},
92+
Spec: DomainRecordSpec{
93+
Name: "example",
94+
TTL: int32Ptr(60),
95+
Target: DomainRecordTarget{
96+
DNS: &DomainRecordTargetDNS{
97+
A: []string{"1.2.3.4"},
98+
},
99+
},
100+
},
101+
Status: DomainRecordStatus{Type: "A"},
102+
},
103+
updated: &DomainRecord{
104+
ObjectMeta: metav1.ObjectMeta{Name: "example--a"},
105+
Spec: DomainRecordSpec{
106+
Name: "example",
107+
TTL: int32Ptr(60),
108+
Target: DomainRecordTarget{
109+
DNS: &DomainRecordTargetDNS{
110+
A: []string{"1.2.3.4"},
111+
},
112+
},
113+
},
114+
Status: DomainRecordStatus{Type: "A/AAAA"},
115+
},
116+
wantErr: true,
117+
errField: "status.type",
118+
errMessage: `must be "A" (derived from spec)`,
119+
},
120+
{
121+
name: "empty status.type on AAAA record is rejected",
122+
old: &DomainRecord{
123+
ObjectMeta: metav1.ObjectMeta{Name: "example--aaaa"},
124+
Spec: DomainRecordSpec{
125+
Name: "example",
126+
TTL: int32Ptr(60),
127+
Target: DomainRecordTarget{
128+
DNS: &DomainRecordTargetDNS{
129+
AAAA: []string{"2001:db8::1"},
130+
},
131+
},
132+
},
133+
Status: DomainRecordStatus{Type: "AAAA"},
134+
},
135+
updated: &DomainRecord{
136+
ObjectMeta: metav1.ObjectMeta{Name: "example--aaaa"},
137+
Spec: DomainRecordSpec{
138+
Name: "example",
139+
TTL: int32Ptr(60),
140+
Target: DomainRecordTarget{
141+
DNS: &DomainRecordTargetDNS{
142+
AAAA: []string{"2001:db8::1"},
143+
},
144+
},
145+
},
146+
Status: DomainRecordStatus{Type: ""},
147+
},
148+
wantErr: true,
149+
errField: "status.type",
150+
errMessage: `must be "AAAA" (derived from spec)`,
151+
},
152+
{
153+
name: "tampered status.type on CNAME record is rejected",
154+
old: &DomainRecord{
155+
ObjectMeta: metav1.ObjectMeta{Name: "example--fqdn"},
156+
Spec: DomainRecordSpec{
157+
Name: "example",
158+
TTL: int32Ptr(60),
159+
Target: DomainRecordTarget{
160+
DNS: &DomainRecordTargetDNS{
161+
FQDN: strPtr("other.example.com"),
162+
},
163+
},
164+
},
165+
Status: DomainRecordStatus{Type: "CNAME"},
166+
},
167+
updated: &DomainRecord{
168+
ObjectMeta: metav1.ObjectMeta{Name: "example--fqdn"},
169+
Spec: DomainRecordSpec{
170+
Name: "example",
171+
TTL: int32Ptr(60),
172+
Target: DomainRecordTarget{
173+
DNS: &DomainRecordTargetDNS{
174+
FQDN: strPtr("other.example.com"),
175+
},
176+
},
177+
},
178+
Status: DomainRecordStatus{Type: "A"},
179+
},
180+
wantErr: true,
181+
errField: "status.type",
182+
errMessage: `must be "CNAME" (derived from spec)`,
183+
},
184+
}
185+
186+
for _, tt := range tests {
187+
t.Run(tt.name, func(t *testing.T) {
188+
errs := tt.updated.ValidateUpdate(context.Background(), tt.old)
189+
if !tt.wantErr {
190+
assert.Empty(t, errs, "expected no validation errors")
191+
return
192+
}
193+
require.NotEmpty(t, errs, "expected validation errors")
194+
var found bool
195+
for _, e := range errs {
196+
if e.Field == tt.errField {
197+
found = true
198+
assert.Contains(t, e.Detail, tt.errMessage)
199+
}
200+
}
201+
assert.True(t, found, "expected error on field %q, got: %v", tt.errField, errs)
202+
})
203+
}
204+
}
205+
206+
func strPtr(s string) *string { return &s }

0 commit comments

Comments
 (0)