feat(executor): secrets, vars and masking from the job (#25)
- commit
- c93689733dd6ba4beb0fbc9568f2b42099bb9b05
- author
- brice · · 2026-10-14 22:08 UTC
- committer
- bnema · · 2026-10-14 22:08 UTC
- parent
- 49c3309
- signature
- SSH Unverified
SHA256:VOCA5lYARHcbKDZ7GhTV45VmcjWnJ6ZQIBazj17LivAThe key is unknown
15 files · +1168 -145
M internal/execstream/exec.proto +22 -0
| @@ -29,9 +29,21 @@ message Frame { | |||
| 29 | 29 | StepSummary step_summary = 11; // executor -> host, before the StepCompleted of the step | |
| 30 | 30 | ResolveRequest resolve_request = 12; // executor -> host | |
| 31 | 31 | ResolveResponse resolve_response = 13; // host -> executor, answers a ResolveRequest | |
| 32 | + | NamedValue secret = 14; // host -> executor, one per secret, before Run | |
| 33 | + | NamedValue var = 15; // host -> executor, one per variable, before Run | |
| 32 | 34 | } | |
| 33 | 35 | } | |
| 34 | 36 | ||
| 37 | + | // NamedValue is one secret (Job.secrets) or one variable (Job.vars) of the job. | |
| 38 | + | // They travel in frames of their own, before Run, rather than in JobSpec: a | |
| 39 | + | // frame is at most 2 MiB (execstream.MaxFrame) and ermite allows many secrets of | |
| 40 | + | // 48 KiB, so a map in Run could not carry them. The executor keeps them until | |
| 41 | + | // Run arrives. Secrets are never logged nor written to disk. | |
| 42 | + | message NamedValue { | |
| 43 | + | string name = 1; | |
| 44 | + | string value = 2; | |
| 45 | + | } | |
| 46 | + | ||
| 35 | 47 | message Hello { | |
| 36 | 48 | string version = 1; // runner version of the executor | |
| 37 | 49 | string sandbox = 2; // "vm" or "container" | |
| @@ -85,6 +97,16 @@ message JobSpec { | |||
| 85 | 97 | // job's repository and refs. | |
| 86 | 98 | uint32 cache_port = 15; | |
| 87 | 99 | string cache_token = 16; | |
| 100 | + | // Job.secrets and Job.vars are not here: they come in NamedValue frames | |
| 101 | + | // before Run (see NamedValue). The executor takes the vars of those frames | |
| 102 | + | // for the "vars" context when contexts has none. | |
| 103 | + | reserved 17, 18; | |
| 104 | + | reserved "secrets", "vars"; | |
| 105 | + | // secrets_withheld is set by the host when the server sends Job.trusted and | |
| 106 | + | // the job is not trusted (feature ci.secrets): "Set up job" then says that | |
| 107 | + | // secrets are not available. Never set for servers without the feature, for | |
| 108 | + | // which an absent Job.trusted only means "not sent". | |
| 109 | + | bool secrets_withheld = 19; | |
| 88 | 110 | } | |
| 89 | 111 | ||
| 90 | 112 | enum StreamKind { | |
| @@ -29,9 +29,21 @@ message Frame { | |||||
| 29 | StepSummary step_summary = 11; // executor -> host, before the StepCompleted of the step | 29 | StepSummary step_summary = 11; // executor -> host, before the StepCompleted of the step | ||
| 30 | ResolveRequest resolve_request = 12; // executor -> host | 30 | ResolveRequest resolve_request = 12; // executor -> host | ||
| 31 | ResolveResponse resolve_response = 13; // host -> executor, answers a ResolveRequest | 31 | ResolveResponse resolve_response = 13; // host -> executor, answers a ResolveRequest | ||
| 32 | + | NamedValue secret = 14; // host -> executor, one per secret, before Run | |||
| 33 | + | NamedValue var = 15; // host -> executor, one per variable, before Run | |||
| 32 | } | 34 | } | ||
| 33 | } | 35 | } | ||
| 34 | 36 | ||||
| 37 | + | // NamedValue is one secret (Job.secrets) or one variable (Job.vars) of the job. | |||
| 38 | + | // They travel in frames of their own, before Run, rather than in JobSpec: a | |||
| 39 | + | // frame is at most 2 MiB (execstream.MaxFrame) and ermite allows many secrets of | |||
| 40 | + | // 48 KiB, so a map in Run could not carry them. The executor keeps them until | |||
| 41 | + | // Run arrives. Secrets are never logged nor written to disk. | |||
| 42 | + | message NamedValue { | |||
| 43 | + | string name = 1; | |||
| 44 | + | string value = 2; | |||
| 45 | + | } | |||
| 46 | + | ||||
| 35 | message Hello { | 47 | message Hello { | ||
| 36 | string version = 1; // runner version of the executor | 48 | string version = 1; // runner version of the executor | ||
| 37 | string sandbox = 2; // "vm" or "container" | 49 | string sandbox = 2; // "vm" or "container" | ||
| @@ -85,6 +97,16 @@ message JobSpec { | |||||
| 85 | // job's repository and refs. | 97 | // job's repository and refs. | ||
| 86 | uint32 cache_port = 15; | 98 | uint32 cache_port = 15; | ||
| 87 | string cache_token = 16; | 99 | string cache_token = 16; | ||
| 100 | + | // Job.secrets and Job.vars are not here: they come in NamedValue frames | |||
| 101 | + | // before Run (see NamedValue). The executor takes the vars of those frames | |||
| 102 | + | // for the "vars" context when contexts has none. | |||
| 103 | + | reserved 17, 18; | |||
| 104 | + | reserved "secrets", "vars"; | |||
| 105 | + | // secrets_withheld is set by the host when the server sends Job.trusted and | |||
| 106 | + | // the job is not trusted (feature ci.secrets): "Set up job" then says that | |||
| 107 | + | // secrets are not available. Never set for servers without the feature, for | |||
| 108 | + | // which an absent Job.trusted only means "not sent". | |||
| 109 | + | bool secrets_withheld = 19; | |||
| 88 | } | 110 | } | ||
| 89 | 111 | ||||
| 90 | enum StreamKind { | 112 | enum StreamKind { | ||
M internal/execstream/execpb/exec.pb.go +219 -106 generated file: collapsed
M internal/execstream/frame.go +27 -0
| @@ -18,6 +18,8 @@ import ( | |||
| 18 | 18 | "errors" | |
| 19 | 19 | "fmt" | |
| 20 | 20 | "io" | |
| 21 | + | "maps" | |
| 22 | + | "slices" | |
| 21 | 23 | "sync" | |
| 22 | 24 | "time" | |
| 23 | 25 | ||
| @@ -167,6 +169,10 @@ type JobHandler struct { | |||
| 167 | 169 | // step. | |
| 168 | 170 | StepAnnotations func(*execpb.StepAnnotations) | |
| 169 | 171 | StepSummary func(*execpb.StepSummary) | |
| 172 | + | // Secrets and Vars (Job.secrets, Job.vars) are sent before the Run frame, | |
| 173 | + | // one NamedValue frame each, so that their total size is not bound by the | |
| 174 | + | // size of one frame (MaxFrame). A single value must fit in a frame. | |
| 175 | + | Secrets, Vars map[string]string | |
| 170 | 176 | } | |
| 171 | 177 | ||
| 172 | 178 | // DriveJob is Drive for a session that carries a job: besides Output frames it | |
| @@ -184,6 +190,9 @@ func DriveJob(rw io.ReadWriter, run *execpb.Run, cancel <-chan *execpb.Cancel, h | |||
| 184 | 190 | if h.Hello != nil { | |
| 185 | 191 | h.Hello(hello) | |
| 186 | 192 | } | |
| 193 | + | if err := sendValues(c, h); err != nil { | |
| 194 | + | return hello, nil, err | |
| 195 | + | } | |
| 187 | 196 | if err := c.Send(&execpb.Frame{Body: &execpb.Frame_Run{Run: run}}); err != nil { | |
| 188 | 197 | return hello, nil, fmt.Errorf("execstream: sending run: %w", err) | |
| 189 | 198 | } | |
| @@ -279,3 +288,21 @@ func DriveJob(rw io.ReadWriter, run *execpb.Run, cancel <-chan *execpb.Cancel, h | |||
| 279 | 288 | } | |
| 280 | 289 | } | |
| 281 | 290 | } | |
| 291 | + | ||
| 292 | + | // sendValues sends the secrets, then the variables of the job, in name order, | |
| 293 | + | // one frame each. The error never holds a value. | |
| 294 | + | func sendValues(c *Conn, h JobHandler) error { | |
| 295 | + | for _, k := range slices.Sorted(maps.Keys(h.Secrets)) { | |
| 296 | + | f := &execpb.Frame{Body: &execpb.Frame_Secret{Secret: &execpb.NamedValue{Name: k, Value: h.Secrets[k]}}} | |
| 297 | + | if err := c.Send(f); err != nil { | |
| 298 | + | return fmt.Errorf("execstream: sending secret %q: %w", k, err) | |
| 299 | + | } | |
| 300 | + | } | |
| 301 | + | for _, k := range slices.Sorted(maps.Keys(h.Vars)) { | |
| 302 | + | f := &execpb.Frame{Body: &execpb.Frame_Var{Var: &execpb.NamedValue{Name: k, Value: h.Vars[k]}}} | |
| 303 | + | if err := c.Send(f); err != nil { | |
| 304 | + | return fmt.Errorf("execstream: sending variable %q: %w", k, err) | |
| 305 | + | } | |
| 306 | + | } | |
| 307 | + | return nil | |
| 308 | + | } | |
| @@ -18,6 +18,8 @@ import ( | |||||
| 18 | "errors" | 18 | "errors" | ||
| 19 | "fmt" | 19 | "fmt" | ||
| 20 | "io" | 20 | "io" | ||
| 21 | + | "maps" | |||
| 22 | + | "slices" | |||
| 21 | "sync" | 23 | "sync" | ||
| 22 | "time" | 24 | "time" | ||
| 23 | 25 | ||||
| @@ -167,6 +169,10 @@ type JobHandler struct { | |||||
| 167 | // step. | 169 | // step. | ||
| 168 | StepAnnotations func(*execpb.StepAnnotations) | 170 | StepAnnotations func(*execpb.StepAnnotations) | ||
| 169 | StepSummary func(*execpb.StepSummary) | 171 | StepSummary func(*execpb.StepSummary) | ||
| 172 | + | // Secrets and Vars (Job.secrets, Job.vars) are sent before the Run frame, | |||
| 173 | + | // one NamedValue frame each, so that their total size is not bound by the | |||
| 174 | + | // size of one frame (MaxFrame). A single value must fit in a frame. | |||
| 175 | + | Secrets, Vars map[string]string | |||
| 170 | } | 176 | } | ||
| 171 | 177 | ||||
| 172 | // DriveJob is Drive for a session that carries a job: besides Output frames it | 178 | // DriveJob is Drive for a session that carries a job: besides Output frames it | ||
| @@ -184,6 +190,9 @@ func DriveJob(rw io.ReadWriter, run *execpb.Run, cancel <-chan *execpb.Cancel, h | |||||
| 184 | if h.Hello != nil { | 190 | if h.Hello != nil { | ||
| 185 | h.Hello(hello) | 191 | h.Hello(hello) | ||
| 186 | } | 192 | } | ||
| 193 | + | if err := sendValues(c, h); err != nil { | |||
| 194 | + | return hello, nil, err | |||
| 195 | + | } | |||
| 187 | if err := c.Send(&execpb.Frame{Body: &execpb.Frame_Run{Run: run}}); err != nil { | 196 | if err := c.Send(&execpb.Frame{Body: &execpb.Frame_Run{Run: run}}); err != nil { | ||
| 188 | return hello, nil, fmt.Errorf("execstream: sending run: %w", err) | 197 | return hello, nil, fmt.Errorf("execstream: sending run: %w", err) | ||
| 189 | } | 198 | } | ||
| @@ -279,3 +288,21 @@ func DriveJob(rw io.ReadWriter, run *execpb.Run, cancel <-chan *execpb.Cancel, h | |||||
| 279 | } | 288 | } | ||
| 280 | } | 289 | } | ||
| 281 | } | 290 | } | ||
| 291 | + | ||||
| 292 | + | // sendValues sends the secrets, then the variables of the job, in name order, | |||
| 293 | + | // one frame each. The error never holds a value. | |||
| 294 | + | func sendValues(c *Conn, h JobHandler) error { | |||
| 295 | + | for _, k := range slices.Sorted(maps.Keys(h.Secrets)) { | |||
| 296 | + | f := &execpb.Frame{Body: &execpb.Frame_Secret{Secret: &execpb.NamedValue{Name: k, Value: h.Secrets[k]}}} | |||
| 297 | + | if err := c.Send(f); err != nil { | |||
| 298 | + | return fmt.Errorf("execstream: sending secret %q: %w", k, err) | |||
| 299 | + | } | |||
| 300 | + | } | |||
| 301 | + | for _, k := range slices.Sorted(maps.Keys(h.Vars)) { | |||
| 302 | + | f := &execpb.Frame{Body: &execpb.Frame_Var{Var: &execpb.NamedValue{Name: k, Value: h.Vars[k]}}} | |||
| 303 | + | if err := c.Send(f); err != nil { | |||
| 304 | + | return fmt.Errorf("execstream: sending variable %q: %w", k, err) | |||
| 305 | + | } | |||
| 306 | + | } | |||
| 307 | + | return nil | |||
| 308 | + | } | |||
M internal/executor/containers_stop.go +2 -2
| @@ -121,7 +121,7 @@ func (j *job) fallbackCleanup() { | |||
| 121 | 121 | return | |
| 122 | 122 | } | |
| 123 | 123 | for _, e := range j.cleanupContainers(nil) { | |
| 124 | - | j.cfg.Log.Warn().Err(e).Msg("fallback cleanup of the job containers") | |
| 124 | + | j.diagWarn(e, "fallback cleanup of the job containers") | |
| 125 | 125 | } | |
| 126 | 126 | } | |
| 127 | 127 | ||
| @@ -273,7 +273,7 @@ func (j *job) warnTo(s *stepRun, msg string) { | |||
| 273 | 273 | j.tagged(s.num, "##[warning]", msg) | |
| 274 | 274 | return | |
| 275 | 275 | } | |
| 276 | - | j.cfg.Log.Warn().Msg(msg) | |
| 276 | + | j.diagWarn(nil, msg) | |
| 277 | 277 | } | |
| 278 | 278 | ||
| 279 | 279 | // absent reports whether docker's answer to a removal says the object does not | |
| @@ -121,7 +121,7 @@ func (j *job) fallbackCleanup() { | |||||
| 121 | return | 121 | return | ||
| 122 | } | 122 | } | ||
| 123 | for _, e := range j.cleanupContainers(nil) { | 123 | for _, e := range j.cleanupContainers(nil) { | ||
| 124 | - | j.cfg.Log.Warn().Err(e).Msg("fallback cleanup of the job containers") | 124 | + | j.diagWarn(e, "fallback cleanup of the job containers") |
| 125 | } | 125 | } | ||
| 126 | } | 126 | } | ||
| 127 | 127 | ||||
| @@ -273,7 +273,7 @@ func (j *job) warnTo(s *stepRun, msg string) { | |||||
| 273 | j.tagged(s.num, "##[warning]", msg) | 273 | j.tagged(s.num, "##[warning]", msg) | ||
| 274 | return | 274 | return | ||
| 275 | } | 275 | } | ||
| 276 | - | j.cfg.Log.Warn().Msg(msg) | 276 | + | j.diagWarn(nil, msg) |
| 277 | } | 277 | } | ||
| 278 | 278 | ||||
| 279 | // absent reports whether docker's answer to a removal says the object does not | 279 | // absent reports whether docker's answer to a removal says the object does not | ||
M internal/executor/contexts.go +54 -11
| @@ -5,6 +5,7 @@ package executor | |||
| 5 | 5 | import ( | |
| 6 | 6 | "encoding/base64" | |
| 7 | 7 | "fmt" | |
| 8 | + | "maps" | |
| 8 | 9 | "path/filepath" | |
| 9 | 10 | "runtime" | |
| 10 | 11 | "slices" | |
| @@ -228,6 +229,23 @@ type State struct { | |||
| 228 | 229 | prepend []string | |
| 229 | 230 | // status is job.status: "success", "failure" or "cancelled". | |
| 230 | 231 | status string | |
| 232 | + | // jobSecrets are Job.secrets: the secrets context besides GITHUB_TOKEN. | |
| 233 | + | // They live in memory only (never in a file, a log or an error). | |
| 234 | + | jobSecrets map[string]string | |
| 235 | + | } | |
| 236 | + | ||
| 237 | + | // WithJobData gives the state the secrets and variables of the job message | |
| 238 | + | // (Job.secrets and Job.vars). The vars context keeps what the server wrote in | |
| 239 | + | // the contexts JSON; the variables are its fallback when the JSON has none. | |
| 240 | + | func (s *State) WithJobData(secrets, vars map[string]string) { | |
| 241 | + | s.jobSecrets = secrets | |
| 242 | + | if d, ok := s.other["vars"].(*expr.Dictionary); (!ok || d.Len() == 0) && len(vars) > 0 { | |
| 243 | + | nd := expr.NewDictionary() | |
| 244 | + | for _, k := range slices.Sorted(maps.Keys(vars)) { | |
| 245 | + | nd.Add(k, expr.String(vars[k])) | |
| 246 | + | } | |
| 247 | + | s.other["vars"] = nd | |
| 248 | + | } | |
| 231 | 249 | } | |
| 232 | 250 | ||
| 233 | 251 | // serverContexts are the keys of the contexts JSON that the runner reads. | |
| @@ -493,32 +511,38 @@ func (s *State) Values(env *ordered) map[string]expr.Value { | |||
| 493 | 511 | return m | |
| 494 | 512 | } | |
| 495 | 513 | ||
| 496 | - | // secrets is the secrets context. Today it holds one entry, GITHUB_TOKEN, as | |
| 497 | - | // on GitHub (Variables.ToSecretsContext lists the secret variables but the | |
| 498 | - | // access token; GitHub's message carries the token as the secret GITHUB_TOKEN, | |
| 499 | - | // recorded: ctx-inputs-vars, secrets.GITHUB_TOKEN == github.token). | |
| 514 | + | // secrets is the secrets context: GITHUB_TOKEN and the secrets of the job | |
| 515 | + | // (Job.secrets), as on GitHub where the message carries the token as the secret | |
| 516 | + | // GITHUB_TOKEN (recorded: ctx-inputs-vars, secrets.GITHUB_TOKEN == github.token). | |
| 517 | + | // An unknown secret evaluates to "" like on GitHub. | |
| 500 | 518 | // | |
| 501 | - | // Where the value comes from: the "token" key of the github context that the | |
| 519 | + | // GITHUB_TOKEN comes from the "token" key of the github context that the | |
| 502 | 520 | // server sends in Job.contexts, the same value as github.token. It must be a | |
| 503 | 521 | // token of its own, scoped to the job (the permissions "Set up job" prints) and | |
| 504 | 522 | // short lived. It must never be the "ermitejob_" job token: that one | |
| 505 | 523 | // authenticates the runner's calls to ermite, stays in the runner host process | |
| 506 | - | // and must not reach the sandbox (runner.proto, "Secrets and trust"). ermite | |
| 507 | - | // does not send a token yet (planned for CI-3, with the secrets of the | |
| 508 | - | // repository); until it does, github.token is empty, so is secrets, and | |
| 509 | - | // a test of secrets.GITHUB_TOKEN against the empty string is false. Without a | |
| 510 | - | // token nothing is masked. | |
| 524 | + | // and must not reach the sandbox (runner.proto, "Secrets and trust"). Without a | |
| 525 | + | // token there is no GITHUB_TOKEN entry, and a test of secrets.GITHUB_TOKEN | |
| 526 | + | // against the empty string is false. GITHUB_TOKEN is reserved: an entry of | |
| 527 | + | // that name in Job.secrets is ignored. | |
| 511 | 528 | func (s *State) secrets() *expr.Dictionary { | |
| 512 | 529 | d := expr.NewDictionary() | |
| 513 | 530 | if t := s.GitHub("token"); t != "" { | |
| 514 | 531 | d.Add("GITHUB_TOKEN", expr.String(t)) | |
| 515 | 532 | } | |
| 533 | + | for _, k := range slices.Sorted(maps.Keys(s.jobSecrets)) { | |
| 534 | + | if strings.EqualFold(k, "GITHUB_TOKEN") { | |
| 535 | + | continue | |
| 536 | + | } | |
| 537 | + | d.Add(k, expr.String(s.jobSecrets[k])) | |
| 538 | + | } | |
| 516 | 539 | return d | |
| 517 | 540 | } | |
| 518 | 541 | ||
| 519 | 542 | // Secrets returns the values that must never leave the job: the token and the | |
| 520 | 543 | // base64 of "x-access-token:<token>" (ExecutionContext.cs:1026-1028); newMasker | |
| 521 | - | // adds their other encodings. | |
| 544 | + | // adds their other encodings. The secrets of the job are registered with | |
| 545 | + | // registerSecrets. | |
| 522 | 546 | func (s *State) Secrets() []string { | |
| 523 | 547 | t := s.GitHub("token") | |
| 524 | 548 | if t == "" { | |
| @@ -527,6 +551,25 @@ func (s *State) Secrets() []string { | |||
| 527 | 551 | return []string{t, base64.StdEncoding.EncodeToString([]byte("x-access-token:" + t))} | |
| 528 | 552 | } | |
| 529 | 553 | ||
| 554 | + | // registerSecrets registers every secret of the job as the runner does before | |
| 555 | + | // the first step (Worker.cs:158-171): the whole value trimmed, which may span | |
| 556 | + | // several lines because individual trace info can contain line breaks, then | |
| 557 | + | // each non-empty trimmed line of it, split on CR and LF, because child | |
| 558 | + | // processes are processed line by line. add ignores empty values and adds the | |
| 559 | + | // encoded forms. GITHUB_TOKEN is not a job secret (see State.secrets). | |
| 560 | + | func registerSecrets(m *masker, secrets map[string]string) { | |
| 561 | + | for _, k := range slices.Sorted(maps.Keys(secrets)) { | |
| 562 | + | if strings.EqualFold(k, "GITHUB_TOKEN") { | |
| 563 | + | continue | |
| 564 | + | } | |
| 565 | + | v := strings.TrimSpace(secrets[k]) | |
| 566 | + | m.add(v) | |
| 567 | + | for _, line := range strings.FieldsFunc(v, func(r rune) bool { return r == '\r' || r == '\n' }) { | |
| 568 | + | m.add(strings.TrimSpace(line)) | |
| 569 | + | } | |
| 570 | + | } | |
| 571 | + | } | |
| 572 | + | ||
| 530 | 573 | // ServerContexts returns the contexts that the server has when it evaluates | |
| 531 | 574 | // the name of a job: github as received (without the keys the runner adds), | |
| 532 | 575 | // needs, strategy, matrix, vars and inputs. | |
| @@ -5,6 +5,7 @@ package executor | |||||
| 5 | import ( | 5 | import ( | ||
| 6 | "encoding/base64" | 6 | "encoding/base64" | ||
| 7 | "fmt" | 7 | "fmt" | ||
| 8 | + | "maps" | |||
| 8 | "path/filepath" | 9 | "path/filepath" | ||
| 9 | "runtime" | 10 | "runtime" | ||
| 10 | "slices" | 11 | "slices" | ||
| @@ -228,6 +229,23 @@ type State struct { | |||||
| 228 | prepend []string | 229 | prepend []string | ||
| 229 | // status is job.status: "success", "failure" or "cancelled". | 230 | // status is job.status: "success", "failure" or "cancelled". | ||
| 230 | status string | 231 | status string | ||
| 232 | + | // jobSecrets are Job.secrets: the secrets context besides GITHUB_TOKEN. | |||
| 233 | + | // They live in memory only (never in a file, a log or an error). | |||
| 234 | + | jobSecrets map[string]string | |||
| 235 | + | } | |||
| 236 | + | ||||
| 237 | + | // WithJobData gives the state the secrets and variables of the job message | |||
| 238 | + | // (Job.secrets and Job.vars). The vars context keeps what the server wrote in | |||
| 239 | + | // the contexts JSON; the variables are its fallback when the JSON has none. | |||
| 240 | + | func (s *State) WithJobData(secrets, vars map[string]string) { | |||
| 241 | + | s.jobSecrets = secrets | |||
| 242 | + | if d, ok := s.other["vars"].(*expr.Dictionary); (!ok || d.Len() == 0) && len(vars) > 0 { | |||
| 243 | + | nd := expr.NewDictionary() | |||
| 244 | + | for _, k := range slices.Sorted(maps.Keys(vars)) { | |||
| 245 | + | nd.Add(k, expr.String(vars[k])) | |||
| 246 | + | } | |||
| 247 | + | s.other["vars"] = nd | |||
| 248 | + | } | |||
| 231 | } | 249 | } | ||
| 232 | 250 | ||||
| 233 | // serverContexts are the keys of the contexts JSON that the runner reads. | 251 | // serverContexts are the keys of the contexts JSON that the runner reads. | ||
| @@ -493,32 +511,38 @@ func (s *State) Values(env *ordered) map[string]expr.Value { | |||||
| 493 | return m | 511 | return m | ||
| 494 | } | 512 | } | ||
| 495 | 513 | ||||
| 496 | - | // secrets is the secrets context. Today it holds one entry, GITHUB_TOKEN, as | 514 | + | // secrets is the secrets context: GITHUB_TOKEN and the secrets of the job |
| 497 | - | // on GitHub (Variables.ToSecretsContext lists the secret variables but the | 515 | + | // (Job.secrets), as on GitHub where the message carries the token as the secret |
| 498 | - | // access token; GitHub's message carries the token as the secret GITHUB_TOKEN, | 516 | + | // GITHUB_TOKEN (recorded: ctx-inputs-vars, secrets.GITHUB_TOKEN == github.token). |
| 499 | - | // recorded: ctx-inputs-vars, secrets.GITHUB_TOKEN == github.token). | 517 | + | // An unknown secret evaluates to "" like on GitHub. |
| 500 | // | 518 | // | ||
| 501 | - | // Where the value comes from: the "token" key of the github context that the | 519 | + | // GITHUB_TOKEN comes from the "token" key of the github context that the |
| 502 | // server sends in Job.contexts, the same value as github.token. It must be a | 520 | // server sends in Job.contexts, the same value as github.token. It must be a | ||
| 503 | // token of its own, scoped to the job (the permissions "Set up job" prints) and | 521 | // token of its own, scoped to the job (the permissions "Set up job" prints) and | ||
| 504 | // short lived. It must never be the "ermitejob_" job token: that one | 522 | // short lived. It must never be the "ermitejob_" job token: that one | ||
| 505 | // authenticates the runner's calls to ermite, stays in the runner host process | 523 | // authenticates the runner's calls to ermite, stays in the runner host process | ||
| 506 | - | // and must not reach the sandbox (runner.proto, "Secrets and trust"). ermite | 524 | + | // and must not reach the sandbox (runner.proto, "Secrets and trust"). Without a |
| 507 | - | // does not send a token yet (planned for CI-3, with the secrets of the | 525 | + | // token there is no GITHUB_TOKEN entry, and a test of secrets.GITHUB_TOKEN |
| 508 | - | // repository); until it does, github.token is empty, so is secrets, and | 526 | + | // against the empty string is false. GITHUB_TOKEN is reserved: an entry of |
| 509 | - | // a test of secrets.GITHUB_TOKEN against the empty string is false. Without a | 527 | + | // that name in Job.secrets is ignored. |
| 510 | - | // token nothing is masked. | |||
| 511 | func (s *State) secrets() *expr.Dictionary { | 528 | func (s *State) secrets() *expr.Dictionary { | ||
| 512 | d := expr.NewDictionary() | 529 | d := expr.NewDictionary() | ||
| 513 | if t := s.GitHub("token"); t != "" { | 530 | if t := s.GitHub("token"); t != "" { | ||
| 514 | d.Add("GITHUB_TOKEN", expr.String(t)) | 531 | d.Add("GITHUB_TOKEN", expr.String(t)) | ||
| 515 | } | 532 | } | ||
| 533 | + | for _, k := range slices.Sorted(maps.Keys(s.jobSecrets)) { | |||
| 534 | + | if strings.EqualFold(k, "GITHUB_TOKEN") { | |||
| 535 | + | continue | |||
| 536 | + | } | |||
| 537 | + | d.Add(k, expr.String(s.jobSecrets[k])) | |||
| 538 | + | } | |||
| 516 | return d | 539 | return d | ||
| 517 | } | 540 | } | ||
| 518 | 541 | ||||
| 519 | // Secrets returns the values that must never leave the job: the token and the | 542 | // Secrets returns the values that must never leave the job: the token and the | ||
| 520 | // base64 of "x-access-token:<token>" (ExecutionContext.cs:1026-1028); newMasker | 543 | // base64 of "x-access-token:<token>" (ExecutionContext.cs:1026-1028); newMasker | ||
| 521 | - | // adds their other encodings. | 544 | + | // adds their other encodings. The secrets of the job are registered with |
| 545 | + | // registerSecrets. | |||
| 522 | func (s *State) Secrets() []string { | 546 | func (s *State) Secrets() []string { | ||
| 523 | t := s.GitHub("token") | 547 | t := s.GitHub("token") | ||
| 524 | if t == "" { | 548 | if t == "" { | ||
| @@ -527,6 +551,25 @@ func (s *State) Secrets() []string { | |||||
| 527 | return []string{t, base64.StdEncoding.EncodeToString([]byte("x-access-token:" + t))} | 551 | return []string{t, base64.StdEncoding.EncodeToString([]byte("x-access-token:" + t))} | ||
| 528 | } | 552 | } | ||
| 529 | 553 | ||||
| 554 | + | // registerSecrets registers every secret of the job as the runner does before | |||
| 555 | + | // the first step (Worker.cs:158-171): the whole value trimmed, which may span | |||
| 556 | + | // several lines because individual trace info can contain line breaks, then | |||
| 557 | + | // each non-empty trimmed line of it, split on CR and LF, because child | |||
| 558 | + | // processes are processed line by line. add ignores empty values and adds the | |||
| 559 | + | // encoded forms. GITHUB_TOKEN is not a job secret (see State.secrets). | |||
| 560 | + | func registerSecrets(m *masker, secrets map[string]string) { | |||
| 561 | + | for _, k := range slices.Sorted(maps.Keys(secrets)) { | |||
| 562 | + | if strings.EqualFold(k, "GITHUB_TOKEN") { | |||
| 563 | + | continue | |||
| 564 | + | } | |||
| 565 | + | v := strings.TrimSpace(secrets[k]) | |||
| 566 | + | m.add(v) | |||
| 567 | + | for _, line := range strings.FieldsFunc(v, func(r rune) bool { return r == '\r' || r == '\n' }) { | |||
| 568 | + | m.add(strings.TrimSpace(line)) | |||
| 569 | + | } | |||
| 570 | + | } | |||
| 571 | + | } | |||
| 572 | + | ||||
| 530 | // ServerContexts returns the contexts that the server has when it evaluates | 573 | // ServerContexts returns the contexts that the server has when it evaluates | ||
| 531 | // the name of a job: github as received (without the keys the runner adds), | 574 | // the name of a job: github as received (without the keys the runner adds), | ||
| 532 | // needs, strategy, matrix, vars and inputs. | 575 | // needs, strategy, matrix, vars and inputs. | ||
M internal/executor/hashfiles.go +1 -1
| @@ -68,7 +68,7 @@ func (j *job) hashFilesFunc() expr.Func { | |||
| 68 | 68 | //nolint:staticcheck // GitHub's text, with its final period | |
| 69 | 69 | return nil, fmt.Errorf("hashFiles('%s') couldn't finish within %d seconds.", desc, int(hashFilesTimeout/time.Second)) | |
| 70 | 70 | case err != nil: | |
| 71 | - | j.cfg.Log.Debug().Err(err).Msg("hashFiles failed") | |
| 71 | + | j.diagDebug(err, "hashFiles failed") | |
| 72 | 72 | return nil, fmt.Errorf("hashFiles('%s') failed. Fail to hash files under directory '%s'", desc, ws) | |
| 73 | 73 | } | |
| 74 | 74 | return expr.String(sum), nil | |
| @@ -68,7 +68,7 @@ func (j *job) hashFilesFunc() expr.Func { | |||||
| 68 | //nolint:staticcheck // GitHub's text, with its final period | 68 | //nolint:staticcheck // GitHub's text, with its final period | ||
| 69 | return nil, fmt.Errorf("hashFiles('%s') couldn't finish within %d seconds.", desc, int(hashFilesTimeout/time.Second)) | 69 | return nil, fmt.Errorf("hashFiles('%s') couldn't finish within %d seconds.", desc, int(hashFilesTimeout/time.Second)) | ||
| 70 | case err != nil: | 70 | case err != nil: | ||
| 71 | - | j.cfg.Log.Debug().Err(err).Msg("hashFiles failed") | 71 | + | j.diagDebug(err, "hashFiles failed") |
| 72 | return nil, fmt.Errorf("hashFiles('%s') failed. Fail to hash files under directory '%s'", desc, ws) | 72 | return nil, fmt.Errorf("hashFiles('%s') failed. Fail to hash files under directory '%s'", desc, ws) | ||
| 73 | } | 73 | } | ||
| 74 | return expr.String(sum), nil | 74 | return expr.String(sum), nil | ||
M internal/executor/job.go +59 -4
| @@ -78,6 +78,13 @@ type Config struct { | |||
| 78 | 78 | // ACTIONS_RUNTIME_TOKEN of the node and docker actions of the job: the cache | |
| 79 | 79 | // service of the runner. The token is masked in the log. | |
| 80 | 80 | CacheURL, CacheToken string | |
| 81 | + | // Secrets and Vars are Job.secrets and Job.vars. Secrets are registered in | |
| 82 | + | // the masker before the first step and form the secrets context; they are | |
| 83 | + | // never logged. Vars are the fallback of the vars context. | |
| 84 | + | Secrets, Vars map[string]string | |
| 85 | + | // SecretsWithheld says that the server did not send secrets because the run | |
| 86 | + | // is not trusted: "Set up job" says so. | |
| 87 | + | SecretsWithheld bool | |
| 81 | 88 | // Debug turns ##[debug] lines on (ACTIONS_STEP_DEBUG). | |
| 82 | 89 | Debug bool | |
| 83 | 90 | // Actions configures the actions of the job (resolution, downloads, Node.js, | |
| @@ -163,8 +170,8 @@ type job struct { | |||
| 163 | 170 | baseEnv []string | |
| 164 | 171 | lookupPath string | |
| 165 | 172 | before map[string]bool | |
| 166 | - | // masker hides the secrets in everything reported; it is set once the | |
| 167 | - | // contexts are built, before any step runs. | |
| 173 | + | // masker hides the secrets in everything reported; Run sets it, with the | |
| 174 | + | // secrets of the job, before the first report. | |
| 168 | 175 | masker atomic.Pointer[masker] | |
| 169 | 176 | // actions holds what the pre-job phase loaded (see prepareActions). | |
| 170 | 177 | actions *actionState | |
| @@ -188,6 +195,23 @@ type job struct { | |||
| 188 | 195 | ||
| 189 | 196 | func (j *job) mask(s string) string { return j.masker.Load().mask(s) } | |
| 190 | 197 | ||
| 198 | + | // The runner's own diagnostics (Config.Log) bypass the reporter, hence the | |
| 199 | + | // masker: a path, a docker message or an error can hold a secret. Every | |
| 200 | + | // diagnostic of the executor goes through these helpers, which mask the | |
| 201 | + | // rendered error, message and fields (TestDiagnosticsAreMasked checks that no | |
| 202 | + | // other code calls Config.Log). | |
| 203 | + | ||
| 204 | + | // diagWarn logs a warning with an optional error and string fields (key, | |
| 205 | + | // value, ...). | |
| 206 | + | func (j *job) diagWarn(err error, msg string, kv ...string) { | |
| 207 | + | j.diag(false, err, msg, kv) | |
| 208 | + | } | |
| 209 | + | ||
| 210 | + | // diagDebug is diagWarn at debug level. | |
| 211 | + | func (j *job) diagDebug(err error, msg string, kv ...string) { | |
| 212 | + | j.diag(true, err, msg, kv) | |
| 213 | + | } | |
| 214 | + | ||
| 191 | 215 | func (j *job) now() time.Time { | |
| 192 | 216 | if j.cfg.Now != nil { | |
| 193 | 217 | return j.cfg.Now() | |
| @@ -202,6 +226,13 @@ func Run(cancel *Canceller, cfg Config) Result { | |||
| 202 | 226 | cancel = NewCanceller() | |
| 203 | 227 | } | |
| 204 | 228 | j := &job{cfg: cfg, cancel: cancel, trackID: "github_" + uuid.NewString(), matchers: &matcherRoot{}} | |
| 229 | + | // The masker exists before anything is reported, with the secrets of the job | |
| 230 | + | // registered (Worker.cs:158-171 does it before the job starts); the token and | |
| 231 | + | // the cache token join it once the contexts are built. GitHub's SecretMasker | |
| 232 | + | // has no minimum length (SecretMasker.AddValue ignores only null or empty | |
| 233 | + | // values), so a one-character secret is masked too. | |
| 234 | + | j.masker.Store(newMasker()) | |
| 235 | + | registerSecrets(j.masker.Load(), cfg.Secrets) | |
| 205 | 236 | j.cfg.Reporter = &maskReporter{j: j, next: cfg.Reporter} | |
| 206 | 237 | defer func() { | |
| 207 | 238 | if j.fc != nil { | |
| @@ -450,8 +481,11 @@ func (j *job) setUp() ([]qstep, error) { | |||
| 450 | 481 | layout = NewLayout(home, filepath.Base(layout.Workspace), cfg.ToolCache) | |
| 451 | 482 | state.WithLayout(layout) | |
| 452 | 483 | } | |
| 484 | + | state.WithJobData(cfg.Secrets, cfg.Vars) | |
| 453 | 485 | j.state = state | |
| 454 | - | j.masker.Store(newMasker(state.Secrets()...)) | |
| 486 | + | for _, v := range state.Secrets() { | |
| 487 | + | j.masker.Load().add(v) | |
| 488 | + | } | |
| 455 | 489 | j.masker.Load().add(cfg.CacheToken) | |
| 456 | 490 | ||
| 457 | 491 | // Features that arrive in later phases fail the job with a clear message. | |
| @@ -687,6 +721,10 @@ const ( | |||
| 687 | 721 | defaultCacheMode = "write" | |
| 688 | 722 | ) | |
| 689 | 723 | ||
| 724 | + | // SecretsWithheldLine is the line of "Set up job" for a run whose secrets the | |
| 725 | + | // server withheld (an ermite text: GitHub has none for fork pull requests). | |
| 726 | + | const SecretsWithheldLine = "Secrets are not available to this job: the run was started by a limited token." | |
| 727 | + | ||
| 690 | 728 | // printTokenInfo prints the GITHUB_TOKEN permissions group, the secret source | |
| 691 | 729 | // and the cache mode. | |
| 692 | 730 | // | |
| @@ -709,6 +747,9 @@ func (j *job) printTokenInfo() { | |||
| 709 | 747 | j.state.SetGitHub("secret_source", src) | |
| 710 | 748 | } | |
| 711 | 749 | j.out(1, "Secret source: "+src) | |
| 750 | + | if j.cfg.SecretsWithheld { | |
| 751 | + | j.out(1, SecretsWithheldLine) | |
| 752 | + | } | |
| 712 | 753 | j.out(1, "Cache mode: "+defaultCacheMode) | |
| 713 | 754 | } | |
| 714 | 755 | ||
| @@ -723,7 +764,7 @@ func (j *job) printTokenInfo() { | |||
| 723 | 764 | func (j *job) jobDisplayName() string { | |
| 724 | 765 | ctx := context.Background() | |
| 725 | 766 | fallback := func(why string, err error) string { | |
| 726 | - | j.cfg.Log.Warn().Err(err).Str("job_key", j.cfg.JobKey).Msg("job display name: " + why + ", using the job key") | |
| 767 | + | j.diagWarn(err, "job display name: "+why+", using the job key", "job_key", j.cfg.JobKey) | |
| 727 | 768 | return j.cfg.JobKey | |
| 728 | 769 | } | |
| 729 | 770 | j.mu.Lock() | |
| @@ -796,3 +837,17 @@ func sameMatrix(a, b *expr.Dictionary) bool { | |||
| 796 | 837 | } | |
| 797 | 838 | return true | |
| 798 | 839 | } | |
| 840 | + | ||
| 841 | + | func (j *job) diag(debug bool, err error, msg string, kv []string) { | |
| 842 | + | ev := j.cfg.Log.Warn() | |
| 843 | + | if debug { | |
| 844 | + | ev = j.cfg.Log.Debug() | |
| 845 | + | } | |
| 846 | + | if err != nil { | |
| 847 | + | ev = ev.Str("error", j.mask(err.Error())) | |
| 848 | + | } | |
| 849 | + | for i := 0; i+1 < len(kv); i += 2 { | |
| 850 | + | ev = ev.Str(kv[i], j.mask(kv[i+1])) | |
| 851 | + | } | |
| 852 | + | ev.Msg(j.mask(msg)) | |
| 853 | + | } | |
| @@ -78,6 +78,13 @@ type Config struct { | |||||
| 78 | // ACTIONS_RUNTIME_TOKEN of the node and docker actions of the job: the cache | 78 | // ACTIONS_RUNTIME_TOKEN of the node and docker actions of the job: the cache | ||
| 79 | // service of the runner. The token is masked in the log. | 79 | // service of the runner. The token is masked in the log. | ||
| 80 | CacheURL, CacheToken string | 80 | CacheURL, CacheToken string | ||
| 81 | + | // Secrets and Vars are Job.secrets and Job.vars. Secrets are registered in | |||
| 82 | + | // the masker before the first step and form the secrets context; they are | |||
| 83 | + | // never logged. Vars are the fallback of the vars context. | |||
| 84 | + | Secrets, Vars map[string]string | |||
| 85 | + | // SecretsWithheld says that the server did not send secrets because the run | |||
| 86 | + | // is not trusted: "Set up job" says so. | |||
| 87 | + | SecretsWithheld bool | |||
| 81 | // Debug turns ##[debug] lines on (ACTIONS_STEP_DEBUG). | 88 | // Debug turns ##[debug] lines on (ACTIONS_STEP_DEBUG). | ||
| 82 | Debug bool | 89 | Debug bool | ||
| 83 | // Actions configures the actions of the job (resolution, downloads, Node.js, | 90 | // Actions configures the actions of the job (resolution, downloads, Node.js, | ||
| @@ -163,8 +170,8 @@ type job struct { | |||||
| 163 | baseEnv []string | 170 | baseEnv []string | ||
| 164 | lookupPath string | 171 | lookupPath string | ||
| 165 | before map[string]bool | 172 | before map[string]bool | ||
| 166 | - | // masker hides the secrets in everything reported; it is set once the | 173 | + | // masker hides the secrets in everything reported; Run sets it, with the |
| 167 | - | // contexts are built, before any step runs. | 174 | + | // secrets of the job, before the first report. |
| 168 | masker atomic.Pointer[masker] | 175 | masker atomic.Pointer[masker] | ||
| 169 | // actions holds what the pre-job phase loaded (see prepareActions). | 176 | // actions holds what the pre-job phase loaded (see prepareActions). | ||
| 170 | actions *actionState | 177 | actions *actionState | ||
| @@ -188,6 +195,23 @@ type job struct { | |||||
| 188 | 195 | ||||
| 189 | func (j *job) mask(s string) string { return j.masker.Load().mask(s) } | 196 | func (j *job) mask(s string) string { return j.masker.Load().mask(s) } | ||
| 190 | 197 | ||||
| 198 | + | // The runner's own diagnostics (Config.Log) bypass the reporter, hence the | |||
| 199 | + | // masker: a path, a docker message or an error can hold a secret. Every | |||
| 200 | + | // diagnostic of the executor goes through these helpers, which mask the | |||
| 201 | + | // rendered error, message and fields (TestDiagnosticsAreMasked checks that no | |||
| 202 | + | // other code calls Config.Log). | |||
| 203 | + | ||||
| 204 | + | // diagWarn logs a warning with an optional error and string fields (key, | |||
| 205 | + | // value, ...). | |||
| 206 | + | func (j *job) diagWarn(err error, msg string, kv ...string) { | |||
| 207 | + | j.diag(false, err, msg, kv) | |||
| 208 | + | } | |||
| 209 | + | ||||
| 210 | + | // diagDebug is diagWarn at debug level. | |||
| 211 | + | func (j *job) diagDebug(err error, msg string, kv ...string) { | |||
| 212 | + | j.diag(true, err, msg, kv) | |||
| 213 | + | } | |||
| 214 | + | ||||
| 191 | func (j *job) now() time.Time { | 215 | func (j *job) now() time.Time { | ||
| 192 | if j.cfg.Now != nil { | 216 | if j.cfg.Now != nil { | ||
| 193 | return j.cfg.Now() | 217 | return j.cfg.Now() | ||
| @@ -202,6 +226,13 @@ func Run(cancel *Canceller, cfg Config) Result { | |||||
| 202 | cancel = NewCanceller() | 226 | cancel = NewCanceller() | ||
| 203 | } | 227 | } | ||
| 204 | j := &job{cfg: cfg, cancel: cancel, trackID: "github_" + uuid.NewString(), matchers: &matcherRoot{}} | 228 | j := &job{cfg: cfg, cancel: cancel, trackID: "github_" + uuid.NewString(), matchers: &matcherRoot{}} | ||
| 229 | + | // The masker exists before anything is reported, with the secrets of the job | |||
| 230 | + | // registered (Worker.cs:158-171 does it before the job starts); the token and | |||
| 231 | + | // the cache token join it once the contexts are built. GitHub's SecretMasker | |||
| 232 | + | // has no minimum length (SecretMasker.AddValue ignores only null or empty | |||
| 233 | + | // values), so a one-character secret is masked too. | |||
| 234 | + | j.masker.Store(newMasker()) | |||
| 235 | + | registerSecrets(j.masker.Load(), cfg.Secrets) | |||
| 205 | j.cfg.Reporter = &maskReporter{j: j, next: cfg.Reporter} | 236 | j.cfg.Reporter = &maskReporter{j: j, next: cfg.Reporter} | ||
| 206 | defer func() { | 237 | defer func() { | ||
| 207 | if j.fc != nil { | 238 | if j.fc != nil { | ||
| @@ -450,8 +481,11 @@ func (j *job) setUp() ([]qstep, error) { | |||||
| 450 | layout = NewLayout(home, filepath.Base(layout.Workspace), cfg.ToolCache) | 481 | layout = NewLayout(home, filepath.Base(layout.Workspace), cfg.ToolCache) | ||
| 451 | state.WithLayout(layout) | 482 | state.WithLayout(layout) | ||
| 452 | } | 483 | } | ||
| 484 | + | state.WithJobData(cfg.Secrets, cfg.Vars) | |||
| 453 | j.state = state | 485 | j.state = state | ||
| 454 | - | j.masker.Store(newMasker(state.Secrets()...)) | 486 | + | for _, v := range state.Secrets() { |
| 487 | + | j.masker.Load().add(v) | |||
| 488 | + | } | |||
| 455 | j.masker.Load().add(cfg.CacheToken) | 489 | j.masker.Load().add(cfg.CacheToken) | ||
| 456 | 490 | ||||
| 457 | // Features that arrive in later phases fail the job with a clear message. | 491 | // Features that arrive in later phases fail the job with a clear message. | ||
| @@ -687,6 +721,10 @@ const ( | |||||
| 687 | defaultCacheMode = "write" | 721 | defaultCacheMode = "write" | ||
| 688 | ) | 722 | ) | ||
| 689 | 723 | ||||
| 724 | + | // SecretsWithheldLine is the line of "Set up job" for a run whose secrets the | |||
| 725 | + | // server withheld (an ermite text: GitHub has none for fork pull requests). | |||
| 726 | + | const SecretsWithheldLine = "Secrets are not available to this job: the run was started by a limited token." | |||
| 727 | + | ||||
| 690 | // printTokenInfo prints the GITHUB_TOKEN permissions group, the secret source | 728 | // printTokenInfo prints the GITHUB_TOKEN permissions group, the secret source | ||
| 691 | // and the cache mode. | 729 | // and the cache mode. | ||
| 692 | // | 730 | // | ||
| @@ -709,6 +747,9 @@ func (j *job) printTokenInfo() { | |||||
| 709 | j.state.SetGitHub("secret_source", src) | 747 | j.state.SetGitHub("secret_source", src) | ||
| 710 | } | 748 | } | ||
| 711 | j.out(1, "Secret source: "+src) | 749 | j.out(1, "Secret source: "+src) | ||
| 750 | + | if j.cfg.SecretsWithheld { | |||
| 751 | + | j.out(1, SecretsWithheldLine) | |||
| 752 | + | } | |||
| 712 | j.out(1, "Cache mode: "+defaultCacheMode) | 753 | j.out(1, "Cache mode: "+defaultCacheMode) | ||
| 713 | } | 754 | } | ||
| 714 | 755 | ||||
| @@ -723,7 +764,7 @@ func (j *job) printTokenInfo() { | |||||
| 723 | func (j *job) jobDisplayName() string { | 764 | func (j *job) jobDisplayName() string { | ||
| 724 | ctx := context.Background() | 765 | ctx := context.Background() | ||
| 725 | fallback := func(why string, err error) string { | 766 | fallback := func(why string, err error) string { | ||
| 726 | - | j.cfg.Log.Warn().Err(err).Str("job_key", j.cfg.JobKey).Msg("job display name: " + why + ", using the job key") | 767 | + | j.diagWarn(err, "job display name: "+why+", using the job key", "job_key", j.cfg.JobKey) |
| 727 | return j.cfg.JobKey | 768 | return j.cfg.JobKey | ||
| 728 | } | 769 | } | ||
| 729 | j.mu.Lock() | 770 | j.mu.Lock() | ||
| @@ -796,3 +837,17 @@ func sameMatrix(a, b *expr.Dictionary) bool { | |||||
| 796 | } | 837 | } | ||
| 797 | return true | 838 | return true | ||
| 798 | } | 839 | } | ||
| 840 | + | ||||
| 841 | + | func (j *job) diag(debug bool, err error, msg string, kv []string) { | |||
| 842 | + | ev := j.cfg.Log.Warn() | |||
| 843 | + | if debug { | |||
| 844 | + | ev = j.cfg.Log.Debug() | |||
| 845 | + | } | |||
| 846 | + | if err != nil { | |||
| 847 | + | ev = ev.Str("error", j.mask(err.Error())) | |||
| 848 | + | } | |||
| 849 | + | for i := 0; i+1 < len(kv); i += 2 { | |||
| 850 | + | ev = ev.Str(kv[i], j.mask(kv[i+1])) | |||
| 851 | + | } | |||
| 852 | + | ev.Msg(j.mask(msg)) | |||
| 853 | + | } | |||
A internal/executor/secrets_test.go +319 -0
| @@ -0,0 +1,319 @@ | |||
| 1 | + | // SPDX-License-Identifier: MIT | |
| 2 | + | ||
| 3 | + | package executor | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "bytes" | |
| 7 | + | "errors" | |
| 8 | + | "os" | |
| 9 | + | "path/filepath" | |
| 10 | + | ||
| 11 | + | "bnema.dev/ermite-actions/expr" | |
| 12 | + | "encoding/base64" | |
| 13 | + | "encoding/json" | |
| 14 | + | "github.com/bnema/zerowrap" | |
| 15 | + | "net/url" | |
| 16 | + | "strings" | |
| 17 | + | "testing" | |
| 18 | + | ) | |
| 19 | + | ||
| 20 | + | // jobSecretMasker is the masker of a job holding these secrets, built the way | |
| 21 | + | // Run does: the token first, then the secrets of the job. | |
| 22 | + | func jobSecretMasker(t *testing.T, secrets map[string]string) *masker { | |
| 23 | + | t.Helper() | |
| 24 | + | s, _, err := NewState(testGitHubContext("main"), "", "main", RunnerInfo{}, "") | |
| 25 | + | if err != nil { | |
| 26 | + | t.Fatal(err) | |
| 27 | + | } | |
| 28 | + | s.WithJobData(secrets, nil) | |
| 29 | + | m := newMasker(s.Secrets()...) | |
| 30 | + | registerSecrets(m, secrets) | |
| 31 | + | return m | |
| 32 | + | } | |
| 33 | + | ||
| 34 | + | // A secret is masked as is and in the encodings of CI-2 (Worker.cs:158-171, | |
| 35 | + | // HostContext.cs:103-113): base64, JSON, URL. | |
| 36 | + | func TestJobSecretIsMaskedInItsEncodings(t *testing.T) { | |
| 37 | + | const sec = `p@ss"w/rd é+1` | |
| 38 | + | m := jobSecretMasker(t, map[string]string{"DB_PASSWORD": sec}) | |
| 39 | + | j, _ := json.Marshal(sec) | |
| 40 | + | for name, text := range map[string]string{ | |
| 41 | + | "plain": "x " + sec + " y", | |
| 42 | + | "base64": "Basic " + base64.StdEncoding.EncodeToString([]byte(sec)), | |
| 43 | + | "json": "{\"k\":" + string(j) + "}", | |
| 44 | + | "url": "?q=" + strings.ReplaceAll(url.QueryEscape(sec), "+", "%20"), // Uri.EscapeDataString | |
| 45 | + | } { | |
| 46 | + | got := m.mask(text) | |
| 47 | + | if got == text || !strings.Contains(got, "***") { | |
| 48 | + | t.Errorf("%s: %q was not masked: %q", name, text, got) | |
| 49 | + | } | |
| 50 | + | } | |
| 51 | + | if got := m.mask("harmless"); got != "harmless" { | |
| 52 | + | t.Errorf("%q", got) | |
| 53 | + | } | |
| 54 | + | } | |
| 55 | + | ||
| 56 | + | // Worker.cs:158-171: the value is trimmed, and each non-empty trimmed line of a | |
| 57 | + | // multi-line value is registered too (split on CR and LF). | |
| 58 | + | func TestJobSecretLinesAndTrim(t *testing.T) { | |
| 59 | + | m := jobSecretMasker(t, map[string]string{ | |
| 60 | + | "KEY": " \n-----BEGIN KEY-----\r\n line-two-secret \n\n-----END KEY-----\n ", | |
| 61 | + | "PAD": " padded-secret \t\n", | |
| 62 | + | "NONE": "", | |
| 63 | + | "WS": " \r\n ", | |
| 64 | + | }) | |
| 65 | + | for _, text := range []string{"-----BEGIN KEY-----", "line-two-secret", "-----END KEY-----", "padded-secret", | |
| 66 | + | "-----BEGIN KEY-----\r\n line-two-secret \n\n-----END KEY-----"} { | |
| 67 | + | if m.mask("a "+text+" b") == "a "+text+" b" { | |
| 68 | + | t.Errorf("%q is not masked", text) | |
| 69 | + | } | |
| 70 | + | } | |
| 71 | + | // empty and blank values register nothing: text is untouched | |
| 72 | + | for _, text := range []string{"", " ", "\r\n", "plain text", "a b"} { | |
| 73 | + | if got := m.mask(text); got != text { | |
| 74 | + | t.Errorf("%q -> %q", text, got) | |
| 75 | + | } | |
| 76 | + | } | |
| 77 | + | // the value as sent, with its padding, is masked as far as the trimmed one | |
| 78 | + | if got := m.mask("v=padded-secret "); got != "v=*** " { | |
| 79 | + | t.Errorf("%q", got) | |
| 80 | + | } | |
| 81 | + | } | |
| 82 | + | ||
| 83 | + | func TestSecretsContext(t *testing.T) { | |
| 84 | + | s, _, err := NewState(tokenContexts(), "", "main", RunnerInfo{}, "") | |
| 85 | + | if err != nil { | |
| 86 | + | t.Fatal(err) | |
| 87 | + | } | |
| 88 | + | s.WithJobData(map[string]string{"B": "2", "A": "1", "GITHUB_TOKEN": "forged", "EMPTY": ""}, nil) | |
| 89 | + | d := s.secrets() | |
| 90 | + | got := map[string]string{} | |
| 91 | + | for _, p := range d.Pairs() { | |
| 92 | + | got[p.Key] = str(d, p.Key) | |
| 93 | + | } | |
| 94 | + | if len(got) != 4 || got["GITHUB_TOKEN"] != testToken || got["A"] != "1" || got["B"] != "2" || got["EMPTY"] != "" { | |
| 95 | + | t.Errorf("secrets context %v", got) | |
| 96 | + | } | |
| 97 | + | // without a token and without secrets the context is empty | |
| 98 | + | s0, _, _ := NewState(testGitHubContext("main"), "", "main", RunnerInfo{}, "") | |
| 99 | + | if s0.secrets().Len() != 0 { | |
| 100 | + | t.Error("secrets context is not empty") | |
| 101 | + | } | |
| 102 | + | } | |
| 103 | + | ||
| 104 | + | func TestVarsContext(t *testing.T) { | |
| 105 | + | vars := map[string]string{"A": "from-job"} | |
| 106 | + | // the contexts JSON has no vars: Job.vars is the fallback | |
| 107 | + | s, _, _ := NewState(testGitHubContext("main"), "", "main", RunnerInfo{}, "") | |
| 108 | + | s.WithJobData(nil, vars) | |
| 109 | + | if got := dictOf(t, s.other["vars"]); got != "A=from-job" { | |
| 110 | + | t.Errorf("fallback: %s", got) | |
| 111 | + | } | |
| 112 | + | // the contexts JSON has vars: it wins | |
| 113 | + | withVars := strings.Replace(testGitHubContext("main"), `{"github"`, `{"vars":{"A":"from-contexts","B":"b"},"github"`, 1) | |
| 114 | + | s2, _, err := NewState(withVars, "", "main", RunnerInfo{}, "") | |
| 115 | + | if err != nil { | |
| 116 | + | t.Fatal(err) | |
| 117 | + | } | |
| 118 | + | s2.WithJobData(nil, vars) | |
| 119 | + | if got := dictOf(t, s2.other["vars"]); got != "A=from-contexts B=b" { | |
| 120 | + | t.Errorf("contexts: %s", got) | |
| 121 | + | } | |
| 122 | + | // neither: an empty object | |
| 123 | + | s3, _, _ := NewState(testGitHubContext("main"), "", "main", RunnerInfo{}, "") | |
| 124 | + | s3.WithJobData(nil, nil) | |
| 125 | + | if got := dictOf(t, s3.other["vars"]); got != "" { | |
| 126 | + | t.Errorf("none: %s", got) | |
| 127 | + | } | |
| 128 | + | } | |
| 129 | + | ||
| 130 | + | func TestJobSecretsEndToEnd(t *testing.T) { | |
| 131 | + | needTools(t, "bash") | |
| 132 | + | const ( | |
| 133 | + | pw = "hunter2-secret" | |
| 134 | + | key = "-----BEGIN-----\nsecond-line-secret\n-----END-----" | |
| 135 | + | ) | |
| 136 | + | yaml := `on: push | |
| 137 | + | jobs: | |
| 138 | + | main: | |
| 139 | + | runs-on: ubuntu-24.04 | |
| 140 | + | outputs: | |
| 141 | + | leak: ${{ steps.s.outputs.v }} | |
| 142 | + | leak64: ${{ steps.s.outputs.b }} | |
| 143 | + | line: ${{ steps.s.outputs.l }} | |
| 144 | + | fine: ${{ steps.s.outputs.w }} | |
| 145 | + | steps: | |
| 146 | + | - id: s | |
| 147 | + | env: | |
| 148 | + | PW: ${{ secrets.PW }} | |
| 149 | + | KEY: ${{ secrets.KEY }} | |
| 150 | + | V: ${{ vars.V }} | |
| 151 | + | MISSING: ${{ secrets.NOPE }} | |
| 152 | + | run: | | |
| 153 | + | echo "pw=$PW" | |
| 154 | + | echo "b64=$(printf %s "$PW" | base64)" | |
| 155 | + | echo "line=$(printf %s "$KEY" | sed -n 2p)" | |
| 156 | + | echo "v=$V missing=[$MISSING]" | |
| 157 | + | echo "v=pre-$PW-post" >> "$GITHUB_OUTPUT" | |
| 158 | + | echo "b=$(printf %s "$PW" | base64)" >> "$GITHUB_OUTPUT" | |
| 159 | + | echo "l=second-line-secret" >> "$GITHUB_OUTPUT" | |
| 160 | + | echo "w=$V" >> "$GITHUB_OUTPUT" | |
| 161 | + | ` | |
| 162 | + | rec, res := runLocal(t, yaml, "main", func(c *Config) { | |
| 163 | + | c.Secrets = map[string]string{"PW": pw, "KEY": key} | |
| 164 | + | c.Vars = map[string]string{"V": "visible"} | |
| 165 | + | }) | |
| 166 | + | if res.Conclusion != ConclusionSuccess { | |
| 167 | + | t.Fatalf("%+v", res) | |
| 168 | + | } | |
| 169 | + | var all []string | |
| 170 | + | for _, s := range rec.steps { | |
| 171 | + | all = append(all, s.Name, logOf(s)) | |
| 172 | + | } | |
| 173 | + | joined := strings.Join(all, "\n") | |
| 174 | + | for _, secret := range []string{pw, "second-line-secret", base64.StdEncoding.EncodeToString([]byte(pw))} { | |
| 175 | + | if strings.Contains(joined, secret) { | |
| 176 | + | t.Errorf("%q leaked in the log:\n%s", secret, joined) | |
| 177 | + | } | |
| 178 | + | } | |
| 179 | + | for _, want := range []string{"pw=***", "line=***", "v=visible missing=[]"} { | |
| 180 | + | if !strings.Contains(joined, want) { | |
| 181 | + | t.Errorf("log lacks %q:\n%s", want, joined) | |
| 182 | + | } | |
| 183 | + | } | |
| 184 | + | if len(res.Outputs) != 1 || res.Outputs["fine"] != "visible" { | |
| 185 | + | t.Errorf("outputs %v", res.Outputs) | |
| 186 | + | } | |
| 187 | + | done := logOf(rec.step(t, CompleteJobName)) | |
| 188 | + | for _, n := range []string{"leak", "leak64", "line"} { | |
| 189 | + | if w := "##[warning]Skip output '" + n + "' since it may contain secret."; !strings.Contains(done, w) { | |
| 190 | + | t.Errorf("warning %q missing:\n%s", w, done) | |
| 191 | + | } | |
| 192 | + | } | |
| 193 | + | if strings.Contains(done, "Skip output 'fine'") { | |
| 194 | + | t.Error("fine output skipped") | |
| 195 | + | } | |
| 196 | + | } | |
| 197 | + | ||
| 198 | + | func TestSecretsWithheldLine(t *testing.T) { | |
| 199 | + | needTools(t, "bash") | |
| 200 | + | yaml := wfHead + " steps:\n - run: echo hi\n" | |
| 201 | + | with := setupLog(t, yaml, "j", func(c *Config) { c.SecretsWithheld = true }) | |
| 202 | + | if n := strings.Count(with, SecretsWithheldLine); n != 1 || SecretsWithheldLine != "Secrets are not available to this job: the run was started by a limited token." { | |
| 203 | + | t.Errorf("line count %d:\n%s", n, with) | |
| 204 | + | } | |
| 205 | + | if without := setupLog(t, yaml, "j", nil); strings.Contains(without, "Secrets are not available") { | |
| 206 | + | t.Errorf("printed without the flag:\n%s", without) | |
| 207 | + | } | |
| 208 | + | } | |
| 209 | + | ||
| 210 | + | // dictOf renders a string dictionary as "K=v K=v" in order. | |
| 211 | + | func dictOf(t *testing.T, v expr.Value) string { | |
| 212 | + | t.Helper() | |
| 213 | + | d, ok := v.(*expr.Dictionary) | |
| 214 | + | if !ok { | |
| 215 | + | t.Fatalf("not a dictionary: %T", v) | |
| 216 | + | } | |
| 217 | + | var parts []string | |
| 218 | + | for _, p := range d.Pairs() { | |
| 219 | + | parts = append(parts, p.Key+"="+str(d, p.Key)) | |
| 220 | + | } | |
| 221 | + | return strings.Join(parts, " ") | |
| 222 | + | } | |
| 223 | + | ||
| 224 | + | // The first lines of "Set up job" are reported before the contexts exist: the | |
| 225 | + | // secrets of the job are already in the masker then (GO-002). GitHub's | |
| 226 | + | // SecretMasker has no minimum length (SecretMasker.AddValue ignores only null | |
| 227 | + | // or empty values, SecretMasker.cs:78-84), so a one-character secret is masked. | |
| 228 | + | func TestSecretsAreMaskedFromTheFirstLine(t *testing.T) { | |
| 229 | + | needTools(t, "bash") | |
| 230 | + | yaml := wfHead + " steps:\n - run: echo hi\n" | |
| 231 | + | rec, res := runLocal(t, yaml, "j", func(c *Config) { | |
| 232 | + | c.Secrets = map[string]string{"ONE": "7", "LONG": "topsecretvalue"} | |
| 233 | + | c.RunnerVersion = "v7.1-topsecretvalue" | |
| 234 | + | }) | |
| 235 | + | if res.Conclusion != ConclusionSuccess { | |
| 236 | + | t.Fatalf("%+v", res) | |
| 237 | + | } | |
| 238 | + | first := rec.steps[0].Log[0] | |
| 239 | + | if first != "Current runner version: 'v***.1-***'" { | |
| 240 | + | t.Errorf("first line %q", first) | |
| 241 | + | } | |
| 242 | + | } | |
| 243 | + | ||
| 244 | + | // An early failure of "Set up job" (before the contexts are built) is masked in | |
| 245 | + | // the log and in the error of the job. | |
| 246 | + | func TestEarlySetupFailureIsMasked(t *testing.T) { | |
| 247 | + | yaml := wfHead + " steps:\n - run: echo hi\n" | |
| 248 | + | rec, res := runLocal(t, yaml, "no-such-job-S3CRET", func(c *Config) { | |
| 249 | + | c.Secrets = map[string]string{"S": "S3CRET", "ONE": "-"} | |
| 250 | + | }) | |
| 251 | + | if res.Conclusion != ConclusionFailure { | |
| 252 | + | t.Fatalf("%+v", res) | |
| 253 | + | } | |
| 254 | + | all := logOf(rec.steps[0]) + "\n" + res.Error + "\n" + rec.result.Error | |
| 255 | + | if strings.Contains(all, "S3CRET") || strings.Contains(all, "-") || !strings.Contains(all, "no***such***job***") { | |
| 256 | + | t.Errorf("not masked:\n%s", all) | |
| 257 | + | } | |
| 258 | + | } | |
| 259 | + | ||
| 260 | + | // The diagnostics of the runner (Config.Log) bypass the reporter: they are | |
| 261 | + | // masked by diag (GO-001). A path with a secret in a failing hashFiles reaches | |
| 262 | + | // the diagnostics through the error. | |
| 263 | + | func TestDiagnosticsAreMasked(t *testing.T) { | |
| 264 | + | needTools(t, "bash") | |
| 265 | + | const sec = "s3cr3t-dir-name" | |
| 266 | + | yaml := `on: push | |
| 267 | + | jobs: | |
| 268 | + | j: | |
| 269 | + | runs-on: x | |
| 270 | + | steps: | |
| 271 | + | - run: ln -s missing ` + sec + ` | |
| 272 | + | - run: echo ${{ hashFiles('**') }} | |
| 273 | + | ` | |
| 274 | + | var logs bytes.Buffer | |
| 275 | + | rec, res := runLocal(t, yaml, "j", func(c *Config) { | |
| 276 | + | c.Secrets = map[string]string{"D": sec} | |
| 277 | + | c.Log = zerowrap.New(zerowrap.Config{Level: "debug", Format: "json", Output: &logs}) | |
| 278 | + | }) | |
| 279 | + | if res.Conclusion != ConclusionFailure { | |
| 280 | + | t.Fatalf("%+v", res) | |
| 281 | + | } | |
| 282 | + | if !strings.Contains(logs.String(), "hashFiles failed") { | |
| 283 | + | t.Fatalf("no diagnostic:\n%s", logs.String()) | |
| 284 | + | } | |
| 285 | + | if strings.Contains(logs.String(), sec) || !strings.Contains(logs.String(), "/***: statat ***") { | |
| 286 | + | t.Errorf("diagnostics not masked:\n%s", logs.String()) | |
| 287 | + | } | |
| 288 | + | for _, s := range rec.steps { | |
| 289 | + | if strings.Contains(logOf(s), sec) { | |
| 290 | + | t.Errorf("job log leaks:\n%s", logOf(s)) | |
| 291 | + | } | |
| 292 | + | } | |
| 293 | + | ||
| 294 | + | // The helper masks message, error and fields. | |
| 295 | + | logs.Reset() | |
| 296 | + | j := &job{cfg: Config{Log: zerowrap.New(zerowrap.Config{Level: "debug", Format: "json", Output: &logs})}} | |
| 297 | + | j.masker.Store(newMasker(sec)) | |
| 298 | + | j.diagWarn(errors.New("open "+sec+": no"), "msg "+sec, "k", "v "+sec) | |
| 299 | + | j.diagDebug(nil, "d "+sec) | |
| 300 | + | if strings.Contains(logs.String(), sec) || strings.Count(logs.String(), "***") != 4 { | |
| 301 | + | t.Errorf("helper: %s", logs.String()) | |
| 302 | + | } | |
| 303 | + | } | |
| 304 | + | ||
| 305 | + | // Config.Log is used only by diag: any other use would bypass the masker. | |
| 306 | + | func TestOnlyDiagUsesTheDiagnosticsLogger(t *testing.T) { | |
| 307 | + | files, _ := filepath.Glob("*.go") | |
| 308 | + | for _, f := range files { | |
| 309 | + | if strings.HasSuffix(f, "_test.go") { | |
| 310 | + | continue | |
| 311 | + | } | |
| 312 | + | b, _ := os.ReadFile(f) | |
| 313 | + | n := strings.Count(string(b), "cfg.Log") | |
| 314 | + | // job.go: the two uses are in diag. | |
| 315 | + | if (f == "job.go" && n != 2) || (f != "job.go" && n != 0) { | |
| 316 | + | t.Errorf("%s uses cfg.Log %d times: use diagWarn or diagDebug", f, n) | |
| 317 | + | } | |
| 318 | + | } | |
| 319 | + | } | |
| @@ -0,0 +1,319 @@ | |||||
| 1 | + | // SPDX-License-Identifier: MIT | |||
| 2 | + | ||||
| 3 | + | package executor | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "bytes" | |||
| 7 | + | "errors" | |||
| 8 | + | "os" | |||
| 9 | + | "path/filepath" | |||
| 10 | + | ||||
| 11 | + | "bnema.dev/ermite-actions/expr" | |||
| 12 | + | "encoding/base64" | |||
| 13 | + | "encoding/json" | |||
| 14 | + | "github.com/bnema/zerowrap" | |||
| 15 | + | "net/url" | |||
| 16 | + | "strings" | |||
| 17 | + | "testing" | |||
| 18 | + | ) | |||
| 19 | + | ||||
| 20 | + | // jobSecretMasker is the masker of a job holding these secrets, built the way | |||
| 21 | + | // Run does: the token first, then the secrets of the job. | |||
| 22 | + | func jobSecretMasker(t *testing.T, secrets map[string]string) *masker { | |||
| 23 | + | t.Helper() | |||
| 24 | + | s, _, err := NewState(testGitHubContext("main"), "", "main", RunnerInfo{}, "") | |||
| 25 | + | if err != nil { | |||
| 26 | + | t.Fatal(err) | |||
| 27 | + | } | |||
| 28 | + | s.WithJobData(secrets, nil) | |||
| 29 | + | m := newMasker(s.Secrets()...) | |||
| 30 | + | registerSecrets(m, secrets) | |||
| 31 | + | return m | |||
| 32 | + | } | |||
| 33 | + | ||||
| 34 | + | // A secret is masked as is and in the encodings of CI-2 (Worker.cs:158-171, | |||
| 35 | + | // HostContext.cs:103-113): base64, JSON, URL. | |||
| 36 | + | func TestJobSecretIsMaskedInItsEncodings(t *testing.T) { | |||
| 37 | + | const sec = `p@ss"w/rd é+1` | |||
| 38 | + | m := jobSecretMasker(t, map[string]string{"DB_PASSWORD": sec}) | |||
| 39 | + | j, _ := json.Marshal(sec) | |||
| 40 | + | for name, text := range map[string]string{ | |||
| 41 | + | "plain": "x " + sec + " y", | |||
| 42 | + | "base64": "Basic " + base64.StdEncoding.EncodeToString([]byte(sec)), | |||
| 43 | + | "json": "{\"k\":" + string(j) + "}", | |||
| 44 | + | "url": "?q=" + strings.ReplaceAll(url.QueryEscape(sec), "+", "%20"), // Uri.EscapeDataString | |||
| 45 | + | } { | |||
| 46 | + | got := m.mask(text) | |||
| 47 | + | if got == text || !strings.Contains(got, "***") { | |||
| 48 | + | t.Errorf("%s: %q was not masked: %q", name, text, got) | |||
| 49 | + | } | |||
| 50 | + | } | |||
| 51 | + | if got := m.mask("harmless"); got != "harmless" { | |||
| 52 | + | t.Errorf("%q", got) | |||
| 53 | + | } | |||
| 54 | + | } | |||
| 55 | + | ||||
| 56 | + | // Worker.cs:158-171: the value is trimmed, and each non-empty trimmed line of a | |||
| 57 | + | // multi-line value is registered too (split on CR and LF). | |||
| 58 | + | func TestJobSecretLinesAndTrim(t *testing.T) { | |||
| 59 | + | m := jobSecretMasker(t, map[string]string{ | |||
| 60 | + | "KEY": " \n-----BEGIN KEY-----\r\n line-two-secret \n\n-----END KEY-----\n ", | |||
| 61 | + | "PAD": " padded-secret \t\n", | |||
| 62 | + | "NONE": "", | |||
| 63 | + | "WS": " \r\n ", | |||
| 64 | + | }) | |||
| 65 | + | for _, text := range []string{"-----BEGIN KEY-----", "line-two-secret", "-----END KEY-----", "padded-secret", | |||
| 66 | + | "-----BEGIN KEY-----\r\n line-two-secret \n\n-----END KEY-----"} { | |||
| 67 | + | if m.mask("a "+text+" b") == "a "+text+" b" { | |||
| 68 | + | t.Errorf("%q is not masked", text) | |||
| 69 | + | } | |||
| 70 | + | } | |||
| 71 | + | // empty and blank values register nothing: text is untouched | |||
| 72 | + | for _, text := range []string{"", " ", "\r\n", "plain text", "a b"} { | |||
| 73 | + | if got := m.mask(text); got != text { | |||
| 74 | + | t.Errorf("%q -> %q", text, got) | |||
| 75 | + | } | |||
| 76 | + | } | |||
| 77 | + | // the value as sent, with its padding, is masked as far as the trimmed one | |||
| 78 | + | if got := m.mask("v=padded-secret "); got != "v=*** " { | |||
| 79 | + | t.Errorf("%q", got) | |||
| 80 | + | } | |||
| 81 | + | } | |||
| 82 | + | ||||
| 83 | + | func TestSecretsContext(t *testing.T) { | |||
| 84 | + | s, _, err := NewState(tokenContexts(), "", "main", RunnerInfo{}, "") | |||
| 85 | + | if err != nil { | |||
| 86 | + | t.Fatal(err) | |||
| 87 | + | } | |||
| 88 | + | s.WithJobData(map[string]string{"B": "2", "A": "1", "GITHUB_TOKEN": "forged", "EMPTY": ""}, nil) | |||
| 89 | + | d := s.secrets() | |||
| 90 | + | got := map[string]string{} | |||
| 91 | + | for _, p := range d.Pairs() { | |||
| 92 | + | got[p.Key] = str(d, p.Key) | |||
| 93 | + | } | |||
| 94 | + | if len(got) != 4 || got["GITHUB_TOKEN"] != testToken || got["A"] != "1" || got["B"] != "2" || got["EMPTY"] != "" { | |||
| 95 | + | t.Errorf("secrets context %v", got) | |||
| 96 | + | } | |||
| 97 | + | // without a token and without secrets the context is empty | |||
| 98 | + | s0, _, _ := NewState(testGitHubContext("main"), "", "main", RunnerInfo{}, "") | |||
| 99 | + | if s0.secrets().Len() != 0 { | |||
| 100 | + | t.Error("secrets context is not empty") | |||
| 101 | + | } | |||
| 102 | + | } | |||
| 103 | + | ||||
| 104 | + | func TestVarsContext(t *testing.T) { | |||
| 105 | + | vars := map[string]string{"A": "from-job"} | |||
| 106 | + | // the contexts JSON has no vars: Job.vars is the fallback | |||
| 107 | + | s, _, _ := NewState(testGitHubContext("main"), "", "main", RunnerInfo{}, "") | |||
| 108 | + | s.WithJobData(nil, vars) | |||
| 109 | + | if got := dictOf(t, s.other["vars"]); got != "A=from-job" { | |||
| 110 | + | t.Errorf("fallback: %s", got) | |||
| 111 | + | } | |||
| 112 | + | // the contexts JSON has vars: it wins | |||
| 113 | + | withVars := strings.Replace(testGitHubContext("main"), `{"github"`, `{"vars":{"A":"from-contexts","B":"b"},"github"`, 1) | |||
| 114 | + | s2, _, err := NewState(withVars, "", "main", RunnerInfo{}, "") | |||
| 115 | + | if err != nil { | |||
| 116 | + | t.Fatal(err) | |||
| 117 | + | } | |||
| 118 | + | s2.WithJobData(nil, vars) | |||
| 119 | + | if got := dictOf(t, s2.other["vars"]); got != "A=from-contexts B=b" { | |||
| 120 | + | t.Errorf("contexts: %s", got) | |||
| 121 | + | } | |||
| 122 | + | // neither: an empty object | |||
| 123 | + | s3, _, _ := NewState(testGitHubContext("main"), "", "main", RunnerInfo{}, "") | |||
| 124 | + | s3.WithJobData(nil, nil) | |||
| 125 | + | if got := dictOf(t, s3.other["vars"]); got != "" { | |||
| 126 | + | t.Errorf("none: %s", got) | |||
| 127 | + | } | |||
| 128 | + | } | |||
| 129 | + | ||||
| 130 | + | func TestJobSecretsEndToEnd(t *testing.T) { | |||
| 131 | + | needTools(t, "bash") | |||
| 132 | + | const ( | |||
| 133 | + | pw = "hunter2-secret" | |||
| 134 | + | key = "-----BEGIN-----\nsecond-line-secret\n-----END-----" | |||
| 135 | + | ) | |||
| 136 | + | yaml := `on: push | |||
| 137 | + | jobs: | |||
| 138 | + | main: | |||
| 139 | + | runs-on: ubuntu-24.04 | |||
| 140 | + | outputs: | |||
| 141 | + | leak: ${{ steps.s.outputs.v }} | |||
| 142 | + | leak64: ${{ steps.s.outputs.b }} | |||
| 143 | + | line: ${{ steps.s.outputs.l }} | |||
| 144 | + | fine: ${{ steps.s.outputs.w }} | |||
| 145 | + | steps: | |||
| 146 | + | - id: s | |||
| 147 | + | env: | |||
| 148 | + | PW: ${{ secrets.PW }} | |||
| 149 | + | KEY: ${{ secrets.KEY }} | |||
| 150 | + | V: ${{ vars.V }} | |||
| 151 | + | MISSING: ${{ secrets.NOPE }} | |||
| 152 | + | run: | | |||
| 153 | + | echo "pw=$PW" | |||
| 154 | + | echo "b64=$(printf %s "$PW" | base64)" | |||
| 155 | + | echo "line=$(printf %s "$KEY" | sed -n 2p)" | |||
| 156 | + | echo "v=$V missing=[$MISSING]" | |||
| 157 | + | echo "v=pre-$PW-post" >> "$GITHUB_OUTPUT" | |||
| 158 | + | echo "b=$(printf %s "$PW" | base64)" >> "$GITHUB_OUTPUT" | |||
| 159 | + | echo "l=second-line-secret" >> "$GITHUB_OUTPUT" | |||
| 160 | + | echo "w=$V" >> "$GITHUB_OUTPUT" | |||
| 161 | + | ` | |||
| 162 | + | rec, res := runLocal(t, yaml, "main", func(c *Config) { | |||
| 163 | + | c.Secrets = map[string]string{"PW": pw, "KEY": key} | |||
| 164 | + | c.Vars = map[string]string{"V": "visible"} | |||
| 165 | + | }) | |||
| 166 | + | if res.Conclusion != ConclusionSuccess { | |||
| 167 | + | t.Fatalf("%+v", res) | |||
| 168 | + | } | |||
| 169 | + | var all []string | |||
| 170 | + | for _, s := range rec.steps { | |||
| 171 | + | all = append(all, s.Name, logOf(s)) | |||
| 172 | + | } | |||
| 173 | + | joined := strings.Join(all, "\n") | |||
| 174 | + | for _, secret := range []string{pw, "second-line-secret", base64.StdEncoding.EncodeToString([]byte(pw))} { | |||
| 175 | + | if strings.Contains(joined, secret) { | |||
| 176 | + | t.Errorf("%q leaked in the log:\n%s", secret, joined) | |||
| 177 | + | } | |||
| 178 | + | } | |||
| 179 | + | for _, want := range []string{"pw=***", "line=***", "v=visible missing=[]"} { | |||
| 180 | + | if !strings.Contains(joined, want) { | |||
| 181 | + | t.Errorf("log lacks %q:\n%s", want, joined) | |||
| 182 | + | } | |||
| 183 | + | } | |||
| 184 | + | if len(res.Outputs) != 1 || res.Outputs["fine"] != "visible" { | |||
| 185 | + | t.Errorf("outputs %v", res.Outputs) | |||
| 186 | + | } | |||
| 187 | + | done := logOf(rec.step(t, CompleteJobName)) | |||
| 188 | + | for _, n := range []string{"leak", "leak64", "line"} { | |||
| 189 | + | if w := "##[warning]Skip output '" + n + "' since it may contain secret."; !strings.Contains(done, w) { | |||
| 190 | + | t.Errorf("warning %q missing:\n%s", w, done) | |||
| 191 | + | } | |||
| 192 | + | } | |||
| 193 | + | if strings.Contains(done, "Skip output 'fine'") { | |||
| 194 | + | t.Error("fine output skipped") | |||
| 195 | + | } | |||
| 196 | + | } | |||
| 197 | + | ||||
| 198 | + | func TestSecretsWithheldLine(t *testing.T) { | |||
| 199 | + | needTools(t, "bash") | |||
| 200 | + | yaml := wfHead + " steps:\n - run: echo hi\n" | |||
| 201 | + | with := setupLog(t, yaml, "j", func(c *Config) { c.SecretsWithheld = true }) | |||
| 202 | + | if n := strings.Count(with, SecretsWithheldLine); n != 1 || SecretsWithheldLine != "Secrets are not available to this job: the run was started by a limited token." { | |||
| 203 | + | t.Errorf("line count %d:\n%s", n, with) | |||
| 204 | + | } | |||
| 205 | + | if without := setupLog(t, yaml, "j", nil); strings.Contains(without, "Secrets are not available") { | |||
| 206 | + | t.Errorf("printed without the flag:\n%s", without) | |||
| 207 | + | } | |||
| 208 | + | } | |||
| 209 | + | ||||
| 210 | + | // dictOf renders a string dictionary as "K=v K=v" in order. | |||
| 211 | + | func dictOf(t *testing.T, v expr.Value) string { | |||
| 212 | + | t.Helper() | |||
| 213 | + | d, ok := v.(*expr.Dictionary) | |||
| 214 | + | if !ok { | |||
| 215 | + | t.Fatalf("not a dictionary: %T", v) | |||
| 216 | + | } | |||
| 217 | + | var parts []string | |||
| 218 | + | for _, p := range d.Pairs() { | |||
| 219 | + | parts = append(parts, p.Key+"="+str(d, p.Key)) | |||
| 220 | + | } | |||
| 221 | + | return strings.Join(parts, " ") | |||
| 222 | + | } | |||
| 223 | + | ||||
| 224 | + | // The first lines of "Set up job" are reported before the contexts exist: the | |||
| 225 | + | // secrets of the job are already in the masker then (GO-002). GitHub's | |||
| 226 | + | // SecretMasker has no minimum length (SecretMasker.AddValue ignores only null | |||
| 227 | + | // or empty values, SecretMasker.cs:78-84), so a one-character secret is masked. | |||
| 228 | + | func TestSecretsAreMaskedFromTheFirstLine(t *testing.T) { | |||
| 229 | + | needTools(t, "bash") | |||
| 230 | + | yaml := wfHead + " steps:\n - run: echo hi\n" | |||
| 231 | + | rec, res := runLocal(t, yaml, "j", func(c *Config) { | |||
| 232 | + | c.Secrets = map[string]string{"ONE": "7", "LONG": "topsecretvalue"} | |||
| 233 | + | c.RunnerVersion = "v7.1-topsecretvalue" | |||
| 234 | + | }) | |||
| 235 | + | if res.Conclusion != ConclusionSuccess { | |||
| 236 | + | t.Fatalf("%+v", res) | |||
| 237 | + | } | |||
| 238 | + | first := rec.steps[0].Log[0] | |||
| 239 | + | if first != "Current runner version: 'v***.1-***'" { | |||
| 240 | + | t.Errorf("first line %q", first) | |||
| 241 | + | } | |||
| 242 | + | } | |||
| 243 | + | ||||
| 244 | + | // An early failure of "Set up job" (before the contexts are built) is masked in | |||
| 245 | + | // the log and in the error of the job. | |||
| 246 | + | func TestEarlySetupFailureIsMasked(t *testing.T) { | |||
| 247 | + | yaml := wfHead + " steps:\n - run: echo hi\n" | |||
| 248 | + | rec, res := runLocal(t, yaml, "no-such-job-S3CRET", func(c *Config) { | |||
| 249 | + | c.Secrets = map[string]string{"S": "S3CRET", "ONE": "-"} | |||
| 250 | + | }) | |||
| 251 | + | if res.Conclusion != ConclusionFailure { | |||
| 252 | + | t.Fatalf("%+v", res) | |||
| 253 | + | } | |||
| 254 | + | all := logOf(rec.steps[0]) + "\n" + res.Error + "\n" + rec.result.Error | |||
| 255 | + | if strings.Contains(all, "S3CRET") || strings.Contains(all, "-") || !strings.Contains(all, "no***such***job***") { | |||
| 256 | + | t.Errorf("not masked:\n%s", all) | |||
| 257 | + | } | |||
| 258 | + | } | |||
| 259 | + | ||||
| 260 | + | // The diagnostics of the runner (Config.Log) bypass the reporter: they are | |||
| 261 | + | // masked by diag (GO-001). A path with a secret in a failing hashFiles reaches | |||
| 262 | + | // the diagnostics through the error. | |||
| 263 | + | func TestDiagnosticsAreMasked(t *testing.T) { | |||
| 264 | + | needTools(t, "bash") | |||
| 265 | + | const sec = "s3cr3t-dir-name" | |||
| 266 | + | yaml := `on: push | |||
| 267 | + | jobs: | |||
| 268 | + | j: | |||
| 269 | + | runs-on: x | |||
| 270 | + | steps: | |||
| 271 | + | - run: ln -s missing ` + sec + ` | |||
| 272 | + | - run: echo ${{ hashFiles('**') }} | |||
| 273 | + | ` | |||
| 274 | + | var logs bytes.Buffer | |||
| 275 | + | rec, res := runLocal(t, yaml, "j", func(c *Config) { | |||
| 276 | + | c.Secrets = map[string]string{"D": sec} | |||
| 277 | + | c.Log = zerowrap.New(zerowrap.Config{Level: "debug", Format: "json", Output: &logs}) | |||
| 278 | + | }) | |||
| 279 | + | if res.Conclusion != ConclusionFailure { | |||
| 280 | + | t.Fatalf("%+v", res) | |||
| 281 | + | } | |||
| 282 | + | if !strings.Contains(logs.String(), "hashFiles failed") { | |||
| 283 | + | t.Fatalf("no diagnostic:\n%s", logs.String()) | |||
| 284 | + | } | |||
| 285 | + | if strings.Contains(logs.String(), sec) || !strings.Contains(logs.String(), "/***: statat ***") { | |||
| 286 | + | t.Errorf("diagnostics not masked:\n%s", logs.String()) | |||
| 287 | + | } | |||
| 288 | + | for _, s := range rec.steps { | |||
| 289 | + | if strings.Contains(logOf(s), sec) { | |||
| 290 | + | t.Errorf("job log leaks:\n%s", logOf(s)) | |||
| 291 | + | } | |||
| 292 | + | } | |||
| 293 | + | ||||
| 294 | + | // The helper masks message, error and fields. | |||
| 295 | + | logs.Reset() | |||
| 296 | + | j := &job{cfg: Config{Log: zerowrap.New(zerowrap.Config{Level: "debug", Format: "json", Output: &logs})}} | |||
| 297 | + | j.masker.Store(newMasker(sec)) | |||
| 298 | + | j.diagWarn(errors.New("open "+sec+": no"), "msg "+sec, "k", "v "+sec) | |||
| 299 | + | j.diagDebug(nil, "d "+sec) | |||
| 300 | + | if strings.Contains(logs.String(), sec) || strings.Count(logs.String(), "***") != 4 { | |||
| 301 | + | t.Errorf("helper: %s", logs.String()) | |||
| 302 | + | } | |||
| 303 | + | } | |||
| 304 | + | ||||
| 305 | + | // Config.Log is used only by diag: any other use would bypass the masker. | |||
| 306 | + | func TestOnlyDiagUsesTheDiagnosticsLogger(t *testing.T) { | |||
| 307 | + | files, _ := filepath.Glob("*.go") | |||
| 308 | + | for _, f := range files { | |||
| 309 | + | if strings.HasSuffix(f, "_test.go") { | |||
| 310 | + | continue | |||
| 311 | + | } | |||
| 312 | + | b, _ := os.ReadFile(f) | |||
| 313 | + | n := strings.Count(string(b), "cfg.Log") | |||
| 314 | + | // job.go: the two uses are in diag. | |||
| 315 | + | if (f == "job.go" && n != 2) || (f != "job.go" && n != 0) { | |||
| 316 | + | t.Errorf("%s uses cfg.Log %d times: use diagWarn or diagDebug", f, n) | |||
| 317 | + | } | |||
| 318 | + | } | |||
| 319 | + | } | |||
M internal/guest/exec.go +47 -17
| @@ -190,41 +190,71 @@ func serve(ctx context.Context, rw io.ReadWriteCloser, cfg ExecConfig) (int, err | |||
| 190 | 190 | }}}); err != nil { | |
| 191 | 191 | return 1, fmt.Errorf("send hello: %w", err) | |
| 192 | 192 | } | |
| 193 | - | run, err := waitForRun(ctx, conn, cfg.RunTimeout) | |
| 193 | + | run, vals, err := waitForRun(ctx, conn, cfg.RunTimeout) | |
| 194 | 194 | if err != nil { | |
| 195 | 195 | return 1, err | |
| 196 | 196 | } | |
| 197 | 197 | if run.GetJob() != nil { | |
| 198 | - | return sendExit(conn, runJob(ctx, conn, cfg, run.GetJob()), nil) | |
| 198 | + | return sendExit(conn, runJob(ctx, conn, cfg, run.GetJob(), vals), nil) | |
| 199 | 199 | } | |
| 200 | 200 | return runStub(ctx, conn, cfg, run) | |
| 201 | 201 | } | |
| 202 | 202 | ||
| 203 | + | // jobValues are the secrets and variables of the job that the host sent in | |
| 204 | + | // NamedValue frames before Run. They stay in memory. | |
| 205 | + | type jobValues struct{ secrets, vars map[string]string } | |
| 206 | + | ||
| 207 | + | // maxJobValues bounds what a host can make the executor keep before Run. | |
| 208 | + | const maxJobValues = 64 << 20 | |
| 209 | + | ||
| 203 | 210 | // waitForRun waits for the Run frame of the host, bounded: a host that never | |
| 204 | - | // answers must not pin the sandbox. | |
| 205 | - | func waitForRun(ctx context.Context, conn *execstream.Conn, timeout time.Duration) (*execpb.Run, error) { | |
| 211 | + | // answers must not pin the sandbox. The NamedValue frames that come before it | |
| 212 | + | // are collected. | |
| 213 | + | func waitForRun(ctx context.Context, conn *execstream.Conn, timeout time.Duration) (*execpb.Run, jobValues, error) { | |
| 206 | 214 | type recv struct { | |
| 207 | - | f *execpb.Frame | |
| 208 | - | err error | |
| 215 | + | run *execpb.Run | |
| 216 | + | vals jobValues | |
| 217 | + | err error | |
| 209 | 218 | } | |
| 210 | 219 | first := make(chan recv, 1) | |
| 211 | - | go func() { f, err := conn.Recv(); first <- recv{f, err} }() | |
| 220 | + | go func() { | |
| 221 | + | vals := jobValues{secrets: map[string]string{}, vars: map[string]string{}} | |
| 222 | + | size := 0 | |
| 223 | + | for { | |
| 224 | + | f, err := conn.Recv() | |
| 225 | + | if err != nil { | |
| 226 | + | first <- recv{err: fmt.Errorf("waiting for run: %w", err)} | |
| 227 | + | return | |
| 228 | + | } | |
| 229 | + | switch b := f.Body.(type) { | |
| 230 | + | case *execpb.Frame_Run: | |
| 231 | + | first <- recv{run: b.Run, vals: vals} | |
| 232 | + | return | |
| 233 | + | case *execpb.Frame_Secret: | |
| 234 | + | size += len(b.Secret.GetName()) + len(b.Secret.GetValue()) | |
| 235 | + | vals.secrets[b.Secret.GetName()] = b.Secret.GetValue() | |
| 236 | + | case *execpb.Frame_Var: | |
| 237 | + | size += len(b.Var.GetName()) + len(b.Var.GetValue()) | |
| 238 | + | vals.vars[b.Var.GetName()] = b.Var.GetValue() | |
| 239 | + | default: | |
| 240 | + | first <- recv{err: errors.New("first frame from host is not run")} | |
| 241 | + | return | |
| 242 | + | } | |
| 243 | + | if size > maxJobValues { | |
| 244 | + | first <- recv{err: errors.New("the host sent too many secrets and variables before run")} | |
| 245 | + | return | |
| 246 | + | } | |
| 247 | + | } | |
| 248 | + | }() | |
| 212 | 249 | timer := time.NewTimer(timeout) | |
| 213 | 250 | defer timer.Stop() | |
| 214 | 251 | select { | |
| 215 | 252 | case r := <-first: | |
| 216 | - | if r.err != nil { | |
| 217 | - | return nil, fmt.Errorf("waiting for run: %w", r.err) | |
| 218 | - | } | |
| 219 | - | run := r.f.GetRun() | |
| 220 | - | if run == nil { | |
| 221 | - | return nil, errors.New("first frame from host is not run") | |
| 222 | - | } | |
| 223 | - | return run, nil | |
| 253 | + | return r.run, r.vals, r.err | |
| 224 | 254 | case <-timer.C: | |
| 225 | - | return nil, errors.New("timed out waiting for the host's run frame") | |
| 255 | + | return nil, jobValues{}, errors.New("timed out waiting for the host's run frame") | |
| 226 | 256 | case <-ctx.Done(): | |
| 227 | - | return nil, ctx.Err() | |
| 257 | + | return nil, jobValues{}, ctx.Err() | |
| 228 | 258 | } | |
| 229 | 259 | } | |
| 230 | 260 | ||
| @@ -190,41 +190,71 @@ func serve(ctx context.Context, rw io.ReadWriteCloser, cfg ExecConfig) (int, err | |||||
| 190 | }}}); err != nil { | 190 | }}}); err != nil { | ||
| 191 | return 1, fmt.Errorf("send hello: %w", err) | 191 | return 1, fmt.Errorf("send hello: %w", err) | ||
| 192 | } | 192 | } | ||
| 193 | - | run, err := waitForRun(ctx, conn, cfg.RunTimeout) | 193 | + | run, vals, err := waitForRun(ctx, conn, cfg.RunTimeout) |
| 194 | if err != nil { | 194 | if err != nil { | ||
| 195 | return 1, err | 195 | return 1, err | ||
| 196 | } | 196 | } | ||
| 197 | if run.GetJob() != nil { | 197 | if run.GetJob() != nil { | ||
| 198 | - | return sendExit(conn, runJob(ctx, conn, cfg, run.GetJob()), nil) | 198 | + | return sendExit(conn, runJob(ctx, conn, cfg, run.GetJob(), vals), nil) |
| 199 | } | 199 | } | ||
| 200 | return runStub(ctx, conn, cfg, run) | 200 | return runStub(ctx, conn, cfg, run) | ||
| 201 | } | 201 | } | ||
| 202 | 202 | ||||
| 203 | + | // jobValues are the secrets and variables of the job that the host sent in | |||
| 204 | + | // NamedValue frames before Run. They stay in memory. | |||
| 205 | + | type jobValues struct{ secrets, vars map[string]string } | |||
| 206 | + | ||||
| 207 | + | // maxJobValues bounds what a host can make the executor keep before Run. | |||
| 208 | + | const maxJobValues = 64 << 20 | |||
| 209 | + | ||||
| 203 | // waitForRun waits for the Run frame of the host, bounded: a host that never | 210 | // waitForRun waits for the Run frame of the host, bounded: a host that never | ||
| 204 | - | // answers must not pin the sandbox. | 211 | + | // answers must not pin the sandbox. The NamedValue frames that come before it |
| 205 | - | func waitForRun(ctx context.Context, conn *execstream.Conn, timeout time.Duration) (*execpb.Run, error) { | 212 | + | // are collected. |
| 213 | + | func waitForRun(ctx context.Context, conn *execstream.Conn, timeout time.Duration) (*execpb.Run, jobValues, error) { | |||
| 206 | type recv struct { | 214 | type recv struct { | ||
| 207 | - | f *execpb.Frame | 215 | + | run *execpb.Run |
| 208 | - | err error | 216 | + | vals jobValues |
| 217 | + | err error | |||
| 209 | } | 218 | } | ||
| 210 | first := make(chan recv, 1) | 219 | first := make(chan recv, 1) | ||
| 211 | - | go func() { f, err := conn.Recv(); first <- recv{f, err} }() | 220 | + | go func() { |
| 221 | + | vals := jobValues{secrets: map[string]string{}, vars: map[string]string{}} | |||
| 222 | + | size := 0 | |||
| 223 | + | for { | |||
| 224 | + | f, err := conn.Recv() | |||
| 225 | + | if err != nil { | |||
| 226 | + | first <- recv{err: fmt.Errorf("waiting for run: %w", err)} | |||
| 227 | + | return | |||
| 228 | + | } | |||
| 229 | + | switch b := f.Body.(type) { | |||
| 230 | + | case *execpb.Frame_Run: | |||
| 231 | + | first <- recv{run: b.Run, vals: vals} | |||
| 232 | + | return | |||
| 233 | + | case *execpb.Frame_Secret: | |||
| 234 | + | size += len(b.Secret.GetName()) + len(b.Secret.GetValue()) | |||
| 235 | + | vals.secrets[b.Secret.GetName()] = b.Secret.GetValue() | |||
| 236 | + | case *execpb.Frame_Var: | |||
| 237 | + | size += len(b.Var.GetName()) + len(b.Var.GetValue()) | |||
| 238 | + | vals.vars[b.Var.GetName()] = b.Var.GetValue() | |||
| 239 | + | default: | |||
| 240 | + | first <- recv{err: errors.New("first frame from host is not run")} | |||
| 241 | + | return | |||
| 242 | + | } | |||
| 243 | + | if size > maxJobValues { | |||
| 244 | + | first <- recv{err: errors.New("the host sent too many secrets and variables before run")} | |||
| 245 | + | return | |||
| 246 | + | } | |||
| 247 | + | } | |||
| 248 | + | }() | |||
| 212 | timer := time.NewTimer(timeout) | 249 | timer := time.NewTimer(timeout) | ||
| 213 | defer timer.Stop() | 250 | defer timer.Stop() | ||
| 214 | select { | 251 | select { | ||
| 215 | case r := <-first: | 252 | case r := <-first: | ||
| 216 | - | if r.err != nil { | 253 | + | return r.run, r.vals, r.err |
| 217 | - | return nil, fmt.Errorf("waiting for run: %w", r.err) | |||
| 218 | - | } | |||
| 219 | - | run := r.f.GetRun() | |||
| 220 | - | if run == nil { | |||
| 221 | - | return nil, errors.New("first frame from host is not run") | |||
| 222 | - | } | |||
| 223 | - | return run, nil | |||
| 224 | case <-timer.C: | 254 | case <-timer.C: | ||
| 225 | - | return nil, errors.New("timed out waiting for the host's run frame") | 255 | + | return nil, jobValues{}, errors.New("timed out waiting for the host's run frame") |
| 226 | case <-ctx.Done(): | 256 | case <-ctx.Done(): | ||
| 227 | - | return nil, ctx.Err() | 257 | + | return nil, jobValues{}, ctx.Err() |
| 228 | } | 258 | } | ||
| 229 | } | 259 | } | ||
| 230 | 260 | ||||
M internal/guest/jobexec.go +2 -1
| @@ -204,7 +204,7 @@ func cancelKind(k execpb.CancelKind) executor.CancelReason { | |||
| 204 | 204 | ||
| 205 | 205 | // runJob runs the job of a Run frame with the executor and reports on conn. | |
| 206 | 206 | // It returns the exit code to report. | |
| 207 | - | func runJob(ctx context.Context, conn *execstream.Conn, cfg ExecConfig, spec *execpb.JobSpec) int { | |
| 207 | + | func runJob(ctx context.Context, conn *execstream.Conn, cfg ExecConfig, spec *execpb.JobSpec, vals jobValues) int { | |
| 208 | 208 | su, err := LookupStepUser(cfg.StepUser, cfg.AllowRootSteps) | |
| 209 | 209 | if err != nil { | |
| 210 | 210 | // Reported like any failure of "Set up job": the job fails, with the reason | |
| @@ -259,6 +259,7 @@ func runJob(ctx context.Context, conn *execstream.Conn, cfg ExecConfig, spec *ex | |||
| 259 | 259 | Home: su.Home, ToolCache: os.Getenv("RUNNER_TOOL_CACHE"), BaseEnv: stepEnv(os.Environ(), su), Credential: su.Credential, | |
| 260 | 260 | CacheURL: cacheURL(os.Getenv("ERMITE_GATEWAY"), spec.GetCachePort()), CacheToken: spec.GetCacheToken(), | |
| 261 | 261 | Debug: spec.GetDebug(), Reporter: rep, Actions: executor.DefaultActionsConfig(res.resolve), Log: zerowrap.FromCtx(ctx), | |
| 262 | + | Secrets: vals.secrets, Vars: vals.vars, SecretsWithheld: spec.GetSecretsWithheld(), | |
| 262 | 263 | SendAnnotations: spec.GetSendAnnotations(), SendSummaries: spec.GetSendSummaries(), | |
| 263 | 264 | TermAfter: cfg.TermAfter, KillAfter: cfg.KillAfter, WaitDelay: cfg.WaitDelay, | |
| 264 | 265 | }) | |
| @@ -204,7 +204,7 @@ func cancelKind(k execpb.CancelKind) executor.CancelReason { | |||||
| 204 | 204 | ||||
| 205 | // runJob runs the job of a Run frame with the executor and reports on conn. | 205 | // runJob runs the job of a Run frame with the executor and reports on conn. | ||
| 206 | // It returns the exit code to report. | 206 | // It returns the exit code to report. | ||
| 207 | - | func runJob(ctx context.Context, conn *execstream.Conn, cfg ExecConfig, spec *execpb.JobSpec) int { | 207 | + | func runJob(ctx context.Context, conn *execstream.Conn, cfg ExecConfig, spec *execpb.JobSpec, vals jobValues) int { |
| 208 | su, err := LookupStepUser(cfg.StepUser, cfg.AllowRootSteps) | 208 | su, err := LookupStepUser(cfg.StepUser, cfg.AllowRootSteps) | ||
| 209 | if err != nil { | 209 | if err != nil { | ||
| 210 | // Reported like any failure of "Set up job": the job fails, with the reason | 210 | // Reported like any failure of "Set up job": the job fails, with the reason | ||
| @@ -259,6 +259,7 @@ func runJob(ctx context.Context, conn *execstream.Conn, cfg ExecConfig, spec *ex | |||||
| 259 | Home: su.Home, ToolCache: os.Getenv("RUNNER_TOOL_CACHE"), BaseEnv: stepEnv(os.Environ(), su), Credential: su.Credential, | 259 | Home: su.Home, ToolCache: os.Getenv("RUNNER_TOOL_CACHE"), BaseEnv: stepEnv(os.Environ(), su), Credential: su.Credential, | ||
| 260 | CacheURL: cacheURL(os.Getenv("ERMITE_GATEWAY"), spec.GetCachePort()), CacheToken: spec.GetCacheToken(), | 260 | CacheURL: cacheURL(os.Getenv("ERMITE_GATEWAY"), spec.GetCachePort()), CacheToken: spec.GetCacheToken(), | ||
| 261 | Debug: spec.GetDebug(), Reporter: rep, Actions: executor.DefaultActionsConfig(res.resolve), Log: zerowrap.FromCtx(ctx), | 261 | Debug: spec.GetDebug(), Reporter: rep, Actions: executor.DefaultActionsConfig(res.resolve), Log: zerowrap.FromCtx(ctx), | ||
| 262 | + | Secrets: vals.secrets, Vars: vals.vars, SecretsWithheld: spec.GetSecretsWithheld(), | |||
| 262 | SendAnnotations: spec.GetSendAnnotations(), SendSummaries: spec.GetSendSummaries(), | 263 | SendAnnotations: spec.GetSendAnnotations(), SendSummaries: spec.GetSendSummaries(), | ||
| 263 | TermAfter: cfg.TermAfter, KillAfter: cfg.KillAfter, WaitDelay: cfg.WaitDelay, | 264 | TermAfter: cfg.TermAfter, KillAfter: cfg.KillAfter, WaitDelay: cfg.WaitDelay, | ||
| 264 | }) | 265 | }) | ||
A internal/guest/values_test.go +76 -0
| @@ -0,0 +1,76 @@ | |||
| 1 | + | // SPDX-License-Identifier: MIT | |
| 2 | + | ||
| 3 | + | package guest | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "context" | |
| 7 | + | "fmt" | |
| 8 | + | "net" | |
| 9 | + | "strings" | |
| 10 | + | "testing" | |
| 11 | + | "time" | |
| 12 | + | ||
| 13 | + | "bnema.dev/ermite-runner/internal/execstream" | |
| 14 | + | "bnema.dev/ermite-runner/internal/execstream/execpb" | |
| 15 | + | ) | |
| 16 | + | ||
| 17 | + | // 100 secrets of 48 KiB (4.8 MB) do not fit one frame (MaxFrame is 2 MiB) but | |
| 18 | + | // do as NamedValue frames: this crosses the real transport, DriveJob on the | |
| 19 | + | // host side and waitForRun on the executor side. | |
| 20 | + | func TestManyLargeSecretsCrossTheStream(t *testing.T) { | |
| 21 | + | secrets := map[string]string{} | |
| 22 | + | for i := 0; i < 100; i++ { | |
| 23 | + | secrets[fmt.Sprintf("S%03d", i)] = strings.Repeat(string(rune('a'+i%26)), 48<<10) | |
| 24 | + | } | |
| 25 | + | vars := map[string]string{"A": "1", "B": "2"} | |
| 26 | + | ||
| 27 | + | // the old shape, every secret in the Run frame, would not have worked | |
| 28 | + | err := execstream.NewConn(discard{}).Send(&execpb.Frame{Body: &execpb.Frame_Run{Run: &execpb.Run{Job: &execpb.JobSpec{Contexts: strings.Join(valuesOf(secrets), "")}}}}) | |
| 29 | + | if err == nil { | |
| 30 | + | t.Fatal("test premise: all the secrets must exceed one frame") | |
| 31 | + | } | |
| 32 | + | ||
| 33 | + | hostSide, guestSide := net.Pipe() | |
| 34 | + | defer func() { _ = hostSide.Close(); _ = guestSide.Close() }() | |
| 35 | + | got := make(chan error, 1) | |
| 36 | + | go func() { | |
| 37 | + | c := execstream.NewConn(guestSide) | |
| 38 | + | _ = c.Send(&execpb.Frame{Body: &execpb.Frame_Hello{Hello: &execpb.Hello{}}}) | |
| 39 | + | run, vals, err := waitForRun(context.Background(), c, 10*time.Second) | |
| 40 | + | switch { | |
| 41 | + | case err != nil: | |
| 42 | + | got <- err | |
| 43 | + | case run.GetJob().GetJobKey() != "k" || len(vals.secrets) != 100 || len(vals.vars) != 2 || vals.vars["B"] != "2": | |
| 44 | + | got <- fmt.Errorf("run %v, %d secrets, vars %v", run.GetJob().GetJobKey(), len(vals.secrets), vals.vars) | |
| 45 | + | default: | |
| 46 | + | for k, v := range secrets { | |
| 47 | + | if vals.secrets[k] != v { | |
| 48 | + | got <- fmt.Errorf("secret %s differs", k) | |
| 49 | + | return | |
| 50 | + | } | |
| 51 | + | } | |
| 52 | + | got <- nil | |
| 53 | + | _ = c.Send(&execpb.Frame{Body: &execpb.Frame_Exit{Exit: &execpb.Exit{}}}) | |
| 54 | + | } | |
| 55 | + | }() | |
| 56 | + | _, exit, err := execstream.DriveJob(hostSide, &execpb.Run{Job: &execpb.JobSpec{JobKey: "k"}}, nil, execstream.JobHandler{Secrets: secrets, Vars: vars}) | |
| 57 | + | if err != nil || exit == nil { | |
| 58 | + | t.Fatalf("DriveJob: %v", err) | |
| 59 | + | } | |
| 60 | + | if err := <-got; err != nil { | |
| 61 | + | t.Fatal(err) | |
| 62 | + | } | |
| 63 | + | } | |
| 64 | + | ||
| 65 | + | type discard struct{} | |
| 66 | + | ||
| 67 | + | func (discard) Write(p []byte) (int, error) { return len(p), nil } | |
| 68 | + | func (discard) Read([]byte) (int, error) { select {} } | |
| 69 | + | ||
| 70 | + | func valuesOf(m map[string]string) []string { | |
| 71 | + | var out []string | |
| 72 | + | for _, v := range m { | |
| 73 | + | out = append(out, v) | |
| 74 | + | } | |
| 75 | + | return out | |
| 76 | + | } | |
| @@ -0,0 +1,76 @@ | |||||
| 1 | + | // SPDX-License-Identifier: MIT | |||
| 2 | + | ||||
| 3 | + | package guest | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "context" | |||
| 7 | + | "fmt" | |||
| 8 | + | "net" | |||
| 9 | + | "strings" | |||
| 10 | + | "testing" | |||
| 11 | + | "time" | |||
| 12 | + | ||||
| 13 | + | "bnema.dev/ermite-runner/internal/execstream" | |||
| 14 | + | "bnema.dev/ermite-runner/internal/execstream/execpb" | |||
| 15 | + | ) | |||
| 16 | + | ||||
| 17 | + | // 100 secrets of 48 KiB (4.8 MB) do not fit one frame (MaxFrame is 2 MiB) but | |||
| 18 | + | // do as NamedValue frames: this crosses the real transport, DriveJob on the | |||
| 19 | + | // host side and waitForRun on the executor side. | |||
| 20 | + | func TestManyLargeSecretsCrossTheStream(t *testing.T) { | |||
| 21 | + | secrets := map[string]string{} | |||
| 22 | + | for i := 0; i < 100; i++ { | |||
| 23 | + | secrets[fmt.Sprintf("S%03d", i)] = strings.Repeat(string(rune('a'+i%26)), 48<<10) | |||
| 24 | + | } | |||
| 25 | + | vars := map[string]string{"A": "1", "B": "2"} | |||
| 26 | + | ||||
| 27 | + | // the old shape, every secret in the Run frame, would not have worked | |||
| 28 | + | err := execstream.NewConn(discard{}).Send(&execpb.Frame{Body: &execpb.Frame_Run{Run: &execpb.Run{Job: &execpb.JobSpec{Contexts: strings.Join(valuesOf(secrets), "")}}}}) | |||
| 29 | + | if err == nil { | |||
| 30 | + | t.Fatal("test premise: all the secrets must exceed one frame") | |||
| 31 | + | } | |||
| 32 | + | ||||
| 33 | + | hostSide, guestSide := net.Pipe() | |||
| 34 | + | defer func() { _ = hostSide.Close(); _ = guestSide.Close() }() | |||
| 35 | + | got := make(chan error, 1) | |||
| 36 | + | go func() { | |||
| 37 | + | c := execstream.NewConn(guestSide) | |||
| 38 | + | _ = c.Send(&execpb.Frame{Body: &execpb.Frame_Hello{Hello: &execpb.Hello{}}}) | |||
| 39 | + | run, vals, err := waitForRun(context.Background(), c, 10*time.Second) | |||
| 40 | + | switch { | |||
| 41 | + | case err != nil: | |||
| 42 | + | got <- err | |||
| 43 | + | case run.GetJob().GetJobKey() != "k" || len(vals.secrets) != 100 || len(vals.vars) != 2 || vals.vars["B"] != "2": | |||
| 44 | + | got <- fmt.Errorf("run %v, %d secrets, vars %v", run.GetJob().GetJobKey(), len(vals.secrets), vals.vars) | |||
| 45 | + | default: | |||
| 46 | + | for k, v := range secrets { | |||
| 47 | + | if vals.secrets[k] != v { | |||
| 48 | + | got <- fmt.Errorf("secret %s differs", k) | |||
| 49 | + | return | |||
| 50 | + | } | |||
| 51 | + | } | |||
| 52 | + | got <- nil | |||
| 53 | + | _ = c.Send(&execpb.Frame{Body: &execpb.Frame_Exit{Exit: &execpb.Exit{}}}) | |||
| 54 | + | } | |||
| 55 | + | }() | |||
| 56 | + | _, exit, err := execstream.DriveJob(hostSide, &execpb.Run{Job: &execpb.JobSpec{JobKey: "k"}}, nil, execstream.JobHandler{Secrets: secrets, Vars: vars}) | |||
| 57 | + | if err != nil || exit == nil { | |||
| 58 | + | t.Fatalf("DriveJob: %v", err) | |||
| 59 | + | } | |||
| 60 | + | if err := <-got; err != nil { | |||
| 61 | + | t.Fatal(err) | |||
| 62 | + | } | |||
| 63 | + | } | |||
| 64 | + | ||||
| 65 | + | type discard struct{} | |||
| 66 | + | ||||
| 67 | + | func (discard) Write(p []byte) (int, error) { return len(p), nil } | |||
| 68 | + | func (discard) Read([]byte) (int, error) { select {} } | |||
| 69 | + | ||||
| 70 | + | func valuesOf(m map[string]string) []string { | |||
| 71 | + | var out []string | |||
| 72 | + | for _, v := range m { | |||
| 73 | + | out = append(out, v) | |||
| 74 | + | } | |||
| 75 | + | return out | |||
| 76 | + | } | |||
M internal/host/fake_test.go +25 -2
| @@ -10,6 +10,7 @@ import ( | |||
| 10 | 10 | "net/http/httptest" | |
| 11 | 11 | "strings" | |
| 12 | 12 | "sync" | |
| 13 | + | "sync/atomic" | |
| 13 | 14 | "testing" | |
| 14 | 15 | "time" | |
| 15 | 16 | ||
| @@ -409,10 +410,26 @@ func (s *fakeSandbox) Start(ctx context.Context, spec sandbox.JobSpec) (sandbox. | |||
| 409 | 410 | defer func() { _ = guest.Close() }() | |
| 410 | 411 | c := execstream.NewConn(guest) | |
| 411 | 412 | _ = c.Send(&execpb.Frame{Body: &execpb.Frame_Hello{Hello: &execpb.Hello{Version: "test", Sandbox: "container"}}}) | |
| 412 | - | f, err := c.Recv() | |
| 413 | - | if err != nil || f.GetRun() == nil { | |
| 413 | + | // NamedValue frames (secrets, variables) come before Run. | |
| 414 | + | vals := &seenValues{secrets: map[string]string{}, vars: map[string]string{}} | |
| 415 | + | var f *execpb.Frame | |
| 416 | + | for { | |
| 417 | + | var err error | |
| 418 | + | if f, err = c.Recv(); err != nil { | |
| 419 | + | return | |
| 420 | + | } | |
| 421 | + | if f.GetSecret() != nil { | |
| 422 | + | vals.secrets[f.GetSecret().Name] = f.GetSecret().Value | |
| 423 | + | } else if f.GetVar() != nil { | |
| 424 | + | vals.vars[f.GetVar().Name] = f.GetVar().Value | |
| 425 | + | } else { | |
| 426 | + | break | |
| 427 | + | } | |
| 428 | + | } | |
| 429 | + | if f.GetRun() == nil { | |
| 414 | 430 | return | |
| 415 | 431 | } | |
| 432 | + | lastValues.Store(vals) | |
| 416 | 433 | cancels := make(chan *execpb.Cancel, 4) | |
| 417 | 434 | go func() { | |
| 418 | 435 | for { | |
| @@ -606,3 +623,9 @@ func (s *fakeServer) Result(jobID string) hosttest.Result { | |||
| 606 | 623 | defer s.mu.Unlock() | |
| 607 | 624 | return hosttest.Result{Steps: s.steps} | |
| 608 | 625 | } | |
| 626 | + | ||
| 627 | + | // seenValues are the secrets and variables the scripted executor received | |
| 628 | + | // before the Run frame. | |
| 629 | + | type seenValues struct{ secrets, vars map[string]string } | |
| 630 | + | ||
| 631 | + | var lastValues atomic.Pointer[seenValues] | |
| @@ -10,6 +10,7 @@ import ( | |||||
| 10 | "net/http/httptest" | 10 | "net/http/httptest" | ||
| 11 | "strings" | 11 | "strings" | ||
| 12 | "sync" | 12 | "sync" | ||
| 13 | + | "sync/atomic" | |||
| 13 | "testing" | 14 | "testing" | ||
| 14 | "time" | 15 | "time" | ||
| 15 | 16 | ||||
| @@ -409,10 +410,26 @@ func (s *fakeSandbox) Start(ctx context.Context, spec sandbox.JobSpec) (sandbox. | |||||
| 409 | defer func() { _ = guest.Close() }() | 410 | defer func() { _ = guest.Close() }() | ||
| 410 | c := execstream.NewConn(guest) | 411 | c := execstream.NewConn(guest) | ||
| 411 | _ = c.Send(&execpb.Frame{Body: &execpb.Frame_Hello{Hello: &execpb.Hello{Version: "test", Sandbox: "container"}}}) | 412 | _ = c.Send(&execpb.Frame{Body: &execpb.Frame_Hello{Hello: &execpb.Hello{Version: "test", Sandbox: "container"}}}) | ||
| 412 | - | f, err := c.Recv() | 413 | + | // NamedValue frames (secrets, variables) come before Run. |
| 413 | - | if err != nil || f.GetRun() == nil { | 414 | + | vals := &seenValues{secrets: map[string]string{}, vars: map[string]string{}} |
| 415 | + | var f *execpb.Frame | |||
| 416 | + | for { | |||
| 417 | + | var err error | |||
| 418 | + | if f, err = c.Recv(); err != nil { | |||
| 419 | + | return | |||
| 420 | + | } | |||
| 421 | + | if f.GetSecret() != nil { | |||
| 422 | + | vals.secrets[f.GetSecret().Name] = f.GetSecret().Value | |||
| 423 | + | } else if f.GetVar() != nil { | |||
| 424 | + | vals.vars[f.GetVar().Name] = f.GetVar().Value | |||
| 425 | + | } else { | |||
| 426 | + | break | |||
| 427 | + | } | |||
| 428 | + | } | |||
| 429 | + | if f.GetRun() == nil { | |||
| 414 | return | 430 | return | ||
| 415 | } | 431 | } | ||
| 432 | + | lastValues.Store(vals) | |||
| 416 | cancels := make(chan *execpb.Cancel, 4) | 433 | cancels := make(chan *execpb.Cancel, 4) | ||
| 417 | go func() { | 434 | go func() { | ||
| 418 | for { | 435 | for { | ||
| @@ -606,3 +623,9 @@ func (s *fakeServer) Result(jobID string) hosttest.Result { | |||||
| 606 | defer s.mu.Unlock() | 623 | defer s.mu.Unlock() | ||
| 607 | return hosttest.Result{Steps: s.steps} | 624 | return hosttest.Result{Steps: s.steps} | ||
| 608 | } | 625 | } | ||
| 626 | + | ||||
| 627 | + | // seenValues are the secrets and variables the scripted executor received | |||
| 628 | + | // before the Run frame. | |||
| 629 | + | type seenValues struct{ secrets, vars map[string]string } | |||
| 630 | + | ||||
| 631 | + | var lastValues atomic.Pointer[seenValues] | |||
M internal/host/job.go +7 -0
| @@ -690,6 +690,12 @@ func (j *jobRun) drive(conn sandbox.Conn, rt *Runner) (*execpb.Exit, error) { | |||
| 690 | 690 | DeadlineMs: max(1, time.Until(j.deadline()).Milliseconds()), | |
| 691 | 691 | RunnerName: rt.Cfg.Name, RunnerVersion: rt.Cfg.Version, RunnerEnvironment: rt.Cfg.environment(), | |
| 692 | 692 | SendAnnotations: j.features.annotations, SendSummaries: j.features.summaries, | |
| 693 | + | // Job.secrets and Job.vars are not in this frame (it is limited to | |
| 694 | + | // 2 MiB): DriveJob sends them in NamedValue frames before it. The host | |
| 695 | + | // never logs them nor stores them. An old server leaves trusted false | |
| 696 | + | // because it does not send it, so the executor says "not available" | |
| 697 | + | // only when the server has ci.secrets. | |
| 698 | + | SecretsWithheld: j.features.secrets && !j.job.Trusted, | |
| 693 | 699 | }} | |
| 694 | 700 | run.Job.CachePort, run.Job.CacheToken = j.cacheFor(rt) | |
| 695 | 701 | _, exit, err := execstream.DriveJob(conn, run, j.cancelCh, execstream.JobHandler{ | |
| @@ -698,6 +704,7 @@ func (j *jobRun) drive(conn sandbox.Conn, rt *Runner) (*execpb.Exit, error) { | |||
| 698 | 704 | JobResult: func(r *execpb.JobResult) { j.mu.Lock(); j.result = r; j.mu.Unlock() }, | |
| 699 | 705 | Resolve: j.resolveActions, | |
| 700 | 706 | StepAnnotations: j.stepAnnotations, StepSummary: j.stepSummary, | |
| 707 | + | Secrets: j.job.Secrets, Vars: j.job.Vars, | |
| 701 | 708 | }) | |
| 702 | 709 | return exit, err | |
| 703 | 710 | } | |
| @@ -690,6 +690,12 @@ func (j *jobRun) drive(conn sandbox.Conn, rt *Runner) (*execpb.Exit, error) { | |||||
| 690 | DeadlineMs: max(1, time.Until(j.deadline()).Milliseconds()), | 690 | DeadlineMs: max(1, time.Until(j.deadline()).Milliseconds()), | ||
| 691 | RunnerName: rt.Cfg.Name, RunnerVersion: rt.Cfg.Version, RunnerEnvironment: rt.Cfg.environment(), | 691 | RunnerName: rt.Cfg.Name, RunnerVersion: rt.Cfg.Version, RunnerEnvironment: rt.Cfg.environment(), | ||
| 692 | SendAnnotations: j.features.annotations, SendSummaries: j.features.summaries, | 692 | SendAnnotations: j.features.annotations, SendSummaries: j.features.summaries, | ||
| 693 | + | // Job.secrets and Job.vars are not in this frame (it is limited to | |||
| 694 | + | // 2 MiB): DriveJob sends them in NamedValue frames before it. The host | |||
| 695 | + | // never logs them nor stores them. An old server leaves trusted false | |||
| 696 | + | // because it does not send it, so the executor says "not available" | |||
| 697 | + | // only when the server has ci.secrets. | |||
| 698 | + | SecretsWithheld: j.features.secrets && !j.job.Trusted, | |||
| 693 | }} | 699 | }} | ||
| 694 | run.Job.CachePort, run.Job.CacheToken = j.cacheFor(rt) | 700 | run.Job.CachePort, run.Job.CacheToken = j.cacheFor(rt) | ||
| 695 | _, exit, err := execstream.DriveJob(conn, run, j.cancelCh, execstream.JobHandler{ | 701 | _, exit, err := execstream.DriveJob(conn, run, j.cancelCh, execstream.JobHandler{ | ||
| @@ -698,6 +704,7 @@ func (j *jobRun) drive(conn sandbox.Conn, rt *Runner) (*execpb.Exit, error) { | |||||
| 698 | JobResult: func(r *execpb.JobResult) { j.mu.Lock(); j.result = r; j.mu.Unlock() }, | 704 | JobResult: func(r *execpb.JobResult) { j.mu.Lock(); j.result = r; j.mu.Unlock() }, | ||
| 699 | Resolve: j.resolveActions, | 705 | Resolve: j.resolveActions, | ||
| 700 | StepAnnotations: j.stepAnnotations, StepSummary: j.stepSummary, | 706 | StepAnnotations: j.stepAnnotations, StepSummary: j.stepSummary, | ||
| 707 | + | Secrets: j.job.Secrets, Vars: j.job.Vars, | |||
| 701 | }) | 708 | }) | ||
| 702 | return exit, err | 709 | return exit, err | ||
| 703 | } | 710 | } | ||
M internal/host/results.go +6 -1
| @@ -20,6 +20,9 @@ import ( | |||
| 20 | 20 | const ( | |
| 21 | 21 | FeatureAnnotations = "ci.annotations" | |
| 22 | 22 | FeatureSummaries = "ci.summaries" | |
| 23 | + | // FeatureSecrets: the server sends Job.secrets, vars, environment, | |
| 24 | + | // permissions and trusted. Without it Job.trusted is absent, not false. | |
| 25 | + | FeatureSecrets = "ci.secrets" | |
| 23 | 26 | ) | |
| 24 | 27 | ||
| 25 | 28 | // Limits of Annotation and UploadSummaryRequest (runner.proto). The executor | |
| @@ -39,7 +42,7 @@ const ( | |||
| 39 | 42 | type ServerFeatures func(ctx context.Context) ([]string, error) | |
| 40 | 43 | ||
| 41 | 44 | // jobFeatures is what the server accepts for this job. | |
| 42 | - | type jobFeatures struct{ annotations, summaries bool } | |
| 45 | + | type jobFeatures struct{ annotations, summaries, secrets bool } | |
| 43 | 46 | ||
| 44 | 47 | func featuresOf(list []string) jobFeatures { | |
| 45 | 48 | var f jobFeatures | |
| @@ -49,6 +52,8 @@ func featuresOf(list []string) jobFeatures { | |||
| 49 | 52 | f.annotations = true | |
| 50 | 53 | case FeatureSummaries: | |
| 51 | 54 | f.summaries = true | |
| 55 | + | case FeatureSecrets: | |
| 56 | + | f.secrets = true | |
| 52 | 57 | } | |
| 53 | 58 | } | |
| 54 | 59 | return f | |
| @@ -20,6 +20,9 @@ import ( | |||||
| 20 | const ( | 20 | const ( | ||
| 21 | FeatureAnnotations = "ci.annotations" | 21 | FeatureAnnotations = "ci.annotations" | ||
| 22 | FeatureSummaries = "ci.summaries" | 22 | FeatureSummaries = "ci.summaries" | ||
| 23 | + | // FeatureSecrets: the server sends Job.secrets, vars, environment, | |||
| 24 | + | // permissions and trusted. Without it Job.trusted is absent, not false. | |||
| 25 | + | FeatureSecrets = "ci.secrets" | |||
| 23 | ) | 26 | ) | ||
| 24 | 27 | ||||
| 25 | // Limits of Annotation and UploadSummaryRequest (runner.proto). The executor | 28 | // Limits of Annotation and UploadSummaryRequest (runner.proto). The executor | ||
| @@ -39,7 +42,7 @@ const ( | |||||
| 39 | type ServerFeatures func(ctx context.Context) ([]string, error) | 42 | type ServerFeatures func(ctx context.Context) ([]string, error) | ||
| 40 | 43 | ||||
| 41 | // jobFeatures is what the server accepts for this job. | 44 | // jobFeatures is what the server accepts for this job. | ||
| 42 | - | type jobFeatures struct{ annotations, summaries bool } | 45 | + | type jobFeatures struct{ annotations, summaries, secrets bool } |
| 43 | 46 | ||||
| 44 | func featuresOf(list []string) jobFeatures { | 47 | func featuresOf(list []string) jobFeatures { | ||
| 45 | var f jobFeatures | 48 | var f jobFeatures | ||
| @@ -49,6 +52,8 @@ func featuresOf(list []string) jobFeatures { | |||||
| 49 | f.annotations = true | 52 | f.annotations = true | ||
| 50 | case FeatureSummaries: | 53 | case FeatureSummaries: | ||
| 51 | f.summaries = true | 54 | f.summaries = true | ||
| 55 | + | case FeatureSecrets: | |||
| 56 | + | f.secrets = true | |||
| 52 | } | 57 | } | ||
| 53 | } | 58 | } | ||
| 54 | return f | 59 | return f | ||
A internal/host/secrets_test.go +302 -0
| @@ -0,0 +1,302 @@ | |||
| 1 | + | // SPDX-License-Identifier: MIT | |
| 2 | + | ||
| 3 | + | package host | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "context" | |
| 7 | + | "fmt" | |
| 8 | + | "go/ast" | |
| 9 | + | "go/format" | |
| 10 | + | "go/parser" | |
| 11 | + | "go/token" | |
| 12 | + | "os" | |
| 13 | + | "path/filepath" | |
| 14 | + | "regexp" | |
| 15 | + | "strings" | |
| 16 | + | "testing" | |
| 17 | + | ||
| 18 | + | "google.golang.org/protobuf/proto" | |
| 19 | + | ||
| 20 | + | "bnema.dev/ermite-runner/internal/execstream/execpb" | |
| 21 | + | ) | |
| 22 | + | ||
| 23 | + | const ( | |
| 24 | + | secretValue = "s3cr3t-value-DO-NOT-LOG" | |
| 25 | + | secretValue2 = "-----BEGIN-----\nline-two-DO-NOT-LOG\n-----END-----" | |
| 26 | + | ) | |
| 27 | + | ||
| 28 | + | func withSecrets(trusted bool) func(*harness) { | |
| 29 | + | return func(h *harness) { | |
| 30 | + | j := testJob("j1") | |
| 31 | + | j.Secrets = map[string]string{"DB_PASSWORD": secretValue, "KEY": secretValue2} | |
| 32 | + | j.Vars = map[string]string{"REGION": "eu"} | |
| 33 | + | j.Trusted = trusted | |
| 34 | + | h.srv.jobs = nil | |
| 35 | + | h.srv.jobs = append(h.srv.jobs, j) | |
| 36 | + | } | |
| 37 | + | } | |
| 38 | + | ||
| 39 | + | func runWithSecrets(t *testing.T, features []string, trusted bool) (*harness, *execpb.Run, string) { | |
| 40 | + | t.Helper() | |
| 41 | + | seenRun.Store(nil) | |
| 42 | + | state := t.TempDir() | |
| 43 | + | h := newHarness(t, resultsScript(nil)) | |
| 44 | + | h.runner.Cfg.StateDir = state | |
| 45 | + | h.runner.Features = func(context.Context) ([]string, error) { return features, nil } | |
| 46 | + | withSecrets(trusted)(h) | |
| 47 | + | stop := h.run(t) | |
| 48 | + | eventually(t, "CompleteJob", func() bool { return h.srv.completeCount() == 1 }) | |
| 49 | + | if err := stop(); err != nil { | |
| 50 | + | t.Fatal(err) | |
| 51 | + | } | |
| 52 | + | return h, seenRun.Load(), state | |
| 53 | + | } | |
| 54 | + | ||
| 55 | + | func TestSecretsAndVarsReachTheExecutorOnly(t *testing.T) { | |
| 56 | + | h, run, state := runWithSecrets(t, []string{FeatureSecrets, FeatureAnnotations}, true) | |
| 57 | + | if run == nil { | |
| 58 | + | t.Fatal("no Run frame") | |
| 59 | + | } | |
| 60 | + | vals := lastValues.Load() | |
| 61 | + | if vals == nil || vals.secrets["DB_PASSWORD"] != secretValue || vals.secrets["KEY"] != secretValue2 || len(vals.secrets) != 2 { | |
| 62 | + | t.Errorf("secrets not passed: %+v", vals) | |
| 63 | + | } | |
| 64 | + | if vals.vars["REGION"] != "eu" || run.Job.SecretsWithheld { | |
| 65 | + | t.Errorf("vars %v withheld %v", vals.vars, run.Job.SecretsWithheld) | |
| 66 | + | } | |
| 67 | + | // The Run frame itself holds no secret. | |
| 68 | + | if rb, _ := proto.Marshal(run); strings.Contains(string(rb), "DO-NOT-LOG") { | |
| 69 | + | t.Error("the Run frame holds a secret") | |
| 70 | + | } | |
| 71 | + | ||
| 72 | + | // Nothing the host does prints the secrets: its log (debug level) and its | |
| 73 | + | // state directory. (protobuf-go does not redact String(), so the guarantee | |
| 74 | + | // is that no code formats the Job, the Run or the JobSpec; see the grep in | |
| 75 | + | // TestNoCodePrintsTheJobMessages.) | |
| 76 | + | if strings.Contains(h.logs.String(), "DO-NOT-LOG") { | |
| 77 | + | t.Errorf("a secret is in the host log:\n%.500s", h.logs.String()) | |
| 78 | + | } | |
| 79 | + | err := filepath.Walk(state, func(p string, info os.FileInfo, err error) error { | |
| 80 | + | if err != nil || info.IsDir() { | |
| 81 | + | return err | |
| 82 | + | } | |
| 83 | + | b, _ := os.ReadFile(p) | |
| 84 | + | if strings.Contains(string(b), "DO-NOT-LOG") { | |
| 85 | + | t.Errorf("a secret is in the state directory: %s", p) | |
| 86 | + | } | |
| 87 | + | return nil | |
| 88 | + | }) | |
| 89 | + | if err != nil { | |
| 90 | + | t.Fatal(err) | |
| 91 | + | } | |
| 92 | + | } | |
| 93 | + | ||
| 94 | + | // The "not available" line only exists when the server has ci.secrets: an old | |
| 95 | + | // server does not send Job.trusted, which then reads false. | |
| 96 | + | func TestSecretsWithheldOnlyWithTheFeature(t *testing.T) { | |
| 97 | + | for _, tc := range []struct { | |
| 98 | + | name string | |
| 99 | + | features []string | |
| 100 | + | trusted bool | |
| 101 | + | want bool | |
| 102 | + | }{ | |
| 103 | + | {"untrusted, server has ci.secrets", []string{FeatureSecrets}, false, true}, | |
| 104 | + | {"trusted, server has ci.secrets", []string{FeatureSecrets}, true, false}, | |
| 105 | + | {"untrusted, old server", []string{FeatureAnnotations}, false, false}, | |
| 106 | + | {"untrusted, features unreachable", nil, false, false}, | |
| 107 | + | } { | |
| 108 | + | t.Run(tc.name, func(t *testing.T) { | |
| 109 | + | _, run, _ := runWithSecrets(t, tc.features, tc.trusted) | |
| 110 | + | if run == nil || run.Job.SecretsWithheld != tc.want { | |
| 111 | + | t.Errorf("secrets_withheld = %v, want %v", run != nil && run.Job.SecretsWithheld, tc.want) | |
| 112 | + | } | |
| 113 | + | }) | |
| 114 | + | } | |
| 115 | + | } | |
| 116 | + | ||
| 117 | + | // protobuf-go prints map values in String(), so nothing may format a message | |
| 118 | + | // that holds secrets: the Job (ermite.Job, with Secrets) or the Run and JobSpec | |
| 119 | + | // that wrap it. This is a heuristic on names, not a proof: printLeaks looks, | |
| 120 | + | // in the sources of the host, the guest, execstream and the executor, for | |
| 121 | + | // formatting, logging, marshalling and String() calls that are given a value | |
| 122 | + | // named like those messages (job, j.job, r.job, spec, run, x.GetJob()), their | |
| 123 | + | // Secrets field or GetSecrets(). It does not follow a message through other | |
| 124 | + | // names or through values it was copied into. The real guarantees are the | |
| 125 | + | // tests that read the host log and the state directory | |
| 126 | + | // (TestSecretsAndVarsReachTheExecutorOnly) and the absence of secrets in the | |
| 127 | + | // Run frame. | |
| 128 | + | func TestNoCodePrintsTheJobMessages(t *testing.T) { | |
| 129 | + | fset := token.NewFileSet() | |
| 130 | + | for _, dir := range []string{".", "../guest", "../execstream", "../executor"} { | |
| 131 | + | files, err := filepath.Glob(filepath.Join(dir, "*.go")) | |
| 132 | + | if err != nil { | |
| 133 | + | t.Fatal(err) | |
| 134 | + | } | |
| 135 | + | for _, file := range files { | |
| 136 | + | if strings.HasSuffix(file, "_test.go") { | |
| 137 | + | continue | |
| 138 | + | } | |
| 139 | + | src, err := os.ReadFile(file) | |
| 140 | + | if err != nil { | |
| 141 | + | t.Fatal(err) | |
| 142 | + | } | |
| 143 | + | for _, leak := range printLeaks(t, fset, file, src) { | |
| 144 | + | t.Error(leak) | |
| 145 | + | } | |
| 146 | + | } | |
| 147 | + | } | |
| 148 | + | } | |
| 149 | + | ||
| 150 | + | var ( | |
| 151 | + | // printers are the calls that render their arguments: fmt, loggers (zerolog | |
| 152 | + | // events), the text and JSON encoders of messages. | |
| 153 | + | printers = map[string]bool{"Errorf": true, "Printf": true, "Sprintf": true, "Fprintf": true, "Println": true, "Fprintln": true, | |
| 154 | + | "Print": true, "Sprint": true, "Sprintln": true, "Interface": true, "Any": true, "Stringer": true, "Msgf": true, "Msg": true, "Str": true, | |
| 155 | + | "Fatalf": true, "Logf": true, "Infof": true, "Warnf": true, "Format": true, "Marshal": true, "MarshalToString": true, | |
| 156 | + | "Err": true, "Object": true, "Dict": true, "Strs": true} | |
| 157 | + | // the messages by name | |
| 158 | + | messageName = `(?:(?:j|r|rt)\.job|job|spec|run|j\.run|[A-Za-z_.]+\.GetJob\(\)|[A-Za-z_.]+\.Job)` | |
| 159 | + | messageRe = regexp.MustCompile(`^` + messageName + `$`) | |
| 160 | + | // message.String(), and the secrets of a message or of anything | |
| 161 | + | stringRe = regexp.MustCompile(`^` + messageName + `\.String\(\)$`) | |
| 162 | + | secretsRe = regexp.MustCompile(`\.(?:Secrets|GetSecrets\(\))$`) | |
| 163 | + | ) | |
| 164 | + | ||
| 165 | + | func leaky(text string) bool { | |
| 166 | + | return messageRe.MatchString(text) || stringRe.MatchString(text) || secretsRe.MatchString(text) | |
| 167 | + | } | |
| 168 | + | ||
| 169 | + | // printLeaks returns the places of src where a message with secrets, or its | |
| 170 | + | // secrets, is given to a printing call, or where String() is called on such a | |
| 171 | + | // message. | |
| 172 | + | func printLeaks(t *testing.T, fset *token.FileSet, name string, src []byte) []string { | |
| 173 | + | t.Helper() | |
| 174 | + | f, err := parser.ParseFile(fset, name, src, 0) | |
| 175 | + | if err != nil { | |
| 176 | + | t.Fatal(err) | |
| 177 | + | } | |
| 178 | + | text := func(e ast.Expr) string { | |
| 179 | + | var b strings.Builder | |
| 180 | + | _ = format.Node(&b, fset, e) | |
| 181 | + | return b.String() | |
| 182 | + | } | |
| 183 | + | var out []string | |
| 184 | + | report := func(e ast.Expr, what string) { | |
| 185 | + | out = append(out, fmt.Sprintf("%s: %s: %s would print secrets", fset.Position(e.Pos()), what, text(e))) | |
| 186 | + | } | |
| 187 | + | // visit looks for leaky expressions inside an argument of a printing call. A | |
| 188 | + | // field read of a message (j.job.JobId) is a scalar: the message is not | |
| 189 | + | // descended into. | |
| 190 | + | var visit func(e ast.Expr, what string) | |
| 191 | + | visit = func(e ast.Expr, what string) { | |
| 192 | + | if e == nil { | |
| 193 | + | return | |
| 194 | + | } | |
| 195 | + | if leaky(text(e)) { | |
| 196 | + | report(e, what) | |
| 197 | + | return | |
| 198 | + | } | |
| 199 | + | switch x := e.(type) { | |
| 200 | + | case *ast.SelectorExpr: | |
| 201 | + | if !messageRe.MatchString(text(x.X)) { | |
| 202 | + | visit(x.X, what) | |
| 203 | + | } | |
| 204 | + | case *ast.CallExpr: | |
| 205 | + | if id, ok := x.Fun.(*ast.Ident); ok && (id.Name == "len" || id.Name == "cap") { | |
| 206 | + | return // a count is not a secret | |
| 207 | + | } | |
| 208 | + | if sel, ok := x.Fun.(*ast.SelectorExpr); ok { | |
| 209 | + | if !messageRe.MatchString(text(sel.X)) { | |
| 210 | + | visit(sel.X, what) | |
| 211 | + | } | |
| 212 | + | } else { | |
| 213 | + | visit(x.Fun, what) | |
| 214 | + | } | |
| 215 | + | for _, a := range x.Args { | |
| 216 | + | visit(a, what) | |
| 217 | + | } | |
| 218 | + | case *ast.UnaryExpr: | |
| 219 | + | visit(x.X, what) | |
| 220 | + | case *ast.StarExpr: | |
| 221 | + | visit(x.X, what) | |
| 222 | + | case *ast.ParenExpr: | |
| 223 | + | visit(x.X, what) | |
| 224 | + | case *ast.BinaryExpr: | |
| 225 | + | visit(x.X, what) | |
| 226 | + | visit(x.Y, what) | |
| 227 | + | case *ast.IndexExpr: | |
| 228 | + | visit(x.X, what) | |
| 229 | + | case *ast.CompositeLit: | |
| 230 | + | for _, el := range x.Elts { | |
| 231 | + | if kv, ok := el.(*ast.KeyValueExpr); ok { | |
| 232 | + | visit(kv.Value, what) | |
| 233 | + | } else { | |
| 234 | + | visit(el, what) | |
| 235 | + | } | |
| 236 | + | } | |
| 237 | + | } | |
| 238 | + | } | |
| 239 | + | ast.Inspect(f, func(n ast.Node) bool { | |
| 240 | + | call, ok := n.(*ast.CallExpr) | |
| 241 | + | if !ok { | |
| 242 | + | return true | |
| 243 | + | } | |
| 244 | + | if stringRe.MatchString(text(call)) { | |
| 245 | + | report(call, "String()") | |
| 246 | + | return true | |
| 247 | + | } | |
| 248 | + | if sel, ok := call.Fun.(*ast.SelectorExpr); ok && printers[sel.Sel.Name] { | |
| 249 | + | for _, a := range call.Args { | |
| 250 | + | visit(a, sel.Sel.Name+"()") | |
| 251 | + | } | |
| 252 | + | } | |
| 253 | + | return true | |
| 254 | + | }) | |
| 255 | + | return out | |
| 256 | + | } | |
| 257 | + | ||
| 258 | + | // The guard finds what it is meant to find and leaves scalar fields alone. | |
| 259 | + | func TestPrintLeaksHeuristic(t *testing.T) { | |
| 260 | + | positive := []string{ | |
| 261 | + | `fmt.Sprintf("%v", spec)`, | |
| 262 | + | `fmt.Errorf("bad %+v", j.job)`, | |
| 263 | + | `log.Info().Interface("job", j.job).Msg("x")`, | |
| 264 | + | `log.Info().Any("run", run).Msg("x")`, | |
| 265 | + | `_ = spec.String()`, | |
| 266 | + | `_ = j.job.String()`, | |
| 267 | + | `x := run.GetJob().String()`, | |
| 268 | + | `log.Debug().Str("s", fmt.Sprint(j.job.Secrets))`, | |
| 269 | + | `log.Debug().Interface("s", j.job.Secrets).Msg("x")`, | |
| 270 | + | `log.Debug().Interface("s", job.GetSecrets()).Msg("x")`, | |
| 271 | + | `fmt.Println(run.GetJob())`, | |
| 272 | + | `fmt.Println("job", r.job)`, | |
| 273 | + | `_ = prototext.Format(run)`, | |
| 274 | + | `_, _ = json.Marshal(j.job)`, | |
| 275 | + | `fmt.Println(strings.ToUpper(spec.String()))`, | |
| 276 | + | `fmt.Println(&j.job)`, | |
| 277 | + | `fmt.Println([]any{j.job})`, | |
| 278 | + | } | |
| 279 | + | negative := []string{ | |
| 280 | + | `fmt.Sprintf("%s", j.job.JobId)`, | |
| 281 | + | `log.Info().Str("job_id", job.JobId).Int64("run_id", job.RunId).Msg("x")`, | |
| 282 | + | `fmt.Errorf("job %s: %w", j.job.JobKey, err)`, | |
| 283 | + | `log.Info().Str("k", run.GetJob().GetJobKey()).Msg("x")`, | |
| 284 | + | `_ = fmt.Sprint(len(j.job.Secrets))`, | |
| 285 | + | `h := JobHandler{Secrets: j.job.Secrets, Vars: j.job.Vars}`, | |
| 286 | + | `_ = proto.Marshal(frame)`, | |
| 287 | + | `fmt.Println(spec.GetRepo())`, | |
| 288 | + | `_ = other.String()`, | |
| 289 | + | `secrets := j.job.Secrets; _ = secrets`, | |
| 290 | + | } | |
| 291 | + | fset := token.NewFileSet() | |
| 292 | + | for _, c := range positive { | |
| 293 | + | if got := printLeaks(t, fset, "p.go", []byte("package p\nfunc f() {\n"+c+"\n}\n")); len(got) == 0 { | |
| 294 | + | t.Errorf("not detected: %s", c) | |
| 295 | + | } | |
| 296 | + | } | |
| 297 | + | for _, c := range negative { | |
| 298 | + | if got := printLeaks(t, fset, "n.go", []byte("package p\nfunc f() {\n"+c+"\n}\n")); len(got) != 0 { | |
| 299 | + | t.Errorf("false positive: %s: %v", c, got) | |
| 300 | + | } | |
| 301 | + | } | |
| 302 | + | } | |
| @@ -0,0 +1,302 @@ | |||||
| 1 | + | // SPDX-License-Identifier: MIT | |||
| 2 | + | ||||
| 3 | + | package host | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "context" | |||
| 7 | + | "fmt" | |||
| 8 | + | "go/ast" | |||
| 9 | + | "go/format" | |||
| 10 | + | "go/parser" | |||
| 11 | + | "go/token" | |||
| 12 | + | "os" | |||
| 13 | + | "path/filepath" | |||
| 14 | + | "regexp" | |||
| 15 | + | "strings" | |||
| 16 | + | "testing" | |||
| 17 | + | ||||
| 18 | + | "google.golang.org/protobuf/proto" | |||
| 19 | + | ||||
| 20 | + | "bnema.dev/ermite-runner/internal/execstream/execpb" | |||
| 21 | + | ) | |||
| 22 | + | ||||
| 23 | + | const ( | |||
| 24 | + | secretValue = "s3cr3t-value-DO-NOT-LOG" | |||
| 25 | + | secretValue2 = "-----BEGIN-----\nline-two-DO-NOT-LOG\n-----END-----" | |||
| 26 | + | ) | |||
| 27 | + | ||||
| 28 | + | func withSecrets(trusted bool) func(*harness) { | |||
| 29 | + | return func(h *harness) { | |||
| 30 | + | j := testJob("j1") | |||
| 31 | + | j.Secrets = map[string]string{"DB_PASSWORD": secretValue, "KEY": secretValue2} | |||
| 32 | + | j.Vars = map[string]string{"REGION": "eu"} | |||
| 33 | + | j.Trusted = trusted | |||
| 34 | + | h.srv.jobs = nil | |||
| 35 | + | h.srv.jobs = append(h.srv.jobs, j) | |||
| 36 | + | } | |||
| 37 | + | } | |||
| 38 | + | ||||
| 39 | + | func runWithSecrets(t *testing.T, features []string, trusted bool) (*harness, *execpb.Run, string) { | |||
| 40 | + | t.Helper() | |||
| 41 | + | seenRun.Store(nil) | |||
| 42 | + | state := t.TempDir() | |||
| 43 | + | h := newHarness(t, resultsScript(nil)) | |||
| 44 | + | h.runner.Cfg.StateDir = state | |||
| 45 | + | h.runner.Features = func(context.Context) ([]string, error) { return features, nil } | |||
| 46 | + | withSecrets(trusted)(h) | |||
| 47 | + | stop := h.run(t) | |||
| 48 | + | eventually(t, "CompleteJob", func() bool { return h.srv.completeCount() == 1 }) | |||
| 49 | + | if err := stop(); err != nil { | |||
| 50 | + | t.Fatal(err) | |||
| 51 | + | } | |||
| 52 | + | return h, seenRun.Load(), state | |||
| 53 | + | } | |||
| 54 | + | ||||
| 55 | + | func TestSecretsAndVarsReachTheExecutorOnly(t *testing.T) { | |||
| 56 | + | h, run, state := runWithSecrets(t, []string{FeatureSecrets, FeatureAnnotations}, true) | |||
| 57 | + | if run == nil { | |||
| 58 | + | t.Fatal("no Run frame") | |||
| 59 | + | } | |||
| 60 | + | vals := lastValues.Load() | |||
| 61 | + | if vals == nil || vals.secrets["DB_PASSWORD"] != secretValue || vals.secrets["KEY"] != secretValue2 || len(vals.secrets) != 2 { | |||
| 62 | + | t.Errorf("secrets not passed: %+v", vals) | |||
| 63 | + | } | |||
| 64 | + | if vals.vars["REGION"] != "eu" || run.Job.SecretsWithheld { | |||
| 65 | + | t.Errorf("vars %v withheld %v", vals.vars, run.Job.SecretsWithheld) | |||
| 66 | + | } | |||
| 67 | + | // The Run frame itself holds no secret. | |||
| 68 | + | if rb, _ := proto.Marshal(run); strings.Contains(string(rb), "DO-NOT-LOG") { | |||
| 69 | + | t.Error("the Run frame holds a secret") | |||
| 70 | + | } | |||
| 71 | + | ||||
| 72 | + | // Nothing the host does prints the secrets: its log (debug level) and its | |||
| 73 | + | // state directory. (protobuf-go does not redact String(), so the guarantee | |||
| 74 | + | // is that no code formats the Job, the Run or the JobSpec; see the grep in | |||
| 75 | + | // TestNoCodePrintsTheJobMessages.) | |||
| 76 | + | if strings.Contains(h.logs.String(), "DO-NOT-LOG") { | |||
| 77 | + | t.Errorf("a secret is in the host log:\n%.500s", h.logs.String()) | |||
| 78 | + | } | |||
| 79 | + | err := filepath.Walk(state, func(p string, info os.FileInfo, err error) error { | |||
| 80 | + | if err != nil || info.IsDir() { | |||
| 81 | + | return err | |||
| 82 | + | } | |||
| 83 | + | b, _ := os.ReadFile(p) | |||
| 84 | + | if strings.Contains(string(b), "DO-NOT-LOG") { | |||
| 85 | + | t.Errorf("a secret is in the state directory: %s", p) | |||
| 86 | + | } | |||
| 87 | + | return nil | |||
| 88 | + | }) | |||
| 89 | + | if err != nil { | |||
| 90 | + | t.Fatal(err) | |||
| 91 | + | } | |||
| 92 | + | } | |||
| 93 | + | ||||
| 94 | + | // The "not available" line only exists when the server has ci.secrets: an old | |||
| 95 | + | // server does not send Job.trusted, which then reads false. | |||
| 96 | + | func TestSecretsWithheldOnlyWithTheFeature(t *testing.T) { | |||
| 97 | + | for _, tc := range []struct { | |||
| 98 | + | name string | |||
| 99 | + | features []string | |||
| 100 | + | trusted bool | |||
| 101 | + | want bool | |||
| 102 | + | }{ | |||
| 103 | + | {"untrusted, server has ci.secrets", []string{FeatureSecrets}, false, true}, | |||
| 104 | + | {"trusted, server has ci.secrets", []string{FeatureSecrets}, true, false}, | |||
| 105 | + | {"untrusted, old server", []string{FeatureAnnotations}, false, false}, | |||
| 106 | + | {"untrusted, features unreachable", nil, false, false}, | |||
| 107 | + | } { | |||
| 108 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 109 | + | _, run, _ := runWithSecrets(t, tc.features, tc.trusted) | |||
| 110 | + | if run == nil || run.Job.SecretsWithheld != tc.want { | |||
| 111 | + | t.Errorf("secrets_withheld = %v, want %v", run != nil && run.Job.SecretsWithheld, tc.want) | |||
| 112 | + | } | |||
| 113 | + | }) | |||
| 114 | + | } | |||
| 115 | + | } | |||
| 116 | + | ||||
| 117 | + | // protobuf-go prints map values in String(), so nothing may format a message | |||
| 118 | + | // that holds secrets: the Job (ermite.Job, with Secrets) or the Run and JobSpec | |||
| 119 | + | // that wrap it. This is a heuristic on names, not a proof: printLeaks looks, | |||
| 120 | + | // in the sources of the host, the guest, execstream and the executor, for | |||
| 121 | + | // formatting, logging, marshalling and String() calls that are given a value | |||
| 122 | + | // named like those messages (job, j.job, r.job, spec, run, x.GetJob()), their | |||
| 123 | + | // Secrets field or GetSecrets(). It does not follow a message through other | |||
| 124 | + | // names or through values it was copied into. The real guarantees are the | |||
| 125 | + | // tests that read the host log and the state directory | |||
| 126 | + | // (TestSecretsAndVarsReachTheExecutorOnly) and the absence of secrets in the | |||
| 127 | + | // Run frame. | |||
| 128 | + | func TestNoCodePrintsTheJobMessages(t *testing.T) { | |||
| 129 | + | fset := token.NewFileSet() | |||
| 130 | + | for _, dir := range []string{".", "../guest", "../execstream", "../executor"} { | |||
| 131 | + | files, err := filepath.Glob(filepath.Join(dir, "*.go")) | |||
| 132 | + | if err != nil { | |||
| 133 | + | t.Fatal(err) | |||
| 134 | + | } | |||
| 135 | + | for _, file := range files { | |||
| 136 | + | if strings.HasSuffix(file, "_test.go") { | |||
| 137 | + | continue | |||
| 138 | + | } | |||
| 139 | + | src, err := os.ReadFile(file) | |||
| 140 | + | if err != nil { | |||
| 141 | + | t.Fatal(err) | |||
| 142 | + | } | |||
| 143 | + | for _, leak := range printLeaks(t, fset, file, src) { | |||
| 144 | + | t.Error(leak) | |||
| 145 | + | } | |||
| 146 | + | } | |||
| 147 | + | } | |||
| 148 | + | } | |||
| 149 | + | ||||
| 150 | + | var ( | |||
| 151 | + | // printers are the calls that render their arguments: fmt, loggers (zerolog | |||
| 152 | + | // events), the text and JSON encoders of messages. | |||
| 153 | + | printers = map[string]bool{"Errorf": true, "Printf": true, "Sprintf": true, "Fprintf": true, "Println": true, "Fprintln": true, | |||
| 154 | + | "Print": true, "Sprint": true, "Sprintln": true, "Interface": true, "Any": true, "Stringer": true, "Msgf": true, "Msg": true, "Str": true, | |||
| 155 | + | "Fatalf": true, "Logf": true, "Infof": true, "Warnf": true, "Format": true, "Marshal": true, "MarshalToString": true, | |||
| 156 | + | "Err": true, "Object": true, "Dict": true, "Strs": true} | |||
| 157 | + | // the messages by name | |||
| 158 | + | messageName = `(?:(?:j|r|rt)\.job|job|spec|run|j\.run|[A-Za-z_.]+\.GetJob\(\)|[A-Za-z_.]+\.Job)` | |||
| 159 | + | messageRe = regexp.MustCompile(`^` + messageName + `$`) | |||
| 160 | + | // message.String(), and the secrets of a message or of anything | |||
| 161 | + | stringRe = regexp.MustCompile(`^` + messageName + `\.String\(\)$`) | |||
| 162 | + | secretsRe = regexp.MustCompile(`\.(?:Secrets|GetSecrets\(\))$`) | |||
| 163 | + | ) | |||
| 164 | + | ||||
| 165 | + | func leaky(text string) bool { | |||
| 166 | + | return messageRe.MatchString(text) || stringRe.MatchString(text) || secretsRe.MatchString(text) | |||
| 167 | + | } | |||
| 168 | + | ||||
| 169 | + | // printLeaks returns the places of src where a message with secrets, or its | |||
| 170 | + | // secrets, is given to a printing call, or where String() is called on such a | |||
| 171 | + | // message. | |||
| 172 | + | func printLeaks(t *testing.T, fset *token.FileSet, name string, src []byte) []string { | |||
| 173 | + | t.Helper() | |||
| 174 | + | f, err := parser.ParseFile(fset, name, src, 0) | |||
| 175 | + | if err != nil { | |||
| 176 | + | t.Fatal(err) | |||
| 177 | + | } | |||
| 178 | + | text := func(e ast.Expr) string { | |||
| 179 | + | var b strings.Builder | |||
| 180 | + | _ = format.Node(&b, fset, e) | |||
| 181 | + | return b.String() | |||
| 182 | + | } | |||
| 183 | + | var out []string | |||
| 184 | + | report := func(e ast.Expr, what string) { | |||
| 185 | + | out = append(out, fmt.Sprintf("%s: %s: %s would print secrets", fset.Position(e.Pos()), what, text(e))) | |||
| 186 | + | } | |||
| 187 | + | // visit looks for leaky expressions inside an argument of a printing call. A | |||
| 188 | + | // field read of a message (j.job.JobId) is a scalar: the message is not | |||
| 189 | + | // descended into. | |||
| 190 | + | var visit func(e ast.Expr, what string) | |||
| 191 | + | visit = func(e ast.Expr, what string) { | |||
| 192 | + | if e == nil { | |||
| 193 | + | return | |||
| 194 | + | } | |||
| 195 | + | if leaky(text(e)) { | |||
| 196 | + | report(e, what) | |||
| 197 | + | return | |||
| 198 | + | } | |||
| 199 | + | switch x := e.(type) { | |||
| 200 | + | case *ast.SelectorExpr: | |||
| 201 | + | if !messageRe.MatchString(text(x.X)) { | |||
| 202 | + | visit(x.X, what) | |||
| 203 | + | } | |||
| 204 | + | case *ast.CallExpr: | |||
| 205 | + | if id, ok := x.Fun.(*ast.Ident); ok && (id.Name == "len" || id.Name == "cap") { | |||
| 206 | + | return // a count is not a secret | |||
| 207 | + | } | |||
| 208 | + | if sel, ok := x.Fun.(*ast.SelectorExpr); ok { | |||
| 209 | + | if !messageRe.MatchString(text(sel.X)) { | |||
| 210 | + | visit(sel.X, what) | |||
| 211 | + | } | |||
| 212 | + | } else { | |||
| 213 | + | visit(x.Fun, what) | |||
| 214 | + | } | |||
| 215 | + | for _, a := range x.Args { | |||
| 216 | + | visit(a, what) | |||
| 217 | + | } | |||
| 218 | + | case *ast.UnaryExpr: | |||
| 219 | + | visit(x.X, what) | |||
| 220 | + | case *ast.StarExpr: | |||
| 221 | + | visit(x.X, what) | |||
| 222 | + | case *ast.ParenExpr: | |||
| 223 | + | visit(x.X, what) | |||
| 224 | + | case *ast.BinaryExpr: | |||
| 225 | + | visit(x.X, what) | |||
| 226 | + | visit(x.Y, what) | |||
| 227 | + | case *ast.IndexExpr: | |||
| 228 | + | visit(x.X, what) | |||
| 229 | + | case *ast.CompositeLit: | |||
| 230 | + | for _, el := range x.Elts { | |||
| 231 | + | if kv, ok := el.(*ast.KeyValueExpr); ok { | |||
| 232 | + | visit(kv.Value, what) | |||
| 233 | + | } else { | |||
| 234 | + | visit(el, what) | |||
| 235 | + | } | |||
| 236 | + | } | |||
| 237 | + | } | |||
| 238 | + | } | |||
| 239 | + | ast.Inspect(f, func(n ast.Node) bool { | |||
| 240 | + | call, ok := n.(*ast.CallExpr) | |||
| 241 | + | if !ok { | |||
| 242 | + | return true | |||
| 243 | + | } | |||
| 244 | + | if stringRe.MatchString(text(call)) { | |||
| 245 | + | report(call, "String()") | |||
| 246 | + | return true | |||
| 247 | + | } | |||
| 248 | + | if sel, ok := call.Fun.(*ast.SelectorExpr); ok && printers[sel.Sel.Name] { | |||
| 249 | + | for _, a := range call.Args { | |||
| 250 | + | visit(a, sel.Sel.Name+"()") | |||
| 251 | + | } | |||
| 252 | + | } | |||
| 253 | + | return true | |||
| 254 | + | }) | |||
| 255 | + | return out | |||
| 256 | + | } | |||
| 257 | + | ||||
| 258 | + | // The guard finds what it is meant to find and leaves scalar fields alone. | |||
| 259 | + | func TestPrintLeaksHeuristic(t *testing.T) { | |||
| 260 | + | positive := []string{ | |||
| 261 | + | `fmt.Sprintf("%v", spec)`, | |||
| 262 | + | `fmt.Errorf("bad %+v", j.job)`, | |||
| 263 | + | `log.Info().Interface("job", j.job).Msg("x")`, | |||
| 264 | + | `log.Info().Any("run", run).Msg("x")`, | |||
| 265 | + | `_ = spec.String()`, | |||
| 266 | + | `_ = j.job.String()`, | |||
| 267 | + | `x := run.GetJob().String()`, | |||
| 268 | + | `log.Debug().Str("s", fmt.Sprint(j.job.Secrets))`, | |||
| 269 | + | `log.Debug().Interface("s", j.job.Secrets).Msg("x")`, | |||
| 270 | + | `log.Debug().Interface("s", job.GetSecrets()).Msg("x")`, | |||
| 271 | + | `fmt.Println(run.GetJob())`, | |||
| 272 | + | `fmt.Println("job", r.job)`, | |||
| 273 | + | `_ = prototext.Format(run)`, | |||
| 274 | + | `_, _ = json.Marshal(j.job)`, | |||
| 275 | + | `fmt.Println(strings.ToUpper(spec.String()))`, | |||
| 276 | + | `fmt.Println(&j.job)`, | |||
| 277 | + | `fmt.Println([]any{j.job})`, | |||
| 278 | + | } | |||
| 279 | + | negative := []string{ | |||
| 280 | + | `fmt.Sprintf("%s", j.job.JobId)`, | |||
| 281 | + | `log.Info().Str("job_id", job.JobId).Int64("run_id", job.RunId).Msg("x")`, | |||
| 282 | + | `fmt.Errorf("job %s: %w", j.job.JobKey, err)`, | |||
| 283 | + | `log.Info().Str("k", run.GetJob().GetJobKey()).Msg("x")`, | |||
| 284 | + | `_ = fmt.Sprint(len(j.job.Secrets))`, | |||
| 285 | + | `h := JobHandler{Secrets: j.job.Secrets, Vars: j.job.Vars}`, | |||
| 286 | + | `_ = proto.Marshal(frame)`, | |||
| 287 | + | `fmt.Println(spec.GetRepo())`, | |||
| 288 | + | `_ = other.String()`, | |||
| 289 | + | `secrets := j.job.Secrets; _ = secrets`, | |||
| 290 | + | } | |||
| 291 | + | fset := token.NewFileSet() | |||
| 292 | + | for _, c := range positive { | |||
| 293 | + | if got := printLeaks(t, fset, "p.go", []byte("package p\nfunc f() {\n"+c+"\n}\n")); len(got) == 0 { | |||
| 294 | + | t.Errorf("not detected: %s", c) | |||
| 295 | + | } | |||
| 296 | + | } | |||
| 297 | + | for _, c := range negative { | |||
| 298 | + | if got := printLeaks(t, fset, "n.go", []byte("package p\nfunc f() {\n"+c+"\n}\n")); len(got) != 0 { | |||
| 299 | + | t.Errorf("false positive: %s: %v", c, got) | |||
| 300 | + | } | |||
| 301 | + | } | |||
| 302 | + | } | |||