diff --git a/actions/setup/sh/cloud_hypervisor_host_preflight.sh b/actions/setup/sh/cloud_hypervisor_host_preflight.sh index f4c43a096a8..2300e3ec5b5 100755 --- a/actions/setup/sh/cloud_hypervisor_host_preflight.sh +++ b/actions/setup/sh/cloud_hypervisor_host_preflight.sh @@ -38,5 +38,10 @@ if ! test -e /dev/kvm; then exit 1 fi +if ! test -c /dev/kvm; then + echo "::error::/dev/kvm must be a character device." + exit 1 +fi + echo "runner is eligible for cloud-hypervisor preview" echo "::endgroup::" diff --git a/actions/setup/sh/cloud_hypervisor_kvm_access.sh b/actions/setup/sh/cloud_hypervisor_kvm_access.sh index f96074888f4..95d33b746cc 100755 --- a/actions/setup/sh/cloud_hypervisor_kvm_access.sh +++ b/actions/setup/sh/cloud_hypervisor_kvm_access.sh @@ -18,12 +18,21 @@ if [[ ! -e /dev/kvm ]]; then exit 1 fi +if [[ ! -c /dev/kvm ]]; then + echo "::error::/dev/kvm must be a character device." + exit 1 +fi + if ! command -v setfacl >/dev/null 2>&1; then echo "::error::setfacl is required to grant scoped access to /dev/kvm." exit 1 fi runner_uid="$(id -u)" +if [[ ! "${runner_uid}" =~ ^[0-9]+$ ]]; then + echo "::error::failed to resolve a numeric runner UID." + exit 1 +fi sudo setfacl -m "u:${runner_uid}:rw" /dev/kvm if [[ ! -r /dev/kvm || ! -w /dev/kvm ]]; then @@ -31,5 +40,15 @@ if [[ ! -r /dev/kvm || ! -w /dev/kvm ]]; then exit 1 fi +acl_output="$(getfacl -ncp /dev/kvm | sed 's/[[:space:]]#effective:.*$//' || true)" +if [[ -z "${acl_output}" ]]; then + echo "::error::failed to read /dev/kvm ACLs for verification." + exit 1 +fi +if ! grep -Eq "^user:${runner_uid}:rw-$" <<<"${acl_output}"; then + echo "::error::failed to verify scoped ACL entry for the runner user on /dev/kvm." + exit 1 +fi + echo "runner user has scoped read/write access to /dev/kvm" echo "::endgroup::" diff --git a/actions/setup/sh/cloud_hypervisor_setup_bundle.sh b/actions/setup/sh/cloud_hypervisor_setup_bundle.sh index 7932e857182..92442f07a3e 100755 --- a/actions/setup/sh/cloud_hypervisor_setup_bundle.sh +++ b/actions/setup/sh/cloud_hypervisor_setup_bundle.sh @@ -37,8 +37,26 @@ curl -fsSL -o "${bundle_root}/${manifest_name}" "${asset_base_url}/${manifest_na echo "downloaded release assets" echo "::endgroup::" +echo "::group::Validate cloud-hypervisor bundle archive structure" +archive_path="${bundle_root}/${asset_name}" +if tar -tzf "${archive_path}" | grep -E '(^/|(^|/)\.\.(/|$))' >/dev/null; then + echo "::error::cloud-hypervisor bundle contains unsafe archive paths" + exit 1 +fi +archive_table="$(tar -tvzf "${archive_path}")" +if [[ -z "${archive_table}" ]]; then + echo "::error::cloud-hypervisor bundle archive is empty" + exit 1 +fi +if grep -Eq '^[lh]' <<<"${archive_table}"; then + echo "::error::cloud-hypervisor bundle must not include symbolic or hard links" + exit 1 +fi +echo "archive structure validated" +echo "::endgroup::" + echo "::group::Extract cloud-hypervisor bundle" -tar -xzf "${bundle_root}/${asset_name}" -C "${extract_dir}" +tar --no-same-owner --no-same-permissions -xzf "${archive_path}" -C "${extract_dir}" echo "bundle extracted to ${extract_dir}" echo "::endgroup::" @@ -97,6 +115,21 @@ verify_sha256() { fi } +validate_extracted_file() { + local file="$1" + if [[ -z "${file}" || ! -f "${file}" || -L "${file}" ]]; then + echo "::error::invalid extracted cloud-hypervisor bundle file: ${file}" + exit 1 + fi + local real_file real_extract_dir + real_file="$(realpath "${file}")" + real_extract_dir="$(realpath "${extract_dir}")" + if [[ "${real_file}" != "${real_extract_dir}"/* ]]; then + echo "::error::extracted bundle file is outside expected directory: ${file}" + exit 1 + fi +} + # Artifact names are fixed by the gh-aw-firewall cloud-hypervisor release contract. binary_rel="cloud-hypervisor" kernel_rel="vmlinux.bin" @@ -115,6 +148,12 @@ if [[ -z "${binary_path}" || -z "${kernel_path}" || -z "${rootfs_path}" || -z "$ exit 1 fi +validate_extracted_file "${binary_path}" +validate_extracted_file "${kernel_path}" +validate_extracted_file "${rootfs_path}" +validate_extracted_file "${supervisor_path}" +validate_extracted_file "${virtiofsd_path}" + if [[ "$(dirname "${binary_path}")" != "$(dirname "${virtiofsd_path}")" ]]; then echo "::error::virtiofsd must be colocated with the cloud-hypervisor binary" exit 1 diff --git a/pkg/workflow/cloud_hypervisor_test.go b/pkg/workflow/cloud_hypervisor_test.go index fbd8a11587b..569f408807f 100644 --- a/pkg/workflow/cloud_hypervisor_test.go +++ b/pkg/workflow/cloud_hypervisor_test.go @@ -350,15 +350,15 @@ func TestCloudHypervisorShellScriptContent(t *testing.T) { }{ { script: "cloud_hypervisor_kvm_access.sh", - contains: []string{"RUNNER_ENVIRONMENT", "github-hosted", "ImageOS", "setfacl", "u:${runner_uid}:rw", "/dev/kvm", "-r /dev/kvm", "-w /dev/kvm"}, + contains: []string{"RUNNER_ENVIRONMENT", "github-hosted", "ImageOS", "setfacl", "u:${runner_uid}:rw", "/dev/kvm", "-c /dev/kvm", "getfacl -ncp /dev/kvm", "-r /dev/kvm", "-w /dev/kvm"}, }, { script: "cloud_hypervisor_host_preflight.sh", - contains: []string{"RUNNER_ENVIRONMENT", "github-hosted", "ImageOS", "/dev/kvm", "cloud-hypervisor preview"}, + contains: []string{"RUNNER_ENVIRONMENT", "github-hosted", "ImageOS", "/dev/kvm", "test -c /dev/kvm", "cloud-hypervisor preview"}, }, { script: "cloud_hypervisor_setup_bundle.sh", - contains: []string{"cloud-hypervisor-test-x86_64.tar.gz", "cloud-hypervisor-test-x86_64.SHA256SUMS", "cloud-hypervisor-test-x86_64.manifest.json", "vmlinux.bin", "rootfs.ext4", "awf-supervisor", "virtiofsd", "virtiofsd_path=", "virtiofsd_sha256="}, + contains: []string{"cloud-hypervisor-test-x86_64.tar.gz", "cloud-hypervisor-test-x86_64.SHA256SUMS", "cloud-hypervisor-test-x86_64.manifest.json", "archive structure validated", "tar --no-same-owner --no-same-permissions", "validate_extracted_file", "vmlinux.bin", "rootfs.ext4", "awf-supervisor", "virtiofsd", "virtiofsd_path=", "virtiofsd_sha256="}, }, } diff --git a/pkg/workflow/compiler_validators.go b/pkg/workflow/compiler_validators.go index 6247fa23f9f..878b796eaa8 100644 --- a/pkg/workflow/compiler_validators.go +++ b/pkg/workflow/compiler_validators.go @@ -309,6 +309,12 @@ func (c *Compiler) emitGeneralToolWarnings(workflowData *WorkflowData, markdownP "Unsupported hosts are rejected; gh-aw and AWF do not fall back to docker or gvisor.")) c.IncrementWarningCount() } + if isCloudHypervisorRuntime(workflowData) { + fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "warning", + "sandbox.agent.runtime: cloud-hypervisor uses a privileged KVM preview path with an attached MCP gateway topology. "+ + "Require a human security review before merge or rollout, and record explicit approval in your change process.")) + c.IncrementWarningCount() + } if workflowData.SafeOutputs != nil && workflowData.SafeOutputs.AssignToAgent != nil && workflowData.SafeOutputs.GitHubApp != nil && workflowData.SafeOutputs.AssignToAgent.GitHubToken == "" { fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr( diff --git a/pkg/workflow/compiler_validators_test.go b/pkg/workflow/compiler_validators_test.go index fd966e35c91..379376e78aa 100644 --- a/pkg/workflow/compiler_validators_test.go +++ b/pkg/workflow/compiler_validators_test.go @@ -191,7 +191,6 @@ func TestEmitExperimentalFeatureWarningsGHAWDetection(t *testing.T) { os.Stderr = w t.Cleanup(func() { os.Stderr = oldStderr - _ = w.Close() _ = r.Close() }) @@ -218,6 +217,40 @@ func TestEmitExperimentalFeatureWarningsGHAWDetection(t *testing.T) { } } +func TestEmitGeneralToolWarningsCloudHypervisorReviewTrigger(t *testing.T) { + compiler := NewCompiler() + workflowData := &WorkflowData{ + SandboxConfig: &SandboxConfig{ + Agent: &AgentSandboxConfig{ + Runtime: AgentRuntimeCloudHypervisor, + }, + }, + } + + oldStderr := os.Stderr + r, w, err := os.Pipe() + require.NoError(t, err) + os.Stderr = w + t.Cleanup(func() { + os.Stderr = oldStderr + _ = r.Close() + }) + + compiler.emitGeneralToolWarnings(workflowData, "test.md") + + require.NoError(t, w.Close()) + os.Stderr = oldStderr + + var buf bytes.Buffer + _, err = io.Copy(&buf, r) + require.NoError(t, err) + stderrOutput := buf.String() + + assert.Contains(t, stderrOutput, "sandbox.agent.runtime: cloud-hypervisor uses a privileged KVM preview path") + assert.Contains(t, stderrOutput, "Require a human security review before merge or rollout") + assert.Equal(t, 1, compiler.GetWarningCount()) +} + // TestValidatePermissions tests permission parsing and MCP tool constraint validation. func TestValidatePermissions(t *testing.T) { tests := []struct {