diff --git a/CHANGELOG.md b/CHANGELOG.md index b3e29ef..ab096f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.1.7] - 2026-05-10 + +### Security +- **APT `Clean()` now respects `DryRun`**. Previously, `apt autoclean` executed regardless of the `DryRun` option, silently defeating the safety mechanism users expect from dry-run mode. The YUM/Snap/Flatpak Clean implementations already honored DryRun; APT now matches that behavior. Regression tests added in `manager/apt/apt_clean_dryrun_test.go`. +- **Docker test runners no longer mask test failures**. The compose entrypoints used `bash -c` with `&&` chains and trailing `|| true` on fixture-generation steps; due to bash operator precedence, the `|| true` caught failures from earlier in the chain (including `go test`), letting failed tests pass CI silently. Switched to `bash -ec` with explicit `;` separators so test failures abort immediately while fixture-generation steps remain individually allowed to fail. +- Added `read_only: true` and `no-new-privileges:true` to the `test-all` aggregator service for defense-in-depth. + +### Changed +- **`PackageInfo` JSON output now uses snake_case field names** (e.g. `package_manager`, `new_version`, `additional_data`) instead of Go's default PascalCase. Required fields (`name`, `status`, `package_manager`) are always emitted; optional fields use `omitempty`. Consumers parsing JSON output must update field names. (#40, thanks @aijanai) + +### Fixed +- `go.mod` now declares `go 1.23.0` (full version) instead of `go 1.23`, resolving Go toolchain download failures in some environments. (#40) + ## [0.1.6] - 2025-11-01 ### Added @@ -37,14 +50,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Technical debt cleanup and APT Upgrade method fix - APT Upgrade method now correctly uses `apt install` for specific packages -## Recent Achievements ✅ +## Recent Achievements ✅ ### Architecture & Code Quality - ✅ **CommandRunner Architecture**: Complete architectural consistency (Issue #20, PR #26) - ✅ **APT & YUM executeCommand Pattern**: Centralized command execution, eliminated code duplication - ✅ **Technical Debt Cleanup**: Fixed APT Upgrade method bug, removed misleading TODOs, verified no resource leaks -### Security Enhancements +### Security Enhancements - ✅ **Security Enhancements**: Input validation for package names (Issue #23, PR #25) - ✅ **Command Injection Prevention**: Comprehensive ValidatePackageName implementation across all package managers @@ -77,7 +90,7 @@ Current development focus areas (see [GitHub Issues](https://github.com/bluet/sy - **Security scanning with Snyk** - Add to CI/CD pipeline - **CommandRunner migration** - Complete Snap and Flatpak integration (Issues #28, #29) -### Medium Priority Pending +### Medium Priority Pending - **Test coverage improvements** - YUM gaps (Issue #32), Snap & Flatpak comprehensive suites - **CLI improvements** - Upgrade display (Issue #3), macOS apt conflict (Issue #2) - **Code quality** - Context support, custom error types, DRY principle improvements @@ -90,7 +103,7 @@ Current development focus areas (see [GitHub Issues](https://github.com/bluet/sy ### Currently Supported ✅ - **APT** (Ubuntu/Debian) - Full feature support -- **YUM** (Rocky Linux/AlmaLinux/RHEL) - Full feature support +- **YUM** (Rocky Linux/AlmaLinux/RHEL) - Full feature support - **Snap** (Universal packages) - Full feature support - **Flatpak** (Universal packages) - Full feature support @@ -101,4 +114,4 @@ Current development focus areas (see [GitHub Issues](https://github.com/bluet/sy ### Planned 📋 - **Homebrew** (macOS) - Planned for cross-platform expansion - **Chocolatey/Scoop/winget** (Windows) - Planned for Windows support -- **Zypper** (openSUSE) - Lower priority \ No newline at end of file +- **Zypper** (openSUSE) - Lower priority diff --git a/Makefile b/Makefile index 4476ec6..01eecfe 100644 --- a/Makefile +++ b/Makefile @@ -88,29 +88,29 @@ install-tools: # Docker testing targets test-docker: @echo "Running tests in Docker containers..." - docker-compose -f testing/docker/docker-compose.test.yml up --abort-on-container-exit --remove-orphans + docker compose -f testing/docker/docker-compose.test.yml up --abort-on-container-failure --remove-orphans test-docker-ubuntu: @echo "Running Ubuntu APT tests..." - docker-compose -f testing/docker/docker-compose.test.yml up ubuntu-apt-test --abort-on-container-exit + docker compose -f testing/docker/docker-compose.test.yml up ubuntu-apt-test --abort-on-container-failure test-docker-rocky: @echo "Running Rocky Linux YUM tests..." - docker-compose -f testing/docker/docker-compose.test.yml up rockylinux-yum-test --abort-on-container-exit + docker compose -f testing/docker/docker-compose.test.yml up rockylinux-yum-test --abort-on-container-failure test-docker-alma: @echo "Running AlmaLinux YUM tests..." - docker-compose -f testing/docker/docker-compose.test.yml up almalinux-yum-test --abort-on-container-exit + docker compose -f testing/docker/docker-compose.test.yml up almalinux-yum-test --abort-on-container-failure # TODO: Enable when DNF support is implemented # test-docker-fedora: # @echo "Running Fedora DNF tests..." -# docker-compose -f testing/docker/docker-compose.test.yml up fedora-dnf-test --abort-on-container-exit +# docker compose -f testing/docker/docker-compose.test.yml up fedora-dnf-test --abort-on-container-failure # TODO: Enable when APK support is implemented # test-docker-alpine: # @echo "Running Alpine APK tests..." -# docker-compose -f testing/docker/docker-compose.test.yml up alpine-apk-test --abort-on-container-exit +# docker compose -f testing/docker/docker-compose.test.yml up alpine-apk-test --abort-on-container-failure test-docker-all: test-docker @@ -118,13 +118,13 @@ test-docker-all: test-docker test-fixtures: @echo "Generating test fixtures from multiple OS..." @mkdir -p testing/fixtures/{apt,yum,dnf,apk} - docker-compose -f testing/docker/docker-compose.test.yml up --abort-on-container-exit + docker compose -f testing/docker/docker-compose.test.yml up --abort-on-container-failure @echo "Test fixtures generated in testing/fixtures/" # Clean up Docker resources test-docker-clean: @echo "Cleaning up Docker test resources..." - docker-compose -f testing/docker/docker-compose.test.yml down --volumes --remove-orphans + docker compose -f testing/docker/docker-compose.test.yml down --volumes --remove-orphans docker system prune -f --filter "label=com.docker.compose.project=syspkg-test" # Unit tests only (no integration/system tests) diff --git a/manager/apt/apt.go b/manager/apt/apt.go index 18a7071..2b90e05 100644 --- a/manager/apt/apt.go +++ b/manager/apt/apt.go @@ -378,9 +378,6 @@ func (a *PackageManager) UpgradeAll(opts *manager.Options) ([]manager.PackageInf // Clean cleans the local package cache used by the apt package manager. func (a *PackageManager) Clean(opts *manager.Options) error { - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) - defer cancel() - if opts == nil { opts = &manager.Options{ DryRun: false, @@ -388,6 +385,16 @@ func (a *PackageManager) Clean(opts *manager.Options) error { Verbose: false, } } + + // Handle dry run mode + if opts.DryRun { + log.Println("Dry run mode: would execute 'apt autoclean'") + return nil + } + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + args := []string{"autoclean"} out, err := a.executeCommand(ctx, args, opts) if err != nil { diff --git a/manager/apt/apt_clean_dryrun_test.go b/manager/apt/apt_clean_dryrun_test.go new file mode 100644 index 0000000..7a41bac --- /dev/null +++ b/manager/apt/apt_clean_dryrun_test.go @@ -0,0 +1,65 @@ +package apt_test + +import ( + "testing" + + "github.com/bluet/syspkg/manager" + "github.com/bluet/syspkg/manager/apt" +) + +// TestCleanRespectsDryRun is the regression test for the security-relevant bug +// where Clean() executed `apt autoclean` even when opts.DryRun was true. +// Behavior contract: Clean(DryRun=true) MUST NOT execute any underlying command. +func TestCleanRespectsDryRun(t *testing.T) { + mockRunner := manager.NewMockCommandRunner() + pm := apt.NewPackageManagerWithCustomRunner(mockRunner) + + if err := pm.Clean(&manager.Options{DryRun: true}); err != nil { + t.Fatalf("Clean(DryRun=true) returned error: %v", err) + } + + if got := len(mockRunner.EnvCalls); got != 0 { + t.Errorf("Clean(DryRun=true) executed %d non-interactive command(s); expected 0. Calls: %v", + got, mockRunner.EnvCalls) + } + if got := len(mockRunner.InteractiveCalls); got != 0 { + t.Errorf("Clean(DryRun=true) executed %d interactive command(s); expected 0. Calls: %v", + got, mockRunner.InteractiveCalls) + } +} + +// TestCleanRunsWithoutDryRun guards against the Clean(DryRun) fix being +// implemented as a blanket no-op. Without DryRun, Clean MUST invoke +// `apt autoclean`. +func TestCleanRunsWithoutDryRun(t *testing.T) { + mockRunner := manager.NewMockCommandRunner() + mockRunner.AddCommand("apt", []string{"autoclean"}, []byte("Reading package lists...\n"), nil) + pm := apt.NewPackageManagerWithCustomRunner(mockRunner) + + if err := pm.Clean(&manager.Options{DryRun: false}); err != nil { + t.Fatalf("Clean(DryRun=false) returned error: %v", err) + } + + if _, ok := mockRunner.EnvCalls["apt autoclean"]; !ok { + t.Errorf("Clean(DryRun=false) didn't invoke 'apt autoclean'. Recorded calls: %v", + mockRunner.EnvCalls) + } +} + +// TestCleanRespectsDryRunWithNilOptsDefault verifies the nil-opts branch: +// when opts == nil, the code path defaults DryRun to false, so Clean +// SHOULD execute (proving the nil-opts default is preserved by the fix). +func TestCleanRunsWithNilOpts(t *testing.T) { + mockRunner := manager.NewMockCommandRunner() + mockRunner.AddCommand("apt", []string{"autoclean"}, []byte("Reading package lists...\n"), nil) + pm := apt.NewPackageManagerWithCustomRunner(mockRunner) + + if err := pm.Clean(nil); err != nil { + t.Fatalf("Clean(nil) returned error: %v", err) + } + + if _, ok := mockRunner.EnvCalls["apt autoclean"]; !ok { + t.Errorf("Clean(nil) didn't invoke 'apt autoclean'. Recorded calls: %v", + mockRunner.EnvCalls) + } +} diff --git a/testing/docker/docker-compose.test.yml b/testing/docker/docker-compose.test.yml index d1955bd..5c2ab02 100644 --- a/testing/docker/docker-compose.test.yml +++ b/testing/docker/docker-compose.test.yml @@ -18,13 +18,19 @@ services: volumes: - ../..:/workspace working_dir: /workspace + # NOTE: bash -ec ensures any failed required command (test, apt update) + # exits immediately. The `|| true` on fixture-generation lines is the + # explicit opt-out — those steps are allowed to fail without failing the + # build. Previously this used `&&` chaining which silently masked test + # failures because trailing `|| true` caught failures from earlier in the + # chain. command: > - bash -c " - echo 'Running Ubuntu APT tests...' && - go test -v -tags='unit integration apt' ./manager/apt ./osinfo && - echo 'Generating APT fixtures...' && - apt update && - apt search vim > testing/fixtures/apt/search-vim-ubuntu22.txt 2>/dev/null || true && + bash -ec " + echo 'Running Ubuntu APT tests...'; + go test -v -tags='unit integration apt' ./manager/apt ./osinfo; + echo 'Generating APT fixtures...'; + apt update; + apt search vim > testing/fixtures/apt/search-vim-ubuntu22.txt 2>/dev/null || true; apt show vim > testing/fixtures/apt/show-vim-ubuntu22.txt 2>/dev/null || true " @@ -42,13 +48,14 @@ services: volumes: - ../..:/workspace working_dir: /workspace + # See ubuntu-apt-test for rationale on bash -ec + ; separators. command: > - bash -c " - echo 'Running Rocky Linux YUM tests...' && - go test -v -tags='unit integration yum' ./manager/yum ./osinfo && - echo 'Generating YUM fixtures...' && - yum search vim > testing/fixtures/yum/search-vim-rocky8.txt 2>/dev/null || true && - yum info vim-enhanced > testing/fixtures/yum/info-vim-rocky8.txt 2>/dev/null || true && + bash -ec " + echo 'Running Rocky Linux YUM tests...'; + go test -v -tags='unit integration yum' ./manager/yum ./osinfo; + echo 'Generating YUM fixtures...'; + yum search vim > testing/fixtures/yum/search-vim-rocky8.txt 2>/dev/null || true; + yum info vim-enhanced > testing/fixtures/yum/info-vim-rocky8.txt 2>/dev/null || true; yum list --installed > testing/fixtures/yum/list-installed-rocky8.txt 2>/dev/null || true " @@ -66,12 +73,13 @@ services: volumes: - ../..:/workspace working_dir: /workspace + # See ubuntu-apt-test for rationale on bash -ec + ; separators. command: > - bash -c " - echo 'Running AlmaLinux YUM tests...' && - go test -v -tags='unit integration yum' ./manager/yum ./osinfo && - echo 'Generating YUM fixtures...' && - yum search vim > testing/fixtures/yum/search-vim-alma8.txt 2>/dev/null || true && + bash -ec " + echo 'Running AlmaLinux YUM tests...'; + go test -v -tags='unit integration yum' ./manager/yum ./osinfo; + echo 'Generating YUM fixtures...'; + yum search vim > testing/fixtures/yum/search-vim-alma8.txt 2>/dev/null || true; yum info vim-enhanced > testing/fixtures/yum/info-vim-alma8.txt 2>/dev/null || true " @@ -125,14 +133,34 @@ services: # Test runner that runs all tests in parallel test-all: image: ubuntu:24.04 + # Long-form depends_on with service_completed_successfully so test-all + # waits for the actual tests to complete before running. With short-form, + # test-all's `echo` could finish in milliseconds and abort the compose + # run (--abort-on-container-exit) before the real tests had a chance to + # report failure — exactly the same class of CI-honesty bug the bash -ec + # change in this same release fixes one layer up. depends_on: - - ubuntu-apt-test - - rockylinux-yum-test - - almalinux-yum-test - # - fedora-dnf-test # TODO: Enable when DNF support is implemented - # - alpine-apk-test # TODO: Enable when APK support is implemented + ubuntu-apt-test: + condition: service_completed_successfully + rockylinux-yum-test: + condition: service_completed_successfully + almalinux-yum-test: + condition: service_completed_successfully + # fedora-dnf-test: # TODO: Enable when DNF support is implemented + # condition: service_completed_successfully + # alpine-apk-test: # TODO: Enable when APK support is implemented + # condition: service_completed_successfully + # Defense-in-depth: this aggregator service only runs an echo, so it can + # safely use a read-only root, no-new-privileges, and a read-only bind + # mount. Required services (ubuntu/rocky/alma) need write access for + # fixture generation, so they don't get these constraints. + read_only: true + security_opt: + # Quoted to avoid YAML parser ambiguity around `key:value` parsing + # while preserving Docker's documented `no-new-privileges:true` form. + - "no-new-privileges:true" volumes: - - ../..:/workspace + - ../..:/workspace:ro working_dir: /workspace command: > bash -c "