Run the self-hosted incremental build without --privileged by anchapin · Pull Request #5682 · NatLabRockies/OpenStudio

anchapin · 2026-11-06T22:52:01Z

incremental-build.yml runs on the self-hosted runner linux-openstudio-2 (vcpu36_1) in a container started with --privileged. A privileged container is root on the host, so any code that runs in the build could take over the runner machine and every later job on it. That includes a compromised dependency in an otherwise trusted PR. Nothing in the workflow needs it: no Docker-in-Docker, mounts, ptrace or device access.

Testing: this is a draft and isn't labeled yet, so it hasn't run. Add Pull Request - Ready for CI to build it on the runner. If something does need a privilege, the failing step will say which, and we can add that one capability (--cap-add ...) instead of --privileged.

Two related items from the self-hosted runner security review, not changed here:

NODE_TLS_REJECT_UNAUTHORIZED: 0 turns off TLS certificate checks for every JavaScript action in this job, presumably because of the NREL TLS-inspecting proxy. The fix is to trust the NREL Root CA in the image or on the runner (NODE_EXTRA_CA_CERTS), then remove this setting.
runs-on: linux-openstudio-2 targets the runner by name. It should use a pool label (e.g. linux-x64) once the runner has one, so machines can be added or swapped without editing the workflow.

🤖 Generated with Claude Code

A privileged container is root on the runner host, so any code in the build could take over linux-openstudio-2 and every later job on it. Nothing in this workflow needs extra privileges. Co-Authored-By: Claude <noreply@anthropic.com>

Copilot

🔵 Needs a closer look

The workflow has not run without --privileged, so dependency installation, packaging, and CTest remain unverified with the custom container image on linux-openstudio-2.

No open issues 🎉

What changed in this PR

Reduces host-access privileges for the self-hosted incremental build by removing --privileged from its job container.

Changes:

Preserves the container image, root user, locale setting, and volume mounts.
Leaves TLS verification and runner targeting unchanged; runner validation is still pending.

File	Description
`.github/workflows/incremental-build.yml`	Removes `--privileged` from the incremental build container options.

_{Review effort: Balanced}

anchapin · 2026-11-10T22:33:41Z

    container: # Define the Docker container for the job. All subsequent steps run inside it.
      image: nrel/openstudio-cmake-tools:noble-main
-      options: --privileged -u root -e "LANG=en_US.UTF-8" # These options are passed to the 'docker run' command internally
+      options: -u root -e "LANG=en_US.UTF-8" # These options are passed to the 'docker run' command internally


Dropping --privileged doesn't close the host-takeover path by itself.

The runner adds -v "/var/run/docker.sock":"/var/run/docker.sock" to every Linux job container, though this workflow never asks for it. It's in the docker create line under "Initialize containers" in the last run, and it comes from ContainerInfo.cs in actions/runner. With -u root, any step can still reach the host Docker daemon and ask it for a new container with whatever privileges and host mounts it likes. The earlier --privileged runs started without --userns=host, so the daemon isn't using userns-remap: unless it runs rootless, container root is host root.

Nothing in options: or volumes: can remove that mount: a second mount at the same path fails docker create. Closing it takes a runner-side change, for example:

container hooks (ACTIONS_RUNNER_CONTAINER_HOOKS) that create the job container without the socket and as a non-root user (the image's default user is ubuntu; -u root is what makes the job uid 0)

a rootless or userns-remapped daemon

ephemeral runners

The change is still worth having: seccomp and AppArmor come back on, and host devices and /sys are no longer exposed. But the description overstates what it fixes.

anchapin · 2026-11-10T22:33:42Z

+      options: -u root -e "LANG=en_US.UTF-8" # These options are passed to the 'docker run' command internally
      volumes: # envs don't work in volume definition for containers
        - "/srv/data/jenkins/docker-volumes/conan-data/.conan2:/github/home/.conan2" # Conan cache
        - "/srv/data/jenkins/docker-volumes/ubuntu-2404:/github/home/Ubuntu"


"Every later job on it" still holds, with no privilege needed.

The job runs as root with read-write access to state that later runs reuse as-is:

the persistent source and build tree on this line

the Conan home on line 52

the runner's reused workspace (/__w)

the ccache store

Anything one run's build writes there carries into the next PR, push or dispatch run, and into the packages those runs upload. None of this depends on --privileged, so it isn't caused by this PR. Closing it would mean not sharing writable state between PR runs, or rebuilding that state from a trusted run.

Separately, the actions/cache step on ~/.ccache restores nothing. ccache 4 keeps its store in ~/.cache/ccache unless ~/.ccache already exists, and the last run logged "Path(s) specified in the action for caching do(es) not exist" while ccache started with a full 2 GB store.

anchapin · 2026-11-10T22:33:43Z

    container: # Define the Docker container for the job. All subsequent steps run inside it.
      image: nrel/openstudio-cmake-tools:noble-main
-      options: --privileged -u root -e "LANG=en_US.UTF-8" # These options are passed to the 'docker run' command internally
+      options: -u root -e "LANG=en_US.UTF-8" # These options are passed to the 'docker run' command internally


os-4.0-dev still runs this job privileged on the same runner.

That branch has its own copy of this workflow: runs-on: linux-openstudio-2, options: --privileged -u root ..., and the jammy image. A pull_request run takes its workflow from the PR's merge ref. So after this merges, labeled PRs into os-4.0-dev will keep starting privileged containers here; for example, the open, already-labeled PR 5640 will on its next push.

develop hasn't been merged into os-4.0-dev since May, so the fix won't carry over on its own. Consider making the same one-line change on os-4.0-dev, or enforcing container options on the runner side so that no branch's copy can opt back in.

anchapin · 2026-11-10T22:33:44Z

    container: # Define the Docker container for the job. All subsequent steps run inside it.
      image: nrel/openstudio-cmake-tools:noble-main
-      options: --privileged -u root -e "LANG=en_US.UTF-8" # These options are passed to the 'docker run' command internally
+      options: -u root -e "LANG=en_US.UTF-8" # These options are passed to the 'docker run' command internally


Test plan: adding the label won't start a run.

on: pull_request (lines 7-8) has no types:, so only opened, synchronize and reopened start this workflow. Labeling fires labeled. Re-running the earlier skipped run reuses its original, unlabeled payload, so it is skipped again. Past PRs show the same pattern: PR 5626 ran only after a push that followed the label, and PR 5619 only after a close/reopen.

So you need a push, or a close/reopen, after adding the label. Until then build stays "skipped", and this could merge without the unprivileged container ever having run on the runner.

anchapin · 2026-11-10T22:33:45Z

-      options: --privileged -u root -e "LANG=en_US.UTF-8" # These options are passed to the 'docker run' command internally
+      options: -u root -e "LANG=en_US.UTF-8" # These options are passed to the 'docker run' command internally
      volumes: # envs don't work in volume definition for containers
        - "/srv/data/jenkins/docker-volumes/conan-data/.conan2:/github/home/.conan2" # Conan cache


This branch predates 7befcd6, which moved this volume to conan-data-gha.

The merge with develop is clean, and a pull_request run uses the merge ref, so it gets conan-data-gha. But a workflow_dispatch on this branch would run the branch's stale copy instead, and so would a push to incremental_build_debug. Dispatch is the natural fallback if labeling doesn't trigger a run. That would validate a different volume from the one that merges, and with aggressive_clean it would run conan remove "*" against the old conan-data cache.

Merging develop in after labeling fixes both problems: it removes the stale line and fires synchronize, which starts the labeled run.

anchapin · 2026-11-10T22:33:46Z

    container: # Define the Docker container for the job. All subsequent steps run inside it.
      image: nrel/openstudio-cmake-tools:noble-main
-      options: --privileged -u root -e "LANG=en_US.UTF-8" # These options are passed to the 'docker run' command internally
+      options: -u root -e "LANG=en_US.UTF-8" # These options are passed to the 'docker run' command internally


On the fallback in the description ("the failing step will say which, and we can add that one capability"):

A denial in an unprivileged container is usually a bare Operation not permitted or Permission denied, and it doesn't name a capability. Also, several things --privileged lifts aren't capabilities at all:

seccomp blocks such as personality(ADDR_NO_RANDOMIZE) or keyctl

AppArmor's deny mount

masked /proc and /sys paths

missing /dev nodes

Those need --security-opt apparmor=..., seccomp=... or systempaths=unconfined, or --device, not --cap-add. Nothing in this job looks like it needs any of them, so this only matters for diagnosing a failure if one shows up.

anchapin · 2026-11-10T22:33:47Z

    container: # Define the Docker container for the job. All subsequent steps run inside it.
      image: nrel/openstudio-cmake-tools:noble-main
-      options: --privileged -u root -e "LANG=en_US.UTF-8" # These options are passed to the 'docker run' command internally
+      options: -u root -e "LANG=en_US.UTF-8" # These options are passed to the 'docker run' command internally


Nit (optional, probably better as a follow-up): the image already sets LANG=en_US.UTF-8 and LC_ALL=en_US.UTF-8 (the openstudio-cmake-tools Dockerfile), so -e "LANG=en_US.UTF-8" is a no-op. full-build.yml passes the same flag in three matrix entries, so dropping it only here would add drift.

…ileged

github-actions · 2026-11-10T22:46:50Z

🧪 Test Results Dashboard

Summary

Metric	Value
Total Tests	4202
Passed	4202
Failed	0
Errors	0
Skipped	0
Success Rate	100.0%
Generated	2026-11-10 22:46:49 UTC

✅ All Tests Passed!

📊 Test Run Information

Run	XML File	Status
run1	`results.xml`	✅ Found
run2	`results.xml`	✅ Found

ci-commercialbuildings · 2026-11-10T23:11:33Z

CI Results for 218ad11:

Ubuntu 24.04:
- OpenStudio-3.12.0-alpha+218ad11658-Ubuntu-24.04-x86_64.deb
- OpenStudio-3.12.0-alpha+218ad11658-Ubuntu-24.04-x86_64.tar.gz
Ubuntu 22.04:
Darwin:
Windows:
Code Coverage:

anchapin marked this pull request as ready for review November 7, 2026 04:59

anchapin requested a balanced review from Copilot November 7, 2026 04:59

Copilot started reviewing on behalf of anchapin November 7, 2026 04:59

View session

Copilot AI reviewed Nov 7, 2026

View reviewed changes

anchapin commented Nov 10, 2026

View reviewed changes

anchapin added the Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. label Nov 10, 2026

Merge remote-tracking branch 'origin/develop' into incremental-unpriv…

218ad11

…ileged

anchapin mentioned this pull request Nov 10, 2026

Cherry-pick the unprivileged incremental-build container fix onto os-4.0-dev #5695

Open

Navigation Menu

Repository navigation

Run the self-hosted incremental build without --privileged - #5682

Run the self-hosted incremental build without --privileged#5682
anchapin wants to merge 2 commits into
developfrom
incremental-unprivileged

anchapin commented Nov 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

anchapin Nov 10, 2026

Uh oh!

anchapin Nov 10, 2026

Uh oh!

anchapin Nov 10, 2026

Uh oh!

anchapin Nov 10, 2026

Uh oh!

anchapin Nov 10, 2026

Uh oh!

anchapin Nov 10, 2026

Uh oh!

anchapin Nov 10, 2026

Uh oh!

github-actions Bot commented Nov 10, 2026

Uh oh!

ci-commercialbuildings commented Nov 10, 2026 •

edited

Loading

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

anchapin commented Nov 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

anchapin Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

anchapin Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

anchapin Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

anchapin Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

anchapin Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

anchapin Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

anchapin Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 10, 2026

🧪 Test Results Dashboard

Summary

✅ All Tests Passed!

📊 Test Run Information

Uh oh!

ci-commercialbuildings commented Nov 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ci-commercialbuildings commented Nov 10, 2026 •

edited

Loading