Skip to content

Commit d33aad4

Browse files
dmcilvaneyPawelWMS
authored andcommitted
docs(ai): Add instructions for running azldev in workflows
Co-authored-by: Pawel Winogrodzki <pawelwi@microsoft.com>
1 parent 1b9fc98 commit d33aad4

1 file changed

Lines changed: 109 additions & 0 deletions

File tree

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
---
2+
applyTo: ".github/workflows/**"
3+
description: ALWAYS review these instructions when reading or modifying PR check workflows, or any scripts referenced by the workflows.
4+
---
5+
6+
# PR Check Workflow Guidelines
7+
8+
## Fork-PR-safe pattern: stub + reusable
9+
10+
Problem: `pull_request` triggers on fork PRs run without secrets and with a read-only token. `pull_request_target` runs with write access but checks out the BASE ref by default — easy to footgun into RCE if you then check out PR code with full privileges.
11+
12+
Pattern:
13+
14+
1. **Stub workflow** on the default branch — triggered by `pull_request_target`, guards on repo owner, calls the reusable workflow. This is the only file GitHub will load from the base branch, so it locks the entrypoint.
15+
2. **Reusable workflow** (`workflow_call`) holds the real logic. Lives on the PR branch, so contributors can iterate on it.
16+
3. Stub passes `pull-requests: write` / `contents: read` only. Reusable declares its own minimum permissions.
17+
18+
Never check out PR code into a privileged job and then execute it on the host. Either:
19+
- Run untrusted code inside a container with no secrets mounted, **or**
20+
- Keep the privileged job read-only (lint, comment-post) and isolate code execution to a separate unprivileged job.
21+
22+
## Prefer in-container for anything that executes PR code
23+
24+
If the check builds, renders, or runs PR code, do the whole thing inside the build container. Mock is a critical component of many azldev workflows. It requires many privileges to run successfully. It also is not available in Ubuntu, which is the default runner image for GitHub Actions.
25+
26+
- Mount the PR checkout read-only when possible; if writes are needed (e.g. `git add -N`), mount rw but don't leak host paths or secrets.
27+
- Produce **all outputs** (reports, patches, diffs) inside the container and write them to a bind-mounted output dir. Host-side steps then only read these artifacts (json report, patch files, etc.)
28+
- This eliminates a huge class of config-driven git RCE vectors (`core.fsmonitor`, `core.sshCommand`, hook files, etc.) because the host never runs git against PR-controlled config.
29+
30+
### Container config
31+
32+
The shared runner image is [`.github/workflows/containers/azldev-runner.Dockerfile`](../workflows/containers/azldev-runner.Dockerfile). It's a minimal Azure Linux base with `mock`, `git`, `python3`, `sudo`, and `azldev` itself (installed to `/usr/local/bin` during image build) — enough to run any `azldev` subcommand. Reuse it rather than building a per-check image; add extras via a derived `FROM localhost/azldev-runner` stage if a check genuinely needs more.
33+
34+
`azldev` is baked in via `go install …/azldev@main` during image build. The pin lives in the Dockerfile so it can be reviewed and bumped deliberately. Image build context is `.github/workflows/containers/` only — keep it that way so the build can never see PR-controlled files.
35+
36+
Build it with the caller's UID so bind-mounted writes don't end up root-owned:
37+
38+
```yaml
39+
- name: Build azldev runner
40+
run: |
41+
docker build \
42+
--build-arg UID=$(id -u) \
43+
-t localhost/azldev-runner \
44+
-f .github/workflows/containers/azldev-runner.Dockerfile \
45+
.github/workflows/containers/
46+
```
47+
48+
#### Bind-mount conventions
49+
50+
| Mount | Mode | Purpose |
51+
| ----- | ---- | ------- |
52+
| `pr-head/` → `/workdir` | rw | PR checkout. rw because `azldev` writes to `specs/`, `base/build/`, etc. |
53+
| `<host-output-dir>/` → `/output` | rw | Trusted-shape outputs (JSON reports, patches, ...) the container produces for the host to consume after the run. |
54+
| `.github/workflows/scripts/` → `/scripts` | ro | Helper scripts from the trusted base checkout. |
55+
56+
#### Sandbox flags (minimum viable for `mock`)
57+
58+
```yaml
59+
docker run --rm \
60+
--cap-add=SYS_ADMIN \
61+
--security-opt seccomp=unconfined \
62+
--security-opt apparmor=unconfined \
63+
...
64+
```
65+
66+
Why each one is needed:
67+
68+
- **`--cap-add=SYS_ADMIN`** — `mock` sets up mount namespaces for its chroot. Without this you get `mount … exit status 32` during chroot init.
69+
- **`--security-opt seccomp=unconfined`** — `mock` uses syscalls (`unshare`, `pivot_root`, etc.) that Docker's default seccomp profile blocks.
70+
- **`--security-opt apparmor=unconfined`** — `ubuntu-latest` ships the `docker-default` AppArmor profile, which blocks `mount -t tmpfs` on paths under `/var/lib/mock` **even with `SYS_ADMIN` granted**. This is the confusing one; symptom is the same `exit status 32` after seccomp is already unconfined.
71+
72+
Avoid `--privileged` — it grants every capability and removes cgroup restrictions, which is a much bigger blast radius than the three flags above.
73+
74+
`--security-opt no-new-privileges` would be nice but `mock`'s `userhelper` needs setuid, which that flag blocks.
75+
76+
#### Running commands in the container
77+
78+
Use `bash -eu -o pipefail -c '…'` as the entrypoint invocation so a failure inside the heredoc actually fails the step:
79+
80+
```yaml
81+
localhost/azldev-runner \
82+
bash -eu -o pipefail -c '
83+
azldev component render -q -a --clean-stale -O json > /output/render.json
84+
python3 /scripts/check_rendered_specs.py \
85+
--specs-dir "$(azldev config dump -q -f json | jq -r .project.renderedSpecsDir)" \
86+
--report /output/report.json \
87+
--patch /output/rendered-specs.patch
88+
'
89+
```
90+
91+
Use single-quotes around the `-c` payload so host-side `${{ … }}` interpolation doesn't leak into the container script. If you need to pass a host value in, use `-e VAR=…` and reference `"$VAR"` inside — same script-injection concern as any other shell step.
92+
93+
## Shell hardening in workflow steps
94+
95+
- Start every multi-line `run:` with `set -euo pipefail`.
96+
- Quote **every** expansion involving a workflow input, matrix value, or file path: `"${VAR}"`, not `$VAR`.
97+
- Never interpolate `${{ github.event.pull_request.* }}` directly into a shell script — assign to an `env:` var first, then reference as `"$VAR"`. Direct interpolation is a classic script-injection vector.
98+
- For paths that must stay inside the repo, resolve with `realpath -m` and verify they start with the repo root prefix before use.
99+
100+
## Markdown / HTML injection in PR comments
101+
102+
- Escape any PR-controlled string (file paths, error messages) before dropping into Markdown.
103+
- Prefer code spans (`` `path` ``) or fenced blocks for anything path-like.
104+
105+
## zizmor / pedantic linting
106+
107+
Workflows are linted with `zizmor --pedantic`.
108+
109+
Use `# zizmor: ignore[<rule>]` comments as an absolute last resort, and provide a comprehensive justification for why the rule is being ignored.

0 commit comments

Comments
 (0)