Support GitHub job container options and volumes - #373
Conversation
![buildsworth-bk-app[bot]](https://avatars.githubusercontent.com/in/3010600?s=60&v=4){kind=small}
There was a problem hiding this comment.
One cleanup hole remains for an accepted volume list; details inline.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 19630, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
About buildsworth
How to request a review: Comment @buildsworth-bk review on the PR, or request buildsworth-bk as a reviewer.
Risk labels (how buildsworth classifies risk) — buildsworth classifies risk itself from the diff. Unless repository policy already allows L2 approval, grant it by mentioning @buildsworth-bk (see approval ceiling and L2 approval grant):
- L1 — Low risk (dep bumps, docs/copy, lockfiles, small presentational fixes). buildsworth may approve by default.
- L2 — Standard risk (new UI, additive API fields, refactors). Approved only when repository policy or a verified grant allows it; otherwise comment-only.
- L3 — High risk (auth, migrations, payments, secrets, perf-critical paths). Human review always required.
|
@buildsworth-bk review |
Previous concerns have been addressed.
There was a problem hiding this comment.
The duplicate-target cleanup path is fixed. One related validation gap can still leave an earlier named volume behind; details inline.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 19634, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
|
@buildsworth-bk review |
Previous concerns have been addressed.
There was a problem hiding this comment.
The one-character volume-name gap is fixed. One cancellation-time cleanup path remains; details inline.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 19638, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
Previous concern has been addressed.
There was a problem hiding this comment.
The earlier findings are addressed. I found two remaining edge cases in raw-container adaptation and cancellation cleanup; details inline.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 19642, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
There was a problem hiding this comment.
Both previous blocking concerns remain open on the current revision; I found no new issues.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 19642, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
Previous concerns have been addressed.
There was a problem hiding this comment.
The previous blocking concerns are fixed, and I found no new issues. This changes the job-container security boundary, so I'm leaving final approval to a human.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 19647, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
lox
changed the title
There was a problem hiding this comment.
The job-plan schema still rejects values the compiler now accepts; details inline.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 19657, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
Previous concerns have been addressed.
There was a problem hiding this comment.
The schema contract is aligned now. One new cleanup path can still leave an ambiguously-created container behind; details inline.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 19659, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
Previous concerns have been addressed.
There was a problem hiding this comment.
The previous container-reconciliation concern is fixed. A related volume cleanup path is still incomplete; details inline.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 19662, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
Previous concerns have been addressed.
There was a problem hiding this comment.
The previous ambiguous-create volume reconciliation concern is fixed. One separate successful-create timeout path can still leave a named volume behind; details inline.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 19663, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
Previous concerns have been addressed.
There was a problem hiding this comment.
The previous volume-tracking concern is fixed. A separate setup-failure path can treat the pre-existing Docker volume set as job-owned and remove it; details inline. I also noted one documented anonymous-volume form that Docker rejects.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 19665, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
Previous concerns have been addressed.
There was a problem hiding this comment.
The previous concerns are fixed, and I found no new issues. This expands the job-container security boundary through broad Docker create options and host bind mounts, so I’m leaving final approval to a human.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 19666, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
wolfeidau
left a comment
wolfeidau
left a comment
There was a problem hiding this comment.
Couple of small observations.
3c3e7f5 to
2b2f8e6
Compare
There was a problem hiding this comment.
I found no new issues. Broad Docker create options and host bind mounts expand job-container host authority, so I’m leaving final approval to a human.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 19917, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
Amp-Thread-ID: https://ampcode.com/threads/T-01a02836-fd56-769a-b44a-a7412431fd1d Co-authored-by: Lachlan Donald <lachlan@buildkite.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a02836-fd56-769a-b44a-a7412431fd1d Co-authored-by: Lachlan Donald <lachlan@buildkite.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a02836-fd56-769a-b44a-a7412431fd1d Co-authored-by: Lachlan Donald <lachlan@buildkite.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a02836-fd56-769a-b44a-a7412431fd1d Co-authored-by: Lachlan Donald <lachlan@buildkite.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a02836-fd56-769a-b44a-a7412431fd1d Co-authored-by: Lachlan Donald <lachlan@buildkite.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a02836-fd56-769a-b44a-a7412431fd1d Co-authored-by: Lachlan Donald <lachlan@buildkite.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a02836-fd56-769a-b44a-a7412431fd1d Co-authored-by: Lachlan Donald <lachlan@buildkite.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a02836-fd56-769a-b44a-a7412431fd1d Co-authored-by: Lachlan Donald <lachlan@buildkite.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a02836-fd56-769a-b44a-a7412431fd1d Co-authored-by: Lachlan Donald <lachlan@buildkite.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a02836-fd56-769a-b44a-a7412431fd1d Co-authored-by: Lachlan Donald <lachlan@buildkite.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a02836-fd56-769a-b44a-a7412431fd1d Co-authored-by: Lachlan Donald <lachlan@buildkite.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a02836-fd56-769a-b44a-a7412431fd1d Co-authored-by: Lachlan Donald <lachlan@buildkite.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a02836-fd56-769a-b44a-a7412431fd1d Co-authored-by: Lachlan Donald <lachlan@buildkite.com>
Amp-Thread-ID: https://ampcode.com/threads/T-01a02836-fd56-769a-b44a-a7412431fd1d Co-authored-by: Lachlan Donald <lachlan@buildkite.com>
2b2f8e6 to
79a8dff
Compare
There was a problem hiding this comment.
Broad Docker create options and host bind mounts expand job-container host authority, so I’m leaving final approval to a human. I found one non-blocking workflow-program completeness gap inline.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 20113, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
Amp-Thread-ID: https://ampcode.com/threads/T-01a02836-fd56-769a-b44a-a7412431fd1d Co-authored-by: Lachlan Donald <lachlan@buildkite.com>
There was a problem hiding this comment.
The workflow-program inventory gap is fixed. Broad Docker create options and host bind mounts expand job-container host authority, so final approval still needs a human.
Want to dig deeper?
Paste this into your agent to explore the findings from this review's Buildkite build:
Download the buildsworth logs from build 20126, then answer my questions about the findings.
Install the reading-buildsworth-logs skill to run this.
3 participants
Why
GitHub Actions workflows can pass Docker create options and mount named, anonymous, or host volumes on job containers.
buildkite-gharejected these fields; in a partial corpus of 68,194 public workflows, container options blocked 377 workflows and container volumes blocked 101.What
Pass job container options to
docker createas parsed arguments without a shell, matching the existing service-container path. Reject--network,--net, and--entrypoint, including--flag=value, so the runner retains its network and entrypoint. Options reject expressions, line breaks, NUL bytes, and values over 65,536 bytes. Registry credentials remain out of scope.Support GitHub job volumes as
DESTINATIONfor anonymous volumes andSOURCE:DESTINATION[:ro|rw]for named volumes or absolute host bind mounts. Destinations and host sources must be absolute; named sources use Docker volume names. Expressions, duplicate declarations, anonymous volume modes, and lists over 128 entries are rejected. Cleanup snapshots pre-existing volumes, preserves them, and removes only volumes created during the job, including ambiguous create and cancellation paths.General Docker options and host bind mounts can grant broad host authority. The private network, ownership tracking, masking, and cleanup reduce accidental residue; whole-job queue isolation remains the security boundary.