Skip to content

Commit dc95e9a

Browse files
committed
fix(apply): redact secrets in the server dry-run config diff
talm apply --dry-run prints two diffs: talm's structured drift preview and the server-returned "Config diff:" block (Talos ModeDetails). Only the structured preview was redacted; the server diff was printed verbatim, leaking user encrypted-value secrets and Talos bootstrap key material (ca.key, token, encryption secrets) into stderr and CI logs. Redact ModeDetails by value before printing: collect the bootstrap allowlist values from the rendered config plus the user secret set from encrypted value files, then mask every occurrence. --show-secrets-in-drift governs this surface too, matching the structured drift preview. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
1 parent 5e8e522 commit dc95e9a

4 files changed

Lines changed: 407 additions & 2 deletions

File tree

docs/manual-test-plan.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,21 @@ Expected: a value authored in `values-secret.encrypted.yaml` is redacted in the
492492

493493
Regression anchor: the redaction is value-based and exact-match. A secret whose plaintext coincides with an ordinary structural string (e.g. a password literally set to `controlplane` or a bare port) will also redact that unrelated field — a documented sharp edge of value-based sealing, not a bug. Do not encrypt low-entropy values that collide with config strings.
494494

495+
### C5b. Server dry-run diff (`Config diff:`) redacts secrets too
496+
497+
`talm apply --dry-run` prints TWO diffs: talm's own structured drift preview (C5/C5a) AND the server-returned `Config diff:` block (Talos's `ModeDetails`, emitted after `Dry run summary:`). The second one is opaque diff text, so it is redacted by VALUE — covering both the Talos bootstrap allowlist and user encrypted values. With the C5a setup (a user secret rendered into a non-allowlisted field), and on a node whose config carries bootstrap key material:
498+
499+
```bash
500+
talm apply --dry-run -f nodes/node0.yaml 2>&1 | grep -F hunter2 && echo "FAIL: user secret leaked in Config diff" || echo "OK"
501+
# bootstrap key material must not appear verbatim in the Config diff either:
502+
talm apply --dry-run -f nodes/node0.yaml 2>&1 | grep -E '^\s*key: LS0t' && echo "FAIL: bootstrap key leaked" || echo "OK"
503+
talm apply --dry-run --show-secrets-in-drift -f nodes/node0.yaml 2>&1 | grep -cF hunter2 # >=1 with explicit opt-in
504+
```
505+
506+
Expected: in the `Config diff:` block, a user secret renders as `value: ***` and a bootstrap `*.key` / `token` / encryption-secret renders as `key: ***` (etc.) by default. `--show-secrets-in-drift` prints the block verbatim (the same flag that governs the structured drift preview governs this surface too). A leak here is a security-class bug: `talm` prints the server-computed diff verbatim, so an unredacted `ModeDetails` would expose CA private keys and user secrets in CI logs.
507+
508+
Regression anchor: the `Config diff:` shows secrets as context lines too (an unchanged `key:` adjacent to a change hunk), not only on `+`/`-` lines — value-based masking covers both. Public material adjacent to a secret slice (an `acceptedCAs[].crt`) is redacted alongside its key by design (the slice is masked whole); `--show-secrets-in-drift` restores it.
509+
495510
### C6. Drift preview shows secrets with explicit opt-in
496511

497512
```bash

pkg/commands/apply.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -496,7 +496,9 @@ func buildApplyClosure() applyFunc {
496496
return errors.Wrap(annotateApplyConfigError(err), "applying new configuration")
497497
}
498498

499-
helpers.PrintApplyResults(resp)
499+
if err := emitApplyResults(resp, data, true); err != nil {
500+
return err
501+
}
500502

501503
if err := runPostApplyGate(cosiCtx, c, data, nodeID, os.Stderr, true); err != nil {
502504
return err
@@ -612,7 +614,9 @@ func applyOneFileDirectPatchMode(configFile, withSecretsPath string) error {
612614
return errors.Wrap(annotateApplyConfigError(err), "applying new configuration")
613615
}
614616

615-
helpers.PrintApplyResults(resp)
617+
if err := emitApplyResults(resp, result, false); err != nil {
618+
return err
619+
}
616620

617621
return runPostApplyGates(ctx, c, result, targetNodes, false)
618622
})
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
// Copyright Cozystack Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
// Contract: `talm apply --dry-run` prints two diffs — talm's own structured
16+
// drift preview (redacted by path + user value) AND the server-returned
17+
// ModeDetails diff. The second one is opaque text, so it is redacted by VALUE.
18+
// These tests pin that ModeDetails never leaks a Talos bootstrap secret or a
19+
// user encrypted-value secret unless --show-secrets-in-drift is set.
20+
21+
package commands
22+
23+
import (
24+
"bytes"
25+
"strings"
26+
"testing"
27+
28+
machineapi "github.com/siderolabs/talos/pkg/machinery/api/machine"
29+
)
30+
31+
// TestContract_CollectConfigSecretValues pins which rendered-config leaves feed
32+
// the ModeDetails redaction set: Talos bootstrap material (fixed paths) and
33+
// Wireguard key material (by leaf name), but NOT ordinary public fields.
34+
func TestContract_CollectConfigSecretValues(t *testing.T) {
35+
rendered := []byte(`machine:
36+
type: controlplane
37+
token: MACHINE-TOKEN-AAAA
38+
ca:
39+
crt: MACHINE-CA-CRT
40+
key: MACHINE-CA-KEY-BBBB
41+
network:
42+
hostname: node0
43+
interfaces:
44+
- interface: eth0
45+
wireguard:
46+
privateKey: WG-PRIVATE-CCCC
47+
peers:
48+
- publicKey: WG-PUBLIC-PUB
49+
presharedKey: WG-PSK-DDDD
50+
allowedIPs:
51+
- 10.0.0.0/8
52+
cluster:
53+
secret: CLUSTER-SECRET-EEEE
54+
token: CLUSTER-TOKEN-FFFF
55+
ca:
56+
crt: CLUSTER-CA-CRT
57+
key: CLUSTER-CA-KEY-GGGG
58+
acceptedCAs:
59+
- crt: ACCEPTED-CA-CRT
60+
key: ACCEPTED-CA-KEY-HHHH
61+
`)
62+
63+
got, err := collectConfigSecretValues(rendered)
64+
if err != nil {
65+
t.Fatalf("collectConfigSecretValues: %v", err)
66+
}
67+
68+
mustCollect := []string{
69+
"MACHINE-TOKEN-AAAA",
70+
"MACHINE-CA-KEY-BBBB",
71+
"WG-PRIVATE-CCCC",
72+
"WG-PSK-DDDD",
73+
"CLUSTER-SECRET-EEEE",
74+
"CLUSTER-TOKEN-FFFF",
75+
"CLUSTER-CA-KEY-GGGG",
76+
"ACCEPTED-CA-KEY-HHHH",
77+
}
78+
for _, want := range mustCollect {
79+
if _, ok := got[want]; !ok {
80+
t.Errorf("secret value %q must be collected for ModeDetails redaction; got %v", want, keysOf(got))
81+
}
82+
}
83+
84+
// Public / low-entropy fields must NOT enter the value set, so they stay
85+
// readable in the dry-run diff and never clobber unrelated text.
86+
mustNotCollect := []string{"node0", "eth0", "WG-PUBLIC-PUB", "10.0.0.0/8"}
87+
for _, unwanted := range mustNotCollect {
88+
if _, ok := got[unwanted]; ok {
89+
t.Errorf("non-secret field %q must NOT be collected (would over-redact the dry-run diff)", unwanted)
90+
}
91+
}
92+
}
93+
94+
// TestContract_CollectConfigSecretValues_AcceptedCAsCrt pins the documented
95+
// bounded over-collection: acceptedCAs is an allowlisted slice path, so its
96+
// public crt is collected alongside the key. This is intentional — the slice is
97+
// redacted whole — and pinned so a future change to the granularity is a
98+
// conscious decision.
99+
func TestContract_CollectConfigSecretValues_AcceptedCAsCrt(t *testing.T) {
100+
rendered := []byte(`cluster:
101+
acceptedCAs:
102+
- crt: ACCEPTED-CA-CRT-XXXX
103+
key: ACCEPTED-CA-KEY-YYYY
104+
`)
105+
106+
got, err := collectConfigSecretValues(rendered)
107+
if err != nil {
108+
t.Fatalf("collectConfigSecretValues: %v", err)
109+
}
110+
111+
if _, ok := got["ACCEPTED-CA-CRT-XXXX"]; !ok {
112+
t.Error("acceptedCAs crt is collected whole with the slice (documented bounded over-collection)")
113+
}
114+
}
115+
116+
// TestContract_RedactValuesInText pins the by-value text masking used on
117+
// ModeDetails: every occurrence of every secret value becomes the sentinel, an
118+
// empty value set is a no-op (the --show-secrets-in-drift path), and a value
119+
// that is a substring of another does not survive as a fragment.
120+
func TestContract_RedactValuesInText(t *testing.T) {
121+
text := "value: high-entropy-secret\nother: high-entropy-secret-LONGER\n"
122+
values := secretSetOf("high-entropy-secret", "high-entropy-secret-LONGER")
123+
124+
got := redactValuesInText(text, values)
125+
if strings.Contains(got, "high-entropy-secret") {
126+
t.Errorf("no secret fragment may survive redaction:\n%s", got)
127+
}
128+
if !strings.Contains(got, modeDetailsRedactionSentinel) {
129+
t.Errorf("redacted text must carry the sentinel:\n%s", got)
130+
}
131+
132+
if redactValuesInText(text, nil) != text {
133+
t.Error("empty value set must leave the text verbatim (show-secrets path)")
134+
}
135+
}
136+
137+
// TestContract_PrintApplyResultsRedacted pins the end-to-end print path: a
138+
// ModeDetails diff carrying a secret is masked when the value is in scope and
139+
// printed verbatim when the value set is empty (show-secrets).
140+
func TestContract_PrintApplyResultsRedacted(t *testing.T) {
141+
const secret = "high-entropy-value-do-not-collide-abcdef0123456789"
142+
resp := &machineapi.ApplyConfigurationResponse{
143+
Messages: []*machineapi.ApplyConfiguration{
144+
{ModeDetails: "Config diff:\n+ value: " + secret + "\n"},
145+
},
146+
}
147+
148+
var redacted bytes.Buffer
149+
printApplyResultsRedacted(resp, secretSetOf(secret), &redacted)
150+
if strings.Contains(redacted.String(), secret) {
151+
t.Errorf("ModeDetails must redact an in-scope secret:\n%s", redacted.String())
152+
}
153+
154+
var shown bytes.Buffer
155+
printApplyResultsRedacted(resp, nil, &shown)
156+
if !strings.Contains(shown.String(), secret) {
157+
t.Errorf("empty value set (show-secrets) must print ModeDetails verbatim:\n%s", shown.String())
158+
}
159+
}
160+
161+
func keysOf(m map[string]struct{}) []string {
162+
out := make([]string, 0, len(m))
163+
for k := range m {
164+
out = append(out, k)
165+
}
166+
167+
return out
168+
}

0 commit comments

Comments
 (0)