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.

#56

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

merged · by keenan 12d ago · merged into main at f5456470 by keenan 12d ago

keenan opened this 12d ago

Part 1 of #11 (does not close it; part 2 adds the resource caps). Closes the “strict SSH host-key verification” and “validate every job field” items.

What was wrong

client.SSHCommand built an ssh line with no host-key policy, so a runner trusted whatever host answered. That covers the control channel (the job payload), the LFS transfer, and — through GIT_SSH_COMMAND — git’s own clone/fetch/push.

The runner also acted on every field of a leased job without re-checking it. The server validates a workflow at push time, but the runner is the party that turns the payload into filepath.Joins, a --volume, a site deploy and a script, and the channel is the trust boundary the threat model names.

What this does

Host keys. client.HostKeys is a process policy: strict against known_hosts by default, --known-hosts <path> to name a file, VOLE_KNOWN_HOSTS for commands without a flag, and an explicit --insecure-host-key / VOLE_INSECURE_HOST_KEY override that warns when used. The options go before any configured ones, because ssh takes the first value for an option, so a GIT_SSH_COMMAND=... StrictHostKeyChecking=no in the environment cannot loosen the policy. GitSSHCommand() applies the same policy to git’s children, which is how the clone, the record push and the job stream all verify the same host. client/remote_test.go pins all of it.

The test suite uses the explicit override once, in TestMain, instead of every fixture drifting toward trust-everything.

Job validation. ci/agent/validate.go re-checks a leased job before the checkout, the cache, the container or the artifact patterns exist: run id, repository name, attempt, commit id, ref (branch/tag, with git’s component rules), branch, tag, workflow and job names, image or file: definition plus its pinned digest, step count/names/scripts/timeouts, artifact and release artifact patterns, pages path and ref, and the environment. A refusal stops the runner with the reason rather than looping on a payload that will not improve.

Shared rules. ci/validate.go now owns the artifact and pages path rules and the ref/object-id/env predicates; ci/workflow.go calls them instead of keeping its own copy, so the push-time refusal and the runner-side refusal cannot disagree. Validation also gains the checks the runner needed anyway: NUL and control characters in a pages path, and : before a path becomes a --volume argument.

Notes

  • --insecure-host-key is required for the integration fixtures and is set in the test package’s TestMain. It is a real behaviour change for operators: existing runners need ssh-keyscan into known_hosts or --known-hosts. docs/runner.md has a new “Host key verification” section with the command.
  • An operator’s own GIT_SSH_COMMAND still supplies the program and -i key; only the verification options are forced. That is the intended split: they own the key, the instance’s identity is not negotiable.
  • The pages/artifact validators are shared, but the server still composes the messages with jobs.%s... context; the wording is now generated in one place.

Verification

  • gofmt, go vet ./..., go test -race ./... clean.
  • ./vole workflow check and ./vole workflow run verify pass.

Tests

  • client: strict by default, --known-hosts, the environment’s =no losing to the policy, the insecure override, env parsing, and GitSSHCommand keeping the operator’s program and key.
  • ci: refs (namespace, .., //, .lock, control characters), object ids (40/64 hex, uppercase refused), artifact patterns and pages paths.
  • ci/agent: a valid job passes; a table of 22 mutations is each refused with the offending field named, including repo=../../etc, pages.path=../../etc, artifact=../secret, a non-commit SHA, a ref with .., a tag with a newline, an envelope with a newline, an image reference that starts with -, and an image-file without a digest.

Discussion (0)

No discussion yet.