Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.
vole
Clone

Read only, no credentials:

https://rodent.software/git/vole.git

Write over SSH with a registered key:

git clone ssh://git@0.0.0.0:2222/vole.git

A free and open-source forge that lives in your Git repo.

Verify the runner's host and re-check the job it is given

f5456470d22fb7ca06197a8856ed0da7fc56fdd2 · Keenan Nemetz · 12d ago · parent edf9db6e 

Verify the runner's host and re-check the job it is given

A runner executes repository code, so the instance it talks to decides what
runs on it. The client built ssh command lines with no host-key policy: a
man in the middle, or a loose known_hosts, could answer in the instance's
place and feed a runner any job it liked.

Host keys are now verified against known_hosts by default, for the
runner's control channel, for git's own clone/fetch/push and for the LFS
transfer. `--known-hosts` names a file instead of the default,
VOLE_KNOWN_HOSTS does the same for every command, and
`--insecure-host-key` / VOLE_INSECURE_HOST_KEY is the explicit override
for tests and throwaway instances, with a warning when it is used. The
options are placed before any configured ones because ssh takes the first
value for an option, so GIT_SSH_COMMAND or VOLE_SSH_COMMAND cannot
quietly turn the check off. The test package opts out once, in TestMain,
rather than every fixture learning to trust everything.

The runner also re-checks the job it leases before acting on it. The
server validated the workflow at push time, but the runner is the party
that turns the payload into paths, container arguments and scripts. Run
id, repository, attempt, commit, ref, branch, tag, workflow and job names,
image (or image-file plus its pinned digest), steps, artifact patterns,
pages path and environment are all validated; a malformed job stops the
runner with the reason instead of being built from.

The artifact and pages path rules move into ci/validate.go, so the
push-time check and the runner use one implementation rather than two
that can drift. Joining them also closes the gap the runner-side check
was for: `..`, absolute paths, `**`, globs where a directory belongs, and
control characters are refused identically on both sides.

Tests: strict-by-default and known_hosts options, the environment losing
to the explicit policy, the insecure override, and every job field
refused when it climbs out, is the wrong shape, or is missing.

ci/agent/validate.go +164

@@ -0,0 +1,164 @@
1 package agent
2
3 // Validation of a leased job.
4 //
5 // A job arrives from the instance over SSH. The server validated the workflow when
6 // it was pushed, but the runner is the party that acts on the job, and the
7 // channel is a trust boundary: a compromised or buggy instance, or a MITM that
8 // got past a loose host-key check, would otherwise be able to name a repository
9 // outside the cache, point the worktree at a commit that is not a commit, or aim
10 // a site deploy at a directory above the checkout. So every field the runner
11 // builds a path or a command out of is checked again here, before any of it is
12 // used.
13
14 import (
15 "errors"
16 "fmt"
17 "strings"
18 "time"
19
20 "github.com/rodent-software/vole/ci"
21 "github.com/rodent-software/vole/core/id"
22 "github.com/rodent-software/vole/forge/repo"
23 )
24
25 // validateJob reports every reason a leased job must not be run. It is called
26 // before the checkout, the container, the cache mount and the artifact patterns
27 // exist, so a refusal costs nothing but the message.
28 func validateJob(job *runnerJob) error {
29 if job == nil {
30 return errors.New("the lease carried no job")
31 }
32 var problems []string
33 add := func(format string, args ...any) {
34 problems = append(problems, fmt.Sprintf(format, args...))
35 }
36
37 if !id.Valid(job.ID) {
38 add("run id %q is not a run id", job.ID)
39 }
40 if err := repo.ValidateName(job.Repo); err != nil {
41 add("repository %q: %v", job.Repo, err)
42 }
43 if job.Attempt < 1 {
44 add("attempt %d is not a positive attempt number", job.Attempt)
45 }
46 if !ci.ValidObjectID(job.SHA) {
47 add("commit %q is not a commit id", job.SHA)
48 }
49 if !ci.ValidRef(job.Ref) {
50 add("ref %q is not a branch or tag", job.Ref)
51 }
52 if job.Branch != "" && !ci.ValidRefName(job.Branch) {
53 add("branch %q is not a branch name", job.Branch)
54 }
55 if job.Tag != "" && !ci.ValidRefName(job.Tag) {
56 add("tag %q is not a tag name", job.Tag)
57 }
58 if job.WorkflowSHA != "" && !ci.ValidObjectID(job.WorkflowSHA) {
59 add("workflow revision %q is not a commit id", job.WorkflowSHA)
60 }
61 if job.StepsSHA != "" && !isDigest(job.StepsSHA) {
62 add("steps digest %q is not a digest", job.StepsSHA)
63 }
64 for _, field := range []struct{ name, value string }{
65 {"workflow", job.Workflow},
66 {"job", job.Job},
67 } {
68 if field.value == "" || len(field.value) > 256 || strings.ContainsAny(field.value, "\x00\r\n") {
69 add("%s name %q is not a name", field.name, field.value)
70 }
71 }
72
73 if job.Image == "" {
74 add("the job names no image")
75 } else if name, ok := strings.CutPrefix(job.Image, "file:"); ok {
76 // An image-file definition is named, not pathed: the runner resolves the name
77 // under .vole/images itself, so a name with a slash or a dot-dot could not
78 // reach outside the checkout and is refused anyway.
79 if _, err := ci.ImageName(name); err != nil {
80 add("image: %v", err)
81 }
82 if job.ImageFile != name {
83 add("image file %q disagrees with the image reference %q", job.ImageFile, job.Image)
84 }
85 if !isDigest(job.ImageSHA) {
86 add("image file %q has no pinned digest", job.ImageFile)
87 }
88 } else if err := ci.ValidImageRef(job.Image); err != nil {
89 add("image: %v", err)
90 } else if job.ImageFile != "" || job.ImageSHA != "" {
91 add("image %q names a definition but is not a file: reference", job.Image)
92 }
93
94 if len(job.Steps) > ci.MaxSteps {
95 add("%d steps, limit %d", len(job.Steps), ci.MaxSteps)
96 }
97 for i, step := range job.Steps {
98 if strings.TrimSpace(step.Name) == "" || len(step.Name) > 256 ||
99 strings.ContainsAny(step.Name, "\x00\r\n") {
100 add("steps[%d]: name %q is not a name", i, step.Name)
101 }
102 if strings.TrimSpace(step.Run) == "" || len(step.Run) > ci.MaxStepBytes {
103 add("steps[%d]: script is empty or longer than %d bytes", i, ci.MaxStepBytes)
104 }
105 if step.Timeout != "" {
106 d, err := time.ParseDuration(step.Timeout)
107 if err != nil || d <= 0 || d > ci.MaxTimeout {
108 add("steps[%d]: timeout %q is not a duration between 0 and %s", i, step.Timeout, ci.MaxTimeout)
109 }
110 }
111 }
112
113 for i, pattern := range job.Artifacts {
114 if err := ci.ValidateArtifactPath(pattern); err != nil {
115 add("artifacts[%d]: %v", i, err)
116 }
117 }
118 for i, pattern := range job.ReleaseArtifacts {
119 if err := ci.ValidateArtifactPath(pattern); err != nil {
120 add("release_artifacts[%d]: %v", i, err)
121 }
122 }
123 if job.Pages != nil {
124 if err := ci.ValidatePagesPath(job.Pages.Path); err != nil {
125 add("pages: %v", err)
126 }
127 if job.Pages.Ref != "" && !ci.ValidRefName(job.Pages.Ref) {
128 add("pages: ref %q is not a branch name", job.Pages.Ref)
129 }
130 }
131
132 if len(job.Env) > ci.MaxEnv {
133 add("%d environment variables, limit %d", len(job.Env), ci.MaxEnv)
134 }
135 for name, value := range job.Env {
136 if !ci.ValidEnvName(name) {
137 add("env: %q is not a variable name", name)
138 continue
139 }
140 if len(value) > ci.MaxEnvValue || strings.ContainsAny(value, "\r\n\x00") {
141 add("env.%s: the value is too long or not one line", name)
142 }
143 }
144
145 if len(problems) == 0 {
146 return nil
147 }
148 return fmt.Errorf("refusing job %s from the instance: %s", job.ID, strings.Join(problems, "; "))
149 }
150
151 // isDigest reports whether s is a hex SHA-256 digest, which is what the step
152 // digest the instance sends is.
153 func isDigest(s string) bool {
154 if len(s) != 64 {
155 return false
156 }
157 for i := 0; i < len(s); i++ {
158 c := s[i]
159 if (c < '0' || c > '9') && (c < 'a' || c > 'f') {
160 return false
161 }
162 }
163 return true
164 }