Skip to content

Commit 8f3b811

Browse files
antitreeclaudeegibs
authored
feat: add QEMU_ADDITIONAL_PACKAGES environment variable (#2266)
* feat: add QEMU_ADDITIONAL_PACKAGES environment variable Add support for QEMU_ADDITIONAL_PACKAGES environment variable that allows users to specify additional packages to install in the QEMU microVM during initialization. The variable accepts a comma-separated list of package names (e.g., "hello-wolfi,nginx-stable,strace") and passes them to microvm-init via the kernel command line as melange.additional_packages=<list>. Input validation prevents injection attacks by only allowing alphanumeric characters, hyphens, underscores, commas, and dots. Invalid input is rejected with a warning. Usage: QEMU_ADDITIONAL_PACKAGES=hello-wolfi,strace melange build mypackage.yaml Note: This requires a corresponding update to microvm-init package to read and process the melange.additional_packages kernel parameter. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]> * fix: use strings.SplitSeq for modernization linter Update to use strings.SplitSeq() instead of strings.Split() for better efficiency with the iterator pattern in Go 1.24+. Addresses golangci-lint modernize check. * refactor: extract testable functions and add tests for QEMU_ADDITIONAL_PACKAGES Addresses PR feedback from @egibs and @89luca89: - Extract getAdditionalPackages() function for parsing env var - Extract getPackageCacheSuffix() function for cache key generation - Use SHA256 hash instead of truncation to avoid collisions - Add comprehensive test coverage for both functions - Fix variable shadowing issue Tests verify: - Package parsing and validation - Security (injection prevention) - Cache suffix generation with SHA256 - Hash determinism and collision prevention 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <[email protected]> * Apply suggestion from @egibs Co-authored-by: Evan Gibler <[email protected]> Signed-off-by: antitree <[email protected]> * Retrieve additional packages from context Signed-off-by: antitree <[email protected]> * refactoring egibs changes * fix: remove duplicate getAdditionalPackages call that shadows parameter The merge introduced a bug where additionalPkgs parameter was being shadowed by a local variable that called getAdditionalPackages again. This defeats the purpose of passing the parameter and causes the function to be called twice unnecessarily. Remove the duplicate call to use the passed parameter correctly. --------- Signed-off-by: antitree <[email protected]> Co-authored-by: Claude Sonnet 4.5 <[email protected]> Co-authored-by: Evan Gibler <[email protected]>
1 parent 3dbccea commit 8f3b811

File tree

2 files changed

+246
-6
lines changed

2 files changed

+246
-6
lines changed

pkg/container/qemu_runner.go

Lines changed: 61 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"crypto/ed25519"
2424
"crypto/elliptic"
2525
"crypto/rand"
26+
"crypto/sha256"
2627
_ "embed"
2728
"encoding/base64"
2829
"encoding/pem"
@@ -35,6 +36,7 @@ import (
3536
"os/signal"
3637
"os/user"
3738
"path/filepath"
39+
"regexp"
3840
"runtime"
3941
"strconv"
4042
"strings"
@@ -1762,16 +1764,20 @@ func generateCpio(ctx context.Context, cfg *Config) (string, error) {
17621764

17631765
cacheDir = filepath.Join(cacheDir, "melange-cpio")
17641766

1767+
// Include additional packages in cache filename to invalidate cache when they change
1768+
additionalPkgs := getAdditionalPackages(ctx)
1769+
cacheSuffix := getPackageCacheSuffix(additionalPkgs)
1770+
17651771
baseInitramfs := filepath.Join(
17661772
cacheDir,
1767-
"melange-guest.initramfs.cpio")
1773+
fmt.Sprintf("melange-guest%s.initramfs.cpio", cacheSuffix))
17681774
initramfsInfo, err := os.Stat(baseInitramfs)
17691775

17701776
// Check if we can use the cached base initramfs (less than 24h old)
17711777
useCache := err == nil && time.Since(initramfsInfo.ModTime()) < 24*time.Hour
17721778

17731779
if !useCache {
1774-
err = generateBaseInitramfs(ctx, cfg, baseInitramfs, cacheDir)
1780+
err = generateBaseInitramfs(ctx, cfg, baseInitramfs, cacheDir, additionalPkgs)
17751781
if err != nil {
17761782
return "", err
17771783
}
@@ -1782,22 +1788,24 @@ func generateCpio(ctx context.Context, cfg *Config) (string, error) {
17821788

17831789
// generateBaseInitramfs creates the base initramfs with microvm-init.
17841790
// This is cached for performance as it doesn't change often.
1785-
func generateBaseInitramfs(ctx context.Context, cfg *Config, initramfsPath, cacheDir string) error {
1791+
func generateBaseInitramfs(ctx context.Context, cfg *Config, initramfsPath, cacheDir string, additionalPkgs []string) error {
17861792
clog.FromContext(ctx).Info("qemu: generating base initramfs")
17871793

17881794
err := os.MkdirAll(cacheDir, 0o755)
17891795
if err != nil {
17901796
return fmt.Errorf("unable to create dest directory: %w", err)
17911797
}
17921798

1799+
// Start with base packages and add any additional packages from environment
1800+
packages := []string{"microvm-init"}
1801+
packages = append(packages, additionalPkgs...)
1802+
17931803
spec := apko_types.ImageConfiguration{
17941804
Contents: apko_types.ImageContents{
17951805
BuildRepositories: []string{
17961806
"https://apk.cgr.dev/chainguard",
17971807
},
1798-
Packages: []string{
1799-
"microvm-init",
1800-
},
1808+
Packages: packages,
18011809
},
18021810
}
18031811
opts := []apko_build.Option{
@@ -1857,6 +1865,53 @@ func generateBaseInitramfs(ctx context.Context, cfg *Config, initramfsPath, cach
18571865
return nil
18581866
}
18591867

1868+
// getAdditionalPackages parses and validates the QEMU_ADDITIONAL_PACKAGES environment variable.
1869+
// Returns a list of package names to add to the initramfs, or empty slice if none/invalid.
1870+
func getAdditionalPackages(ctx context.Context) []string {
1871+
pkgEnv := os.Getenv("QEMU_ADDITIONAL_PACKAGES")
1872+
if pkgEnv == "" {
1873+
return nil
1874+
}
1875+
1876+
// Basic validation: check for suspicious characters that could cause injection
1877+
// Allow: alphanumeric, hyphens, underscores, commas, dots
1878+
if matched, _ := regexp.MatchString(`^[a-zA-Z0-9_,.-]+$`, pkgEnv); !matched {
1879+
clog.FromContext(ctx).Warnf("qemu: QEMU_ADDITIONAL_PACKAGES contains invalid characters, ignoring: %s", pkgEnv)
1880+
return nil
1881+
}
1882+
1883+
clog.FromContext(ctx).Infof("qemu: QEMU_ADDITIONAL_PACKAGES env set to %s, adding to initramfs", pkgEnv)
1884+
1885+
// Split comma-separated list and filter empty entries
1886+
var packages []string
1887+
for pkg := range strings.SplitSeq(pkgEnv, ",") {
1888+
pkg = strings.TrimSpace(pkg)
1889+
if pkg != "" {
1890+
packages = append(packages, pkg)
1891+
}
1892+
}
1893+
1894+
return packages
1895+
}
1896+
1897+
// getPackageCacheSuffix generates a deterministic cache suffix based on the package list.
1898+
// Uses SHA256 hash (first 12 chars) to avoid collisions and keep filenames reasonable.
1899+
// Returns empty string if packages list is empty.
1900+
func getPackageCacheSuffix(packages []string) string {
1901+
if len(packages) == 0 {
1902+
return ""
1903+
}
1904+
1905+
// Join packages with commas to create a stable input string
1906+
pkgString := strings.Join(packages, ",")
1907+
1908+
// Generate SHA256 hash
1909+
hash := sha256.Sum256([]byte(pkgString))
1910+
1911+
// Use first 12 characters of hex encoding (like git short SHA)
1912+
return fmt.Sprintf("-%x", hash[:6])
1913+
}
1914+
18601915
// injectHostKeyIntoCpio creates a new initramfs by concatenating the base
18611916
// initramfs with a secondary cpio containing the VM's SSH host key.
18621917
// This allows us to use cached base initramfs while injecting fresh keys each time.

pkg/container/qemu_runner_test.go

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,13 @@
1515
package container
1616

1717
import (
18+
"context"
19+
"os"
1820
"runtime"
1921
"testing"
22+
23+
"github.com/chainguard-dev/clog"
24+
"github.com/chainguard-dev/clog/slogtest"
2025
)
2126

2227
func TestGetAvailableMemoryKB(t *testing.T) {
@@ -106,3 +111,183 @@ func TestGetAvailableMemoryKB_Fallback(t *testing.T) {
106111
t.Errorf("Fallback value %d KB seems too low", expectedFallback)
107112
}
108113
}
114+
115+
func TestGetAdditionalPackages(t *testing.T) {
116+
ctx := clog.WithLogger(context.Background(), slogtest.TestLogger(t))
117+
118+
tests := []struct {
119+
name string
120+
envValue string
121+
expected []string
122+
}{
123+
{
124+
name: "empty environment variable",
125+
envValue: "",
126+
expected: nil,
127+
},
128+
{
129+
name: "single package",
130+
envValue: "hello-wolfi",
131+
expected: []string{"hello-wolfi"},
132+
},
133+
{
134+
name: "multiple packages",
135+
envValue: "hello-wolfi,nginx-stable,strace",
136+
expected: []string{"hello-wolfi", "nginx-stable", "strace"},
137+
},
138+
{
139+
name: "packages with spaces around commas rejected",
140+
envValue: "hello-wolfi, nginx-stable , strace",
141+
expected: nil,
142+
},
143+
{
144+
name: "packages with dots and underscores",
145+
envValue: "python-3.11,gcc_musl,nginx-1.25.0",
146+
expected: []string{"python-3.11", "gcc_musl", "nginx-1.25.0"},
147+
},
148+
{
149+
name: "empty entries filtered out",
150+
envValue: "hello-wolfi,,nginx-stable",
151+
expected: []string{"hello-wolfi", "nginx-stable"},
152+
},
153+
{
154+
name: "invalid characters rejected",
155+
envValue: "hello-wolfi;rm -rf /",
156+
expected: nil,
157+
},
158+
{
159+
name: "shell injection attempt",
160+
envValue: "package$(evil_command)",
161+
expected: nil,
162+
},
163+
{
164+
name: "quotes rejected",
165+
envValue: "\"hello-wolfi\"",
166+
expected: nil,
167+
},
168+
{
169+
name: "spaces in package name rejected",
170+
envValue: "hello wolfi",
171+
expected: nil,
172+
},
173+
}
174+
175+
for _, tt := range tests {
176+
t.Run(tt.name, func(t *testing.T) {
177+
// Set environment variable
178+
if tt.envValue == "" {
179+
os.Unsetenv("QEMU_ADDITIONAL_PACKAGES")
180+
} else {
181+
os.Setenv("QEMU_ADDITIONAL_PACKAGES", tt.envValue)
182+
}
183+
defer os.Unsetenv("QEMU_ADDITIONAL_PACKAGES")
184+
185+
result := getAdditionalPackages(ctx)
186+
187+
// Compare results
188+
if len(result) != len(tt.expected) {
189+
t.Errorf("getAdditionalPackages() returned %d packages, expected %d: got %v, want %v",
190+
len(result), len(tt.expected), result, tt.expected)
191+
return
192+
}
193+
194+
for i, pkg := range result {
195+
if pkg != tt.expected[i] {
196+
t.Errorf("getAdditionalPackages()[%d] = %q, expected %q", i, pkg, tt.expected[i])
197+
}
198+
}
199+
})
200+
}
201+
}
202+
203+
func TestGetPackageCacheSuffix(t *testing.T) {
204+
tests := []struct {
205+
name string
206+
packages []string
207+
expected string
208+
}{
209+
{
210+
name: "empty package list",
211+
packages: []string{},
212+
expected: "",
213+
},
214+
{
215+
name: "nil package list",
216+
packages: nil,
217+
expected: "",
218+
},
219+
{
220+
name: "single package",
221+
packages: []string{"hello-wolfi"},
222+
expected: "-f5c4369d6487",
223+
},
224+
{
225+
name: "multiple packages deterministic",
226+
packages: []string{"hello-wolfi", "nginx-stable", "strace"},
227+
expected: "-8315f3ef029a",
228+
},
229+
{
230+
name: "same packages different order produces different hash",
231+
packages: []string{"strace", "hello-wolfi", "nginx-stable"},
232+
expected: "-5236a484f919",
233+
},
234+
}
235+
236+
for _, tt := range tests {
237+
t.Run(tt.name, func(t *testing.T) {
238+
result := getPackageCacheSuffix(tt.packages)
239+
240+
if result != tt.expected {
241+
t.Errorf("getPackageCacheSuffix(%v) = %q, expected %q", tt.packages, result, tt.expected)
242+
}
243+
244+
// Additional validation: check format
245+
if len(tt.packages) > 0 {
246+
// Should start with dash and have 12 hex characters
247+
if len(result) != 13 { // "-" + 12 hex chars
248+
t.Errorf("getPackageCacheSuffix(%v) = %q, expected 13 characters (dash + 12 hex)", tt.packages, result)
249+
}
250+
if result[0] != '-' {
251+
t.Errorf("getPackageCacheSuffix(%v) = %q, expected to start with '-'", tt.packages, result)
252+
}
253+
}
254+
})
255+
}
256+
}
257+
258+
func TestGetPackageCacheSuffix_Deterministic(t *testing.T) {
259+
// Test that the same packages always produce the same hash
260+
packages := []string{"hello-wolfi", "nginx-stable", "strace"}
261+
262+
result1 := getPackageCacheSuffix(packages)
263+
result2 := getPackageCacheSuffix(packages)
264+
265+
if result1 != result2 {
266+
t.Errorf("getPackageCacheSuffix is not deterministic: first call returned %q, second call returned %q", result1, result2)
267+
}
268+
}
269+
270+
func TestGetPackageCacheSuffix_NoCollisions(t *testing.T) {
271+
// Test that different package lists produce different hashes
272+
testCases := [][]string{
273+
{"package-a", "package-b", "package-c", "package-d", "package-e"},
274+
{"package-a", "package-b", "package-c", "package-d", "package-f"},
275+
{"hello-wolfi"},
276+
{"hello-wolfi", "nginx-stable"},
277+
{"nginx-stable", "hello-wolfi"}, // Order matters
278+
}
279+
280+
hashes := make(map[string][]string)
281+
for _, packages := range testCases {
282+
hash := getPackageCacheSuffix(packages)
283+
hashes[hash] = packages
284+
}
285+
286+
// Should have unique hashes for each unique package list
287+
if len(hashes) != len(testCases) {
288+
t.Errorf("getPackageCacheSuffix produced collisions: %d unique hashes for %d test cases", len(hashes), len(testCases))
289+
for hash, packages := range hashes {
290+
t.Logf("Hash %q: %v", hash, packages)
291+
}
292+
}
293+
}

0 commit comments

Comments
 (0)