chadtmiller.com/knotless

Switch repository

Loading…

What changes

A step can use a step another repository shares. A workflow step with uses: <owner>/<repo>/<step>@<version> is put in place of by the steps of .knotless/steps/<step>.yml in that repository, in the workflow's own container, named <step> / <theirs>. with gives its inputs, which it sees as INPUT_<NAME>, and secrets the secrets it may see. The step's repository is checked out beside the checkout at the locked commit, as KNOTLESS_STEP_DIR, so a step keeps its scripts as files. A step file can use shared steps itself, at most four deep; a cycle fails the run.

A lock pins each one to a commit, and a command writes it. The using repository keeps .knotless/uses.lock, mapping each uses to a version, a full commit and the release. A step is released as the package @step/<step>, its one file the step file, with --commit. knotless uses update pins each uses (and the ones they use) to the highest release its @ takes, checking the released file against the one at its commit by hash; named ones only, if given. The runner reads the step file at the locked commit from its mirror, never live. A missing or wrong lock is an error check before any step runs.

Steps from a stranger make a pull untrusted. A pull, or a batch holding one, whose lock adds an account the base's lock lacks and the repository does not vouch for runs as an agent's pull does (caches apart, nothing kept, no reused pass), and its log says ==> runs as code no one vouches for, as its lock adds shared steps from <accounts>.

Each step gets its secrets as it starts. The container no longer holds them: each docker exec is given -e NAME for each secret that step may see. A plain step still sees every secret granted; a step from a shared one only those its uses passes. In a workflow using shared steps, what a step given secrets leaves running is ended when it ends.

The design is docs/design/reusable-workflows.md; keys and messages in docs/reference/workflows.md#shared-steps, the command in docs/reference/cli.md.

Why

knotless and grain, now on the same CI setup, paste the same blocks between workflows: installing npm packages when the lockfile changes (8 copies), waiting for this commit's checks before deploying (5, already drifted), Node at a pinned version (4), and more. GitHub shares such steps as actions resolved by tag at run time, which is how tj-actions/changed-files's moved tags ran a malicious commit in 23,000 repositories in March 2025. A lock pinning commits means a shared step changes only through a reviewed change to the repository using it.

Decided with the owner: many steps per repository (each its own release package); the runner keeps a mirror rather than fetching per run; no outputs at first; only steps in a directory, later.

How it was checked

• Unit tests in uses.rs (parsing uses, versions, the lock and its owners, step files, expansion with inputs, needs, secrets and nested steps) and workflow.rs, run locally.
• a_workflow_uses_a_shared_step (knotless-run): pinned commit over a later failing one, inputs and secrets per step, a script from KNOTLESS_STEP_DIR, a nested step, a cycle, a chain too deep, and each lock error.
• a_pull_runs_its_head gains a collaborator's pull whose lock adds the runner's account and the owner's: it runs untrusted and its log names only the runner's.
• shared_steps_are_pinned (CLI): releases at two commits, the highest @1 takes over a pre-release and a 2.0.0, nested pins, update <uses> moving only that one, and every refusal.
• executors and what_a_step_leaves_running_is_ended cover the per-step -e and ending what is left, run locally. Everything else is left to CI.

Shortcomings

• The rest of the shared container is shared: a shared step can read files earlier steps wrote. Only a container per step would keep that apart, at the cost of the tools earlier steps installed.
• Ending what a step given secrets leaves running is docker only, and only in a workflow that uses shared steps, so existing workflows that start a server in one step for the next are unchanged.
• No outputs, by choice; no directory of steps on the pages yet; private step repositories untested.
• uses update has no MCP tool, as it writes a checkout's file; MCP's publish releases a step.

Review

The design first: docs/design/reusable-workflows.md. Then crates/knotless/src/uses.rs, the trust change (strangers in lib.rs, its two uses in queue.rs), and the secrets change in crates/knotless-run/src/executor.rs (Session::command) and lib.rs (vars, run_steps), which changes every run, not only those using shared steps.

🤖 Generated with Claude Code

This adds uses: <owner>/<repo>/<step>@<version>, with the lock in .knotless/uses.lock pinning each use to a commit. It also moves secrets from the container's environment to a per-step -e NAME on docker exec.

I found nothing that must change. I read uses.rs in full, plus the Session::command, run_steps and vars paths. I did not read the whole diff: it was too large to open, and I read the tests, docs and run.rs only through searches. I did see the changeset and the docs pages (docs/design/reusable-workflows.md, docs/reference/workflows.md), so what AGENTS.md asks to change with the code is there.

• Lock: it needs a full 40-hex commit, and the version must be one @ takes. The step file is read at the locked commit from the runner's mirror, and every failure is an error check before any step runs.
• Secrets: a plain step has secrets: None, so it still sees every granted secret. An expanded step gets Some(passed), and expand rejects a secret the step file doesn't ask for. A uses that passes no secrets sees none. Pulls still get no secrets, since vars is unchanged there.
• Injection: needs entries are validated as plain command names and the step name is validated, so the generated check script can't be injected into.

Before merging:

• test, e2e, docs and check were still queued when I looked. Confirm they pass, especially test, which carries the 100% coverage gate and the new a_workflow_uses_a_shared_step run test.
• The executors test in executor.rs and one_container_a_run still expect -e SECRET on docker run. If the container really no longer holds secrets, check that those expectations are right. Since the change file says it does, I assume test will show whether they are.

The shortcomings you list (no uses update, the untrusted-lock tier, a shared step sharing the container) are fine to leave for follow-ups.

The only change since my last review is c7b230c..ad88ed2, which rewords docs/design/reusable-workflows.md to describe shared steps in general rather than through this repository's workflows. It touches no code, so my earlier findings stand and I have nothing new to ask for.

My earlier review asked for two things, both still open: test, e2e, docs and check are still queued, and I haven't seen them pass. Please confirm test in particular, since it carries the coverage gate, the new a_workflow_uses_a_shared_step test, and the executors and one_container_a_run expectations for secrets now moving from docker run to docker exec.

Since my last review there is one new commit, 99ac329. It adds knotless uses update, nested shared steps (at most four deep), the step repository checked out beside the checkout as KNOTLESS_STEP_DIR, and an untrusted tier for a pull whose lock adds an account the repository doesn't vouch for. It also ends whatever a step given secrets leaves running. That covers the "not built" shortcomings from the description.

The diff for this commit is about 1,800 lines and I read only part of it. I looked at the run_steps change and the Step fields in workflow.rs. I saw that the step repositories are mounted read-only at /steps under docker, so a shared step can't alter another's files. I found nothing that must change in what I read. I did not read the uses update code or the trust-tier code, so I have no view on them.

My earlier request is still open. test, e2e, docs and check for this commit are queued or running, and web, shots and lexicons were skipped. Please don't merge until test passes. It carries the coverage gate. It also covers the new a_workflow_uses_a_shared_step test, which now exercises KNOTLESS_STEP_DIR. The executors and one_container_a_run expectations for secrets moving from docker run to docker exec are covered there as well. The new code (update, nesting, the trust tier, the leftover-process cleanup) needs tests that reach the coverage gate.

The only change since my last review is 79bbc26, which makes the pinning test read the CLI's output trimmed. It touches no runtime code, so my earlier findings stand and I have nothing new to ask for.

check passed (fmt, clippy, changes). web, shots and lexicons were skipped as expected. test, e2e and docs were still queued when I looked.

My earlier request is still open: please don't merge until test passes. It carries the 100% coverage gate and runs a_workflow_uses_a_shared_step, shared_steps_are_pinned, executors and one_container_a_run. Those last two cover secrets moving from docker run to docker exec.

I did not read the uses update code or the trust-tier code in strangers and queue.rs, so I have no view on them.

This is the fifth round, and I have no new findings. The change adds uses: shared steps pinned by .knotless/uses.lock, per-step secrets on docker exec, and an untrusted tier for pulls whose lock adds an account the repository doesn't vouch for. I did not re-read the diff this round.

The check and docs checks pass at this commit. web, shots and lexicons were skipped as expected. test and e2e are still running, so please don't merge until test passes. It carries the 100% coverage gate and runs a_workflow_uses_a_shared_step, shared_steps_are_pinned, executors and one_container_a_run.

I never read the uses update code or the strangers and queue.rs trust-tier code, so I have no view on them.