Skip to content

Commit dfb4ef2

Browse files
authored
fix: mask secrets in stored step outputs (#2861)
1 parent 7c085e2 commit dfb4ef2

8 files changed

Lines changed: 378 additions & 6 deletions

File tree

‎conformance/spec069_secrets/secrets_test.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"net/http"
99
"net/http/httptest"
1010
"os"
11+
"path/filepath"
1112
"testing"
1213

1314
"github.com/dagucloud/dagu/v2/conformance/harness"
@@ -58,6 +59,31 @@ func TestFileProviderMissingFile(t *testing.T) {
5859
result.ExpectStderrContains("secret file not found")
5960
}
6061

62+
// A later step of the same run reads a published output with the secret
63+
// intact. A retry reuses the succeeded step's stored output, which holds the
64+
// mask, including for a secret that JSON escapes.
65+
func TestStepOutputMaskedOnRetry(t *testing.T) {
66+
t.Parallel()
67+
68+
for _, fixture := range []string{"step_output_retry.yaml", "step_output_retry_escaped.yaml"} {
69+
t.Run(fixture, func(t *testing.T) {
70+
t.Parallel()
71+
72+
const runID = "spec069-step-output"
73+
dagu := harness.NewRunner(t)
74+
env := []string{"DAGU_HOME=" + filepath.Join(t.TempDir(), "dagu")}
75+
76+
dagu.RunWithEnv(env, "start", "--run-id="+runID, fixture).ExpectNonZeroExitCode()
77+
dagu.ExpectTextFileContent("received.out", "real\n")
78+
79+
dagu.WriteFile("ready", "")
80+
dagu.RunWithEnv(env, "retry", "--run-id="+runID, fixture).ExpectExitCode(0)
81+
dagu.ExpectTextFileContent("received.out", "real\n*******\n")
82+
dagu.ExpectTextFileContent("logins.out", "login\n")
83+
})
84+
}
85+
}
86+
6187
func TestUnknownProvider(t *testing.T) {
6288
t.Parallel()
6389

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
k7Qx"m2\Vz<p9>&w4
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
working_dir: .
2+
secrets:
3+
- name: API_TOKEN
4+
provider: file
5+
key: file_secret.txt
6+
steps:
7+
- id: login
8+
run: |
9+
printf 'login\n' >> logins.out
10+
printf 'token=%s\n' "$API_TOKEN" >> "$DAGU_OUTPUT_FILE"
11+
outputs:
12+
- name: token
13+
14+
- id: use
15+
depends: login
16+
run: |
17+
if [ '${steps.login.outputs.token}' = "$API_TOKEN" ]; then
18+
printf 'real\n' >> received.out
19+
else
20+
printf '%s\n' '${steps.login.outputs.token}' >> received.out
21+
fi
22+
[ -f ready ]
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
working_dir: .
2+
secrets:
3+
- name: API_TOKEN
4+
provider: file
5+
key: escaped_secret.txt
6+
steps:
7+
- id: login
8+
run: |
9+
printf 'login\n' >> logins.out
10+
printf 'token=%s\n' "$API_TOKEN" >> "$DAGU_OUTPUT_FILE"
11+
outputs:
12+
- name: token
13+
14+
- id: use
15+
depends: login
16+
run: |
17+
if [ '${steps.login.outputs.token}' = "$API_TOKEN" ]; then
18+
printf 'real\n' >> received.out
19+
else
20+
printf '%s\n' '${steps.login.outputs.token}' >> received.out
21+
fi
22+
[ -f ready ]

‎internal/runtime/agent/agent_test.go‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1344,6 +1344,46 @@ steps:
13441344
require.Contains(t, outputs["response"], "*******", "masked placeholder expected")
13451345
}
13461346

1347+
// A later step of the same run reads a published output with the secret
1348+
// intact, while stored run status keeps only the mask.
1349+
func TestAgent_StepOutputSecretMasking(t *testing.T) {
1350+
if runtime.GOOS == "windows" {
1351+
t.Skip("fixture uses POSIX shell commands")
1352+
}
1353+
t.Parallel()
1354+
th := test.Setup(t)
1355+
1356+
secretValue := "step-output-secret-5b7e"
1357+
secretFile := th.TempFile(t, "secret.txt", []byte(secretValue))
1358+
1359+
dag := th.DAG(t, `type: graph
1360+
secrets:
1361+
- name: API_TOKEN
1362+
provider: file
1363+
key: `+secretFile+`
1364+
steps:
1365+
- id: login
1366+
run: printf 'token=%s\n' "$API_TOKEN" >> "$DAGU_OUTPUT_FILE"
1367+
outputs:
1368+
- name: token
1369+
- id: use
1370+
depends: [login]
1371+
run: test '${steps.login.outputs.token}' = "$API_TOKEN"
1372+
`)
1373+
dag.Agent().RunSuccess(t)
1374+
1375+
latest, err := th.DAGRunMgr.GetLatestStatus(th.Context, dag.DAG)
1376+
require.NoError(t, err)
1377+
login, err := latest.NodeByName("login")
1378+
require.NoError(t, err)
1379+
require.NotNil(t, login.StepOutputsValue)
1380+
require.JSONEq(t, `{"token":"*******"}`, *login.StepOutputsValue)
1381+
1382+
statusJSON, err := json.Marshal(latest)
1383+
require.NoError(t, err)
1384+
require.NotContains(t, string(statusJSON), secretValue)
1385+
}
1386+
13471387
func TestAgent_RegistryRefSecretResolution(t *testing.T) {
13481388
t.Parallel()
13491389
th := test.Setup(t)

‎internal/runtime/agent/status_masking.go‎

Lines changed: 113 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@
44
package agent
55

66
import (
7+
"bytes"
8+
"encoding/json"
79
"maps"
10+
"strconv"
11+
"strings"
812

913
"github.com/dagucloud/dagu/v2/internal/cmn/collections"
1014
"github.com/dagucloud/dagu/v2/internal/cmn/masking"
@@ -28,11 +32,39 @@ func (a *Agent) maskStatusSecrets(status *ir.DAGRunStatus) {
2832
status.Error = a.secretMasker.MaskString(status.Error)
2933
}
3034

35+
// newStatusSecretMasker returns a masker for run status. Stored outputs are
36+
// JSON text, so each secret is also masked in the escaped forms JSON encoding
37+
// gives it.
3138
func newStatusSecretMasker(secretEnvs []string) *masking.Masker {
3239
if len(secretEnvs) == 0 {
3340
return nil
3441
}
35-
return masking.NewMasker(masking.SourcedEnvVars{Secrets: secretEnvs})
42+
secrets := make([]string, 0, len(secretEnvs))
43+
for _, env := range secretEnvs {
44+
secrets = append(secrets, env)
45+
name, value, ok := strings.Cut(env, "=")
46+
if !ok || value == "" {
47+
continue
48+
}
49+
for _, escapeHTML := range []bool{true, false} {
50+
if escaped := jsonStringBody(value, escapeHTML); escaped != value {
51+
secrets = append(secrets, name+"="+escaped)
52+
}
53+
}
54+
}
55+
return masking.NewMasker(masking.SourcedEnvVars{Secrets: secrets})
56+
}
57+
58+
// jsonStringBody returns value encoded as a JSON string, without the quotes.
59+
func jsonStringBody(value string, escapeHTML bool) string {
60+
var buf bytes.Buffer
61+
encoder := json.NewEncoder(&buf)
62+
encoder.SetEscapeHTML(escapeHTML)
63+
if err := encoder.Encode(value); err != nil {
64+
return value
65+
}
66+
encoded := strings.TrimSuffix(buf.String(), "\n")
67+
return encoded[1 : len(encoded)-1]
3668
}
3769

3870
func maskNodeSecrets(masker *masking.Masker, node *ir.Node) {
@@ -44,10 +76,89 @@ func maskNodeSecrets(masker *masking.Masker, node *ir.Node) {
4476
node.StatusDetails = maskNodeStatusDetails(masker, node.StatusDetails)
4577
node.OutputVariables = maskOutputVariables(masker, node.OutputVariables)
4678
node.OutputValue = maskStringPointer(masker, node.OutputValue)
47-
node.OutputsValue = maskStringPointer(masker, node.OutputsValue)
79+
node.OutputsValue = maskOutputDocument(masker, node.OutputsValue)
80+
node.StepOutputsValue = maskStepOutputs(masker, node)
4881
node.AgentSession = maskAgentSession(masker, node.AgentSession)
4982
}
5083

84+
// maskStepOutputs masks the outputs a step published. Human-task outputs are
85+
// kept: they are operator input, stored as entered next to HumanTaskInput, and
86+
// a resumed run reads them back from status.
87+
func maskStepOutputs(masker *masking.Masker, node *ir.Node) *string {
88+
if node.Step.HumanTask != nil {
89+
return node.StepOutputsValue
90+
}
91+
return maskOutputDocument(masker, node.StepOutputsValue)
92+
}
93+
94+
// maskOutputDocument masks a stored JSON output document. Plain replacement
95+
// can split a JSON token, such as a secret matching a number; such a document
96+
// is masked value by value instead, so a run that reuses it can still read it.
97+
// It never keeps secret text that plain replacement masks.
98+
func maskOutputDocument(masker *masking.Masker, value *string) *string {
99+
masked := maskStringPointer(masker, value)
100+
if masked == nil || *masked == *value || json.Valid([]byte(*masked)) || !json.Valid([]byte(*value)) {
101+
return masked
102+
}
103+
decoder := json.NewDecoder(strings.NewReader(*value))
104+
decoder.UseNumber()
105+
var decoded any
106+
if err := decoder.Decode(&decoded); err != nil {
107+
return masked
108+
}
109+
var buf bytes.Buffer
110+
encoder := json.NewEncoder(&buf)
111+
encoder.SetEscapeHTML(false)
112+
if err := encoder.Encode(maskJSONValue(masker, decoded)); err != nil {
113+
return masked
114+
}
115+
document := strings.TrimSuffix(buf.String(), "\n")
116+
// A secret spanning JSON values is in no single decoded value, so
117+
// re-encoding writes it back out.
118+
if masker.MaskString(document) != document {
119+
return masked
120+
}
121+
return &document
122+
}
123+
124+
// maskJSONValue masks secrets in decoded JSON. A number, boolean, or null
125+
// holding a secret becomes the masked string.
126+
func maskJSONValue(masker *masking.Masker, value any) any {
127+
switch typed := value.(type) {
128+
case string:
129+
return masker.MaskString(typed)
130+
case json.Number:
131+
return maskJSONLiteral(masker, typed.String(), typed)
132+
case bool:
133+
return maskJSONLiteral(masker, strconv.FormatBool(typed), typed)
134+
case nil:
135+
return maskJSONLiteral(masker, "null", nil)
136+
case []any:
137+
masked := make([]any, len(typed))
138+
for i, item := range typed {
139+
masked[i] = maskJSONValue(masker, item)
140+
}
141+
return masked
142+
case map[string]any:
143+
masked := make(map[string]any, len(typed))
144+
for key, item := range typed {
145+
masked[masker.MaskString(key)] = maskJSONValue(masker, item)
146+
}
147+
return masked
148+
default:
149+
return value
150+
}
151+
}
152+
153+
// maskJSONLiteral returns the masked text of a JSON literal holding a secret,
154+
// and value otherwise.
155+
func maskJSONLiteral(masker *masking.Masker, literal string, value any) any {
156+
if masked := masker.MaskString(literal); masked != literal {
157+
return masked
158+
}
159+
return value
160+
}
161+
51162
// maskAgentSession masks the displayed text of an agent session. Answers are
52163
// kept because a resumed step reads them back from status.
53164
func maskAgentSession(masker *masking.Masker, session *ir.AgentSession) *ir.AgentSession {

0 commit comments

Comments
 (0)