diff --git a/.github/release-please-manifest.json b/.github/release-please-manifest.json index 727a5db..37fcefa 100644 --- a/.github/release-please-manifest.json +++ b/.github/release-please-manifest.json @@ -1,3 +1,3 @@ { - ".": "0.21.1" + ".": "1.0.0" } diff --git a/.github/workflows/release-please.yml b/.github/workflows/release-please.yml index 6b598b9..96203e4 100644 --- a/.github/workflows/release-please.yml +++ b/.github/workflows/release-please.yml @@ -115,7 +115,7 @@ jobs: uses: ./ with: image: check-image:scan - checks: size,root-user,ports,secrets + checks: size,user,ports,secrets max-size: '20' - name: Log in to GitHub Container Registry diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8e9ad94..f020ee2 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -23,12 +23,26 @@ repos: hooks: - id: actionlint - # Go-specific hooks + # Go formatting and modernization - repo: https://github.com/tekwizely/pre-commit-golang rev: v1.0.0-rc.1 hooks: - id: go-fmt args: [-w] + + - repo: local + hooks: + - id: go-fix + name: go-fix + entry: go fix ./... + language: system + pass_filenames: false + types: [go] + + # Go analysis, linting, and testing + - repo: https://github.com/tekwizely/pre-commit-golang + rev: v1.0.0-rc.1 + hooks: - id: go-mod-tidy - id: go-vet-mod - id: golangci-lint-mod diff --git a/CLAUDE.md b/CLAUDE.md index ecbe9df..6adf890 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -56,7 +56,7 @@ The `UpdateResult()` helper in `root.go` enforces this precedence. The iota orde - Controlled by the `--output`/`-o` global flag (values: `text` default, `json`) - Color output controlled by the `--color` global flag (values: `auto` default, `always`, `never`); only applies to `--output=text` - `internal/output/format.go`: Defines `Format` type, `ParseFormat()`, and `RenderJSON()` helper -- `internal/output/results.go`: Result structs (`CheckResult`, `AgeDetails`, `SizeDetails`, `PortsDetails`, `RegistryDetails`, `RootUserDetails`, `HealthcheckDetails`, `SecretsDetails`, `LabelsDetails`, `AllResult`, `Summary`, `VersionResult`) +- `internal/output/results.go`: Result structs (`CheckResult`, `AgeDetails`, `SizeDetails`, `PortsDetails`, `RegistryDetails`, `HealthcheckDetails`, `SecretsDetails`, `LabelsDetails`, `AllResult`, `Summary`, `VersionResult`) - `cmd/check-image/commands/render.go`: Text renderers for each check; `renderResult()` dispatches to JSON or text based on `OutputFmt` - `cmd/check-image/commands/styles.go`: Lip Gloss styles (`PassStyle`, `FailStyle`, `headerStyle`, `keyStyle`, `valueStyle`, `dimStyle`); `initRenderer(colorMode, out)` configures the renderer and updates all styles; `statusPrefix(passed)` returns colored ✓/✗; called from `PersistentPreRunE` after `--color` is parsed - In JSON mode, `main.go` suppresses the final "Validation succeeded/failed" text message (it's already in the JSON) @@ -160,10 +160,6 @@ Implemented with `authn.NewMultiKeychain(staticKC, authn.DefaultKeychain)`. - File format: `{"allowed-ports": [80, 443]}` - Parses ports from image config's `ExposedPorts` field (format: "8080/tcp") -**root-user**: Validates that image runs as non-root -- No flags -- Checks if `config.Config.User` is empty or "root" - **healthcheck**: Validates that the image has a healthcheck defined - No flags - Checks if `config.Config.Healthcheck` is not nil, has a non-empty test command, and test is not `["NONE"]` (explicitly disabled) @@ -208,7 +204,7 @@ Implemented with `authn.NewMultiKeychain(staticKC, authn.DefaultKeychain)`. **user**: Validates that the image user meets security requirements - Flags: `--user-policy` (optional, JSON or YAML file), `--min-uid` (optional), `--max-uid` (optional), `--blocked-users` (comma-separated, optional), `--require-numeric` (optional) -- Without flags/policy: basic non-root check (same behavior as `root-user`) +- Without flags/policy: basic non-root check (rejects empty user, "root", and UID 0) - With policy file or flags: enforces UID ranges, blocked usernames, numeric UID requirements - CLI flags override policy file values; `cmd.Flags().Changed()` distinguishes "not set" from "explicitly set to 0" - Validates raw `config.Config.User` string only (no `/etc/passwd` resolution available) @@ -217,14 +213,13 @@ Implemented with `authn.NewMultiKeychain(staticKC, authn.DefaultKeychain)`. - Collects all violations (no short-circuit); reports machine-readable `rule` and human-readable `message` - Returns `UserDetails` with `user`, `is-numeric`, `uid`, violations, and policy constraints - Implementation: `internal/user/` package (`policy.go`, `validator.go`), `cmd/check-image/commands/user.go` -- Coexists with `root-user` in `all` — no automatic exclusion; user manages with `--skip`/`--include` - Sample config files: `config/user-policy.yaml`, `config/user-policy.json` **all**: Runs all validation checks on a container image at once - Flags: `--config` (`-c`, config file), `--include` (comma-separated checks to run), `--skip` (comma-separated checks to skip), `--fail-fast` (stop on first failure), plus all individual check flags (`--max-age`, `--max-size`, `--max-layers`, `--allowed-ports`, `--allowed-platforms`, `--registry-policy`, `--labels-policy`, `--secrets-policy`, `--skip-env-vars`, `--skip-files`, `--allow-shell-form`, `--user-policy`, `--min-uid`, `--max-uid`, `--blocked-users`, `--require-numeric`) - `--include` and `--skip` are mutually exclusive - Precedence: CLI flags > config file values > defaults; `--include` and `--skip` always take precedence over config file check selection -- Without `--config`: runs all 11 checks with defaults (except skipped, or only included) +- Without `--config`: runs all 10 checks with defaults (except skipped, or only included) - With `--config`: only runs checks present in the config file (except skipped); `--include` overrides config check selection - Uses `applyConfigValues()` with `cmd.Flags().Changed()` to respect CLI overrides - Wrappers: `runPortsForAll()` calls `parseAllowedPorts()` before `runPorts()`; `runPlatformForAll()` calls `parseAllowedPlatforms()` before `runPlatform()` @@ -390,7 +385,7 @@ Runs on push to `main`, PRs to `main`, and weekly (Monday 06:00 UTC). Performs C Runs on every push to `main`. Three chained jobs: 1. **release-please**: Creates/updates a release PR with changelog and version bump. On merge, creates the git tag and GitHub release. Exports `releases_created` and `tag_name`. 2. **goreleaser**: Depends on release-please. Only runs when `releases_created == 'true'`. Builds binaries for linux/darwin (amd64/arm64) and windows (amd64 only), uploads to the GitHub release via `mode: append`. -3. **docker**: Depends on both release-please and goreleaser. Only runs when `releases_created == 'true'`. Lints Dockerfile with hadolint, builds single-arch image for Trivy security scanning (CRITICAL/HIGH), validates image with check-image (dogfooding: size, root-user, ports, secrets), then builds and pushes multi-arch image (linux/amd64, linux/arm64) to GHCR with semver tags via `docker/metadata-action`. +3. **docker**: Depends on both release-please and goreleaser. Only runs when `releases_created == 'true'`. Lints Dockerfile with hadolint, builds single-arch image for Trivy security scanning (CRITICAL/HIGH), validates image with check-image (dogfooding: size, user, ports, secrets), then builds and pushes multi-arch image (linux/amd64, linux/arm64) to GHCR with semver tags via `docker/metadata-action`. All release jobs must be in the same workflow because tags created by `GITHUB_TOKEN` do not trigger other workflows (GitHub limitation to prevent infinite loops). @@ -470,11 +465,11 @@ All release jobs must be in the same workflow because tags created by `GITHUB_TO #### Formatting and Tooling - Format code with `gofmt`. -- Run `go fix ./...` to apply any API migration rewrites. -- Run `go vet` and `golangci-lint` to ensure idiomatic Go. +- Run `go fix ./...` to apply any API migration rewrites (e.g., inlining `//go:fix inline` functions to use `new(v)` syntax). This is enforced by the `go-fix` pre-commit hook. +- Run `go vet` and `golangci-lint` to ensure idiomatic Go. The `modernize` linter (enabled by default in golangci-lint v2) detects code that `go fix` would rewrite, catching missed modernization in CI. - Keep `go.mod` tidy. - Run `pre-commit run --all-files` explicitly before committing and fix any reported issues before proceeding. -- Pre-commit hooks enforce these requirements automatically on `git commit` as well. +- Pre-commit hooks enforce these requirements automatically on `git commit` as well. The `go-fix` hook runs `go fix ./...` to apply code modernization rewrites before linting and testing. - See `.golangci.yml` for linter configuration (balanced settings). - Install hooks with: `pre-commit install && pre-commit install --hook-type commit-msg`. diff --git a/README.md b/README.md index 6886ec1..90d3d42 100644 --- a/README.md +++ b/README.md @@ -107,8 +107,8 @@ docker run --rm ghcr.io/jarfernandez/check-image age nginx:latest --max-age 30 # Check image size docker run --rm ghcr.io/jarfernandez/check-image size nginx:latest --max-size 100 -# Check for root user -docker run --rm ghcr.io/jarfernandez/check-image root-user nginx:latest +# Check user security requirements +docker run --rm ghcr.io/jarfernandez/check-image user nginx:latest # Run all checks with JSON output docker run --rm ghcr.io/jarfernandez/check-image all nginx:latest -o json @@ -179,7 +179,7 @@ The config file determines which checks to run and their parameters. See [All Ch - uses: jarfernandez/check-image@v0.21.1 # x-release-please-version with: image: nginx:latest - checks: age,size,root-user + checks: age,size,user max-age: '30' max-size: '200' ``` @@ -388,13 +388,6 @@ check-image ports --allowed-ports Options: - `--allowed-ports`: Comma-separated list of allowed ports or `@` with JSON/YAML array -#### `root-user` -Validates that the image runs as non-root user. - -```bash -check-image root-user -``` - #### `healthcheck` Validates that the image has a healthcheck defined. @@ -482,14 +475,12 @@ Options: - `--blocked-users`: Comma-separated list of blocked usernames (optional) - `--require-numeric`: Require user to be a numeric UID (optional) -Without any flags or policy file, performs a basic non-root check (same behavior as `root-user`). With flags or a policy file, enforces UID ranges, blocked usernames, and numeric UID requirements. +Without any flags or policy file, performs a basic non-root check (rejects empty user, "root", and UID 0). With flags or a policy file, enforces UID ranges, blocked usernames, and numeric UID requirements. Precedence: CLI flags override policy file values. When both are provided, the policy file is loaded first, then CLI flags are overlaid on top. **Limitation:** Without the image's `/etc/passwd`, username-to-UID resolution is not possible. The command validates the raw `User` field string only. UID range checks (`--min-uid`, `--max-uid`) only apply when the user is a numeric value. -The `user` command coexists with `root-user` — both can run in the `all` command simultaneously. Use `--skip` or `--include` to control which checks run. - #### `all` Runs all validation checks on a container image at once. @@ -499,8 +490,8 @@ check-image all [flags] Options: - `--config`, `-c`: Path to configuration file (JSON or YAML) -- `--include`: Comma-separated list of checks to run (age, size, ports, registry, root-user, healthcheck, secrets, labels, entrypoint, platform, user) -- `--skip`: Comma-separated list of checks to skip (age, size, ports, registry, root-user, healthcheck, secrets, labels, entrypoint, platform, user) +- `--include`: Comma-separated list of checks to run (age, size, ports, registry, healthcheck, secrets, labels, entrypoint, platform, user) +- `--skip`: Comma-separated list of checks to skip (age, size, ports, registry, healthcheck, secrets, labels, entrypoint, platform, user) - `--max-age`, `-a`: Maximum age in days (default: 90) - `--max-size`, `-m`: Maximum size in MB (default: 500) - `--max-layers`, `-y`: Maximum number of layers (default: 20) @@ -522,7 +513,7 @@ Options: Note: `--include` and `--skip` are mutually exclusive. Precedence rules: -1. Without `--config`: all 11 checks run with defaults, except those in `--skip` +1. Without `--config`: all 10 checks run with defaults, except those in `--skip` 2. With `--config`: only checks present in the config file run, except those in `--skip` 3. `--include` overrides config file check selection (runs only specified checks) 4. CLI flags override config file values @@ -599,7 +590,7 @@ echo "mytoken" | check-image age my-registry.example.com/private-image:latest \ --password-stdin # Or read from a file -check-image root-user my-registry.example.com/private-image:latest \ +check-image user my-registry.example.com/private-image:latest \ --username myuser \ --password-stdin < ~/.secrets/registry-token ``` @@ -945,7 +936,7 @@ go install ./cmd/check-image ### Pre-Commit Hooks -This project uses pre-commit hooks to enforce code quality and formatting standards before each commit. The hooks automatically run `gofmt`, `go vet`, `golangci-lint`, `go mod tidy`, execute tests with `go-test-mod`, and validate commit messages follow Conventional Commits format. +This project uses pre-commit hooks to enforce code quality and formatting standards before each commit. The hooks automatically run `gofmt`, `go fix`, `go vet`, `golangci-lint`, `go mod tidy`, execute tests with `go-test-mod`, and validate commit messages follow Conventional Commits format. #### Installation @@ -1004,6 +995,7 @@ The hooks run automatically on `git commit`. You can also: - Config validation: YAML and JSON syntax - Go formatting: `gofmt` - Go tidying: `go mod tidy` +- Go modernization: `go fix` (applies API migration rewrites, e.g., `//go:fix inline`) - Go analysis: `go vet` - Go linting: `golangci-lint` (see `.golangci.yml` for configuration) - Go tests: `go test` via `go-test-mod` @@ -1071,7 +1063,7 @@ go tool cover -html=coverage.out - **internal/secrets**: 97.4% coverage - **internal/fileutil**: 90.0% coverage - **internal/imageutil**: 88.7% coverage -- **cmd/check-image/commands**: 91.5% coverage +- **cmd/check-image/commands**: 91.6% coverage - **cmd/check-image**: 73.9% coverage All tests are deterministic, fast, and run without requiring Docker daemon, registry access, or network connectivity. Tests use in-memory images, temporary directories, and OCI layout structures for validation. @@ -1118,7 +1110,7 @@ Releases are fully automated using [release-please](https://github.com/googleapi - **docker job**: - Lints `Dockerfile` with hadolint - Builds a single-arch image (`linux/amd64`) for Trivy security scanning (CRITICAL/HIGH vulnerabilities) - - Validates the image with check-image itself (dogfooding: size, root-user, ports, secrets) + - Validates the image with check-image itself (dogfooding: size, user, ports, secrets) - Builds and pushes a multi-arch image (`linux/amd64`, `linux/arm64`) to GHCR with semver tags (`major.minor.patch`, `major.minor`, `major`, `latest`) ### Supported Platforms diff --git a/cmd/check-image/commands/all_config.go b/cmd/check-image/commands/all_config.go index 0439df5..31734b2 100644 --- a/cmd/check-image/commands/all_config.go +++ b/cmd/check-image/commands/all_config.go @@ -18,7 +18,6 @@ const ( checkSize = "size" checkPorts = "ports" checkRegistry = "registry" - checkRootUser = "root-user" checkSecrets = "secrets" checkHealthcheck = "healthcheck" checkLabels = "labels" @@ -29,7 +28,7 @@ const ( // validCheckNames lists all check names recognized by the all command. var validCheckNames = []string{ - checkAge, checkSize, checkPorts, checkRegistry, checkRootUser, + checkAge, checkSize, checkPorts, checkRegistry, checkSecrets, checkHealthcheck, checkLabels, checkEntrypoint, checkPlatform, checkUser, } @@ -44,7 +43,6 @@ type allChecksConfig struct { Size *sizeCheckConfig `json:"size,omitempty" yaml:"size,omitempty"` Ports *portsCheckConfig `json:"ports,omitempty" yaml:"ports,omitempty"` Registry *registryCheckConfig `json:"registry,omitempty" yaml:"registry,omitempty"` - RootUser *rootUserCheckConfig `json:"root-user,omitempty" yaml:"root-user,omitempty"` Secrets *secretsCheckConfig `json:"secrets,omitempty" yaml:"secrets,omitempty"` Healthcheck *healthcheckCheckConfig `json:"healthcheck,omitempty" yaml:"healthcheck,omitempty"` Labels *labelsCheckConfig `json:"labels,omitempty" yaml:"labels,omitempty"` @@ -70,8 +68,6 @@ type registryCheckConfig struct { RegistryPolicy any `json:"registry-policy,omitempty" yaml:"registry-policy,omitempty"` } -type rootUserCheckConfig struct{} - type healthcheckCheckConfig struct{} type entrypointCheckConfig struct { diff --git a/cmd/check-image/commands/all_config_test.go b/cmd/check-image/commands/all_config_test.go index 5b6c55c..45f7ed0 100644 --- a/cmd/check-image/commands/all_config_test.go +++ b/cmd/check-image/commands/all_config_test.go @@ -34,8 +34,8 @@ func TestParseCheckNameList(t *testing.T) { }, { name: "all checks", - input: "age,size,ports,registry,root-user,secrets", - expected: map[string]bool{"age": true, "size": true, "ports": true, "registry": true, "root-user": true, "secrets": true}, + input: "age,size,ports,registry,secrets", + expected: map[string]bool{"age": true, "size": true, "ports": true, "registry": true, "secrets": true}, }, { name: "with whitespace", @@ -88,7 +88,7 @@ func TestLoadAllConfig(t *testing.T) { size: max-size: 200 max-layers: 10 - root-user: {} + user: {} ` err := os.WriteFile(cfgFile, []byte(content), 0600) require.NoError(t, err) @@ -107,7 +107,7 @@ func TestLoadAllConfig(t *testing.T) { require.NotNil(t, cfg.Checks.Size.MaxLayers) assert.Equal(t, uint(10), *cfg.Checks.Size.MaxLayers) - require.NotNil(t, cfg.Checks.RootUser) + require.NotNil(t, cfg.Checks.User) assert.Nil(t, cfg.Checks.Ports) assert.Nil(t, cfg.Checks.Registry) @@ -136,21 +136,21 @@ func TestLoadAllConfig(t *testing.T) { require.NotNil(t, cfg.Checks.Secrets) assert.Nil(t, cfg.Checks.Size) assert.Nil(t, cfg.Checks.Registry) - assert.Nil(t, cfg.Checks.RootUser) + assert.Nil(t, cfg.Checks.User) }) - t.Run("root-user with empty object", func(t *testing.T) { + t.Run("user with empty object", func(t *testing.T) { tmpDir := t.TempDir() cfgFile := filepath.Join(tmpDir, "config.yaml") content := `checks: - root-user: {} + user: {} ` err := os.WriteFile(cfgFile, []byte(content), 0600) require.NoError(t, err) cfg, err := loadAllConfig(cfgFile) require.NoError(t, err) - require.NotNil(t, cfg.Checks.RootUser) + require.NotNil(t, cfg.Checks.User) }) t.Run("nonexistent file", func(t *testing.T) { @@ -898,7 +898,7 @@ func TestLoadAllConfig_Healthcheck(t *testing.T) { content := `checks: age: max-age: 30 - root-user: {} + user: {} healthcheck: {} ` err := os.WriteFile(cfgFile, []byte(content), 0600) @@ -907,7 +907,7 @@ func TestLoadAllConfig_Healthcheck(t *testing.T) { cfg, err := loadAllConfig(cfgFile) require.NoError(t, err) require.NotNil(t, cfg.Checks.Age) - require.NotNil(t, cfg.Checks.RootUser) + require.NotNil(t, cfg.Checks.User) require.NotNil(t, cfg.Checks.Healthcheck) assert.Nil(t, cfg.Checks.Size) assert.Nil(t, cfg.Checks.Ports) diff --git a/cmd/check-image/commands/all_orchestration.go b/cmd/check-image/commands/all_orchestration.go index 35dfb61..afdc400 100644 --- a/cmd/check-image/commands/all_orchestration.go +++ b/cmd/check-image/commands/all_orchestration.go @@ -21,7 +21,7 @@ var allCmd = &cobra.Command{ Short: "Run all validation checks on a container image", Long: `Run all validation checks on a container image at once. -By default, runs all checks (age, size, ports, registry, root-user, secrets, healthcheck, labels, entrypoint, platform, user). +By default, runs all checks (age, size, ports, registry, secrets, healthcheck, labels, entrypoint, platform, user). Use --config to specify which checks to run and their parameters. Use --include to run only specific checks. Use --skip to skip specific checks. @@ -42,12 +42,12 @@ Precedence rules: 5. --include and --skip always take precedence over the config file ` + imageArgFormatsDoc, - Example: ` check-image all nginx:latest --include age,size,root-user --max-age 30 --max-size 200 + Example: ` check-image all nginx:latest --include age,size,user --max-age 30 --max-size 200 check-image all nginx:latest --skip registry,secrets,labels,platform check-image all nginx:latest --allowed-platforms linux/amd64,linux/arm64 --skip registry,labels check-image all nginx:latest --config config/config.json check-image all nginx:latest -c config/config.yaml --max-age 20 --skip secrets - check-image all oci:/path/to/layout:1.0 --include age,size,root-user,ports,healthcheck + check-image all oci:/path/to/layout:1.0 --include age,size,user,ports,healthcheck check-image all oci-archive:/path/to/image.tar:latest --skip ports,registry,secrets,labels,platform check-image all nginx:latest --fail-fast --skip registry --config config/config.yaml --output json cat config/config.json | check-image all nginx:latest --config -`, @@ -65,8 +65,8 @@ func init() { rootCmd.AddCommand(allCmd) allCmd.Flags().StringVarP(&configFile, "config", "c", "", "Configuration file (JSON or YAML) (optional)") - allCmd.Flags().StringVar(&skipChecks, "skip", "", "Comma-separated list of checks to skip (age, size, ports, registry, root-user, secrets, healthcheck, labels, entrypoint, platform, user) (optional)") - allCmd.Flags().StringVar(&includeChecks, "include", "", "Comma-separated list of checks to run (age, size, ports, registry, root-user, secrets, healthcheck, labels, entrypoint, platform, user) (optional)") + allCmd.Flags().StringVar(&skipChecks, "skip", "", "Comma-separated list of checks to skip (age, size, ports, registry, secrets, healthcheck, labels, entrypoint, platform, user) (optional)") + allCmd.Flags().StringVar(&includeChecks, "include", "", "Comma-separated list of checks to run (age, size, ports, registry, secrets, healthcheck, labels, entrypoint, platform, user) (optional)") allCmd.Flags().UintVarP(&maxAge, "max-age", "a", defaultMaxAgeDays, "Maximum age in days (optional)") allCmd.Flags().UintVarP(&maxSize, "max-size", "m", defaultMaxSizeMB, "Maximum size in megabytes (optional)") allCmd.Flags().UintVarP(&maxLayers, "max-layers", "y", defaultMaxLayerCount, "Maximum number of layers (optional)") @@ -158,7 +158,6 @@ func buildCheckDefs(cfg *allConfig, p checkParams) []checkDef { {checkRegistry, noCfg || cfg.Checks.Registry != nil, func(ctx context.Context, img string) (*output.CheckResult, error) { return runRegistry(ctx, img, p.registryPolicy) }, renderRegistryText}, - {checkRootUser, noCfg || cfg.Checks.RootUser != nil, runRootUser, renderRootUserText}, {checkSecrets, noCfg || cfg.Checks.Secrets != nil, func(ctx context.Context, img string) (*output.CheckResult, error) { return runSecrets(ctx, img, p.secretsPolicy, p.skipEnvVars, p.skipFiles) }, renderSecretsText}, diff --git a/cmd/check-image/commands/all_orchestration_test.go b/cmd/check-image/commands/all_orchestration_test.go index 9f54212..49d68c4 100644 --- a/cmd/check-image/commands/all_orchestration_test.go +++ b/cmd/check-image/commands/all_orchestration_test.go @@ -83,21 +83,21 @@ func TestAllCommandFlags(t *testing.T) { } func TestDetermineChecks(t *testing.T) { - t.Run("no config no skip runs all 11 checks", func(t *testing.T) { + t.Run("no config no skip runs all 10 checks", func(t *testing.T) { checks := determineChecks(nil, nil, nil, currentCheckParams()) - assert.Len(t, checks, 11) + assert.Len(t, checks, 10) names := make([]string, len(checks)) for i, c := range checks { names[i] = c.name } - assert.Equal(t, []string{"age", "size", "ports", "registry", "root-user", "secrets", "healthcheck", "labels", "entrypoint", "platform", "user"}, names) + assert.Equal(t, []string{"age", "size", "ports", "registry", "secrets", "healthcheck", "labels", "entrypoint", "platform", "user"}, names) }) t.Run("skip excludes checks", func(t *testing.T) { skipMap := map[string]bool{"registry": true, "secrets": true} checks := determineChecks(nil, skipMap, nil, currentCheckParams()) - assert.Len(t, checks, 9) + assert.Len(t, checks, 8) for _, c := range checks { assert.NotEqual(t, "registry", c.name) @@ -108,35 +108,35 @@ func TestDetermineChecks(t *testing.T) { t.Run("config selects subset", func(t *testing.T) { cfg := &allConfig{ Checks: allChecksConfig{ - Age: &ageCheckConfig{}, - RootUser: &rootUserCheckConfig{}, + Age: &ageCheckConfig{}, + User: &userCheckConfig{}, }, } checks := determineChecks(cfg, nil, nil, currentCheckParams()) assert.Len(t, checks, 2) assert.Equal(t, "age", checks[0].name) - assert.Equal(t, "root-user", checks[1].name) + assert.Equal(t, "user", checks[1].name) }) t.Run("config plus skip", func(t *testing.T) { cfg := &allConfig{ Checks: allChecksConfig{ - Age: &ageCheckConfig{}, - Size: &sizeCheckConfig{}, - RootUser: &rootUserCheckConfig{}, + Age: &ageCheckConfig{}, + Size: &sizeCheckConfig{}, + User: &userCheckConfig{}, }, } skipMap := map[string]bool{"size": true} checks := determineChecks(cfg, skipMap, nil, currentCheckParams()) assert.Len(t, checks, 2) assert.Equal(t, "age", checks[0].name) - assert.Equal(t, "root-user", checks[1].name) + assert.Equal(t, "user", checks[1].name) }) t.Run("skip all gives 0 checks", func(t *testing.T) { skipMap := map[string]bool{ "age": true, "size": true, "ports": true, - "registry": true, "root-user": true, "secrets": true, + "registry": true, "secrets": true, "healthcheck": true, "labels": true, "entrypoint": true, "platform": true, "user": true, } @@ -160,7 +160,6 @@ func TestDetermineChecks(t *testing.T) { Size: &sizeCheckConfig{}, Ports: &portsCheckConfig{}, Registry: ®istryCheckConfig{}, - RootUser: &rootUserCheckConfig{}, Secrets: &secretsCheckConfig{}, Healthcheck: &healthcheckCheckConfig{}, Labels: &labelsCheckConfig{}, @@ -261,7 +260,7 @@ func TestRunAll_AllChecksPass(t *testing.T) { assert.Equal(t, ValidationSucceeded, Result) assert.Contains(t, output, "── age") assert.Contains(t, output, "── size") - assert.Contains(t, output, "── root-user") + assert.Contains(t, output, "── user") assert.Contains(t, output, "── secrets") assert.NotContains(t, output, "── registry") } @@ -270,7 +269,7 @@ func TestRunAll_OneCheckFails_OthersContinue(t *testing.T) { resetAllGlobals(t) skipChecks = "registry,healthcheck,labels,platform" // skip checks that require policy files or missing healthcheck - // Image runs as root -> root-user check fails + // Image runs as root -> user check fails imageRef := createTestImage(t, testImageOptions{ user: "root", created: time.Now().Add(-10 * 24 * time.Hour), @@ -286,15 +285,15 @@ func TestRunAll_OneCheckFails_OthersContinue(t *testing.T) { // Verify other checks still ran assert.Contains(t, output, "── age") assert.Contains(t, output, "── size") - assert.Contains(t, output, "── root-user") + assert.Contains(t, output, "── user") assert.Contains(t, output, "── secrets") } func TestRunAll_SkipFailingCheck(t *testing.T) { resetAllGlobals(t) - skipChecks = "root-user,registry,healthcheck,labels,entrypoint,platform,user" // skip root-user (would fail) and checks that require policy files or missing healthcheck/entrypoint + skipChecks = "user,registry,healthcheck,labels,entrypoint,platform" // skip user (would fail) and checks that require policy files or missing healthcheck/entrypoint - // Image runs as root but we skip root-user check + // Image runs as root but we skip user check imageRef := createTestImage(t, testImageOptions{ user: "root", created: time.Now().Add(-10 * 24 * time.Hour), @@ -307,7 +306,7 @@ func TestRunAll_SkipFailingCheck(t *testing.T) { }) assert.Equal(t, ValidationSucceeded, Result) - assert.NotContains(t, output, "── root-user") + assert.NotContains(t, output, "── user") assert.NotContains(t, output, "── registry") } @@ -319,7 +318,7 @@ func TestRunAll_ConfigSelectsSubset(t *testing.T) { content := `checks: age: max-age: 90 - root-user: {} + user: {} ` err := os.WriteFile(cfgFile, []byte(content), 0600) require.NoError(t, err) @@ -339,7 +338,7 @@ func TestRunAll_ConfigSelectsSubset(t *testing.T) { assert.Equal(t, ValidationSucceeded, Result) assert.Contains(t, output, "── age") - assert.Contains(t, output, "── root-user") + assert.Contains(t, output, "── user") assert.NotContains(t, output, "── size") assert.NotContains(t, output, "── ports") assert.NotContains(t, output, "── registry") @@ -357,7 +356,7 @@ func TestRunAll_ConfigPlusSkip(t *testing.T) { max-age: 90 size: max-size: 500 - root-user: {} + user: {} ` err := os.WriteFile(cfgFile, []byte(content), 0600) require.NoError(t, err) @@ -378,7 +377,7 @@ func TestRunAll_ConfigPlusSkip(t *testing.T) { assert.Equal(t, ValidationSucceeded, Result) assert.Contains(t, output, "── age") - assert.Contains(t, output, "── root-user") + assert.Contains(t, output, "── user") assert.NotContains(t, output, "── size") assert.Contains(t, output, "Running 2 checks") } @@ -479,7 +478,7 @@ func TestRunAll_PortsWithoutAllowedPorts(t *testing.T) { func TestRunAll_SkipAll(t *testing.T) { resetAllGlobals(t) - skipChecks = "age,size,ports,registry,root-user,secrets,healthcheck,labels,entrypoint,platform,user" + skipChecks = "age,size,ports,registry,secrets,healthcheck,labels,entrypoint,platform,user" imageRef := createTestImage(t, testImageOptions{ user: "1000", @@ -531,11 +530,11 @@ func TestAllCommandFailFastFlag(t *testing.T) { func TestRunAll_FailFast_StopsOnValidationFailure(t *testing.T) { resetAllGlobals(t) - skipChecks = "registry,healthcheck,labels,platform" // skip checks that require policy files or missing healthcheck + skipChecks = "registry,healthcheck,labels,entrypoint,platform" // skip checks that require policy files or extra config failFast = true - // Image runs as root -> root-user check fails - // age and size run before root-user, secrets runs after + // Image runs as root -> user check fails + // age, size, ports, secrets run before user imageRef := createTestImage(t, testImageOptions{ user: "root", created: time.Now().Add(-10 * 24 * time.Hour), @@ -548,15 +547,13 @@ func TestRunAll_FailFast_StopsOnValidationFailure(t *testing.T) { }) assert.Equal(t, ValidationFailed, Result) - // root-user should have run and failed - assert.Contains(t, output, "── root-user") - // secrets comes after root-user, should NOT have run - assert.NotContains(t, output, "── secrets") + // user should have run and failed + assert.Contains(t, output, "── user") } func TestRunAll_FailFast_StopsOnExecutionError(t *testing.T) { resetAllGlobals(t) - skipChecks = "registry,healthcheck,labels,platform" // skip checks that require policy files or missing healthcheck + skipChecks = "registry,healthcheck,labels,entrypoint,platform" // skip checks that require policy files or extra config failFast = true // Provide invalid allowed-ports to cause an execution error in ports check @@ -576,9 +573,9 @@ func TestRunAll_FailFast_StopsOnExecutionError(t *testing.T) { assert.Equal(t, ExecutionError, Result) // ports should have run (and errored) assert.Contains(t, output, "── ports") - // root-user and secrets come after ports, should NOT have run - assert.NotContains(t, output, "── root-user") + // secrets and user come after ports, should NOT have run assert.NotContains(t, output, "── secrets") + assert.NotContains(t, output, "── user") } func TestRunAll_FailFastDisabled_RunsAllChecks(t *testing.T) { @@ -586,7 +583,7 @@ func TestRunAll_FailFastDisabled_RunsAllChecks(t *testing.T) { skipChecks = "registry,healthcheck,labels,platform" // skip checks that require policy files or missing healthcheck failFast = false - // Image runs as root -> root-user check fails + // Image runs as root -> user check fails imageRef := createTestImage(t, testImageOptions{ user: "root", created: time.Now().Add(-10 * 24 * time.Hour), @@ -599,10 +596,10 @@ func TestRunAll_FailFastDisabled_RunsAllChecks(t *testing.T) { }) assert.Equal(t, ValidationFailed, Result) - // All checks should have run despite root-user failure + // All checks should have run despite user failure assert.Contains(t, output, "── age") assert.Contains(t, output, "── size") - assert.Contains(t, output, "── root-user") + assert.Contains(t, output, "── user") assert.Contains(t, output, "── secrets") } @@ -670,15 +667,15 @@ func TestDetermineChecks_HealthcheckInConfig(t *testing.T) { cfg := &allConfig{ Checks: allChecksConfig{ Age: &ageCheckConfig{}, - RootUser: &rootUserCheckConfig{}, + User: &userCheckConfig{}, Healthcheck: &healthcheckCheckConfig{}, }, } checks := determineChecks(cfg, nil, nil, currentCheckParams()) assert.Len(t, checks, 3) assert.Equal(t, "age", checks[0].name) - assert.Equal(t, "root-user", checks[1].name) - assert.Equal(t, "healthcheck", checks[2].name) + assert.Equal(t, "healthcheck", checks[1].name) + assert.Equal(t, "user", checks[2].name) } func TestDetermineChecks_IncludeMap(t *testing.T) { @@ -693,11 +690,11 @@ func TestDetermineChecks_IncludeMap(t *testing.T) { t.Run("includeMap overrides config selection", func(t *testing.T) { cfg := &allConfig{ Checks: allChecksConfig{ - Age: &ageCheckConfig{}, - RootUser: &rootUserCheckConfig{}, + Age: &ageCheckConfig{}, + User: &userCheckConfig{}, }, } - // Config enables age and root-user, but --include asks only for size + // Config enables age and user, but --include asks only for size includeMap := map[string]bool{"size": true} checks := determineChecks(cfg, nil, includeMap, currentCheckParams()) assert.Len(t, checks, 1) @@ -714,11 +711,11 @@ func TestDetermineChecks_IncludeMap(t *testing.T) { t.Run("includeMap all checks", func(t *testing.T) { includeMap := map[string]bool{ "age": true, "size": true, "ports": true, "registry": true, - "root-user": true, "secrets": true, "healthcheck": true, "labels": true, "entrypoint": true, + "secrets": true, "healthcheck": true, "labels": true, "entrypoint": true, "platform": true, "user": true, } checks := determineChecks(nil, nil, includeMap, currentCheckParams()) - assert.Len(t, checks, 11) + assert.Len(t, checks, 10) }) } @@ -732,12 +729,11 @@ func TestSkippedCheckNames(t *testing.T) { t.Run("with include map", func(t *testing.T) { includeMap := map[string]bool{"age": true, "size": true} names := skippedCheckNames(nil, includeMap) - assert.Len(t, names, 9) + assert.Len(t, names, 8) assert.NotContains(t, names, "age") assert.NotContains(t, names, "size") assert.Contains(t, names, "ports") assert.Contains(t, names, "registry") - assert.Contains(t, names, "root-user") assert.Contains(t, names, "secrets") assert.Contains(t, names, "healthcheck") assert.Contains(t, names, "labels") @@ -754,7 +750,7 @@ func TestSkippedCheckNames(t *testing.T) { t.Run("include all checks returns nil", func(t *testing.T) { includeMap := map[string]bool{ "age": true, "size": true, "ports": true, "registry": true, - "root-user": true, "secrets": true, "healthcheck": true, "labels": true, "entrypoint": true, + "secrets": true, "healthcheck": true, "labels": true, "entrypoint": true, "platform": true, "user": true, } names := skippedCheckNames(nil, includeMap) @@ -784,7 +780,7 @@ func TestRunAll_IncludeAndSkipMutuallyExclusive(t *testing.T) { func TestRunAll_IncludeSelectsSubset(t *testing.T) { resetAllGlobals(t) - includeChecks = "age,root-user" + includeChecks = "age,user" imageRef := createTestImage(t, testImageOptions{ user: "1000", @@ -799,7 +795,7 @@ func TestRunAll_IncludeSelectsSubset(t *testing.T) { assert.Equal(t, ValidationSucceeded, Result) assert.Contains(t, out, "── age") - assert.Contains(t, out, "── root-user") + assert.Contains(t, out, "── user") assert.NotContains(t, out, "── size") assert.NotContains(t, out, "── ports") assert.NotContains(t, out, "── registry") @@ -817,13 +813,13 @@ func TestRunAll_IncludeOverridesConfig(t *testing.T) { content := `checks: age: max-age: 90 - root-user: {} + user: {} ` err := os.WriteFile(cfgFile, []byte(content), 0600) require.NoError(t, err) configFile = cfgFile - // Config enables age and root-user, but --include overrides to only size + // Config enables age and user, but --include overrides to only size includeChecks = "size" imageRef := createTestImage(t, testImageOptions{ @@ -840,7 +836,7 @@ func TestRunAll_IncludeOverridesConfig(t *testing.T) { assert.Equal(t, ValidationSucceeded, Result) assert.Contains(t, out, "── size") assert.NotContains(t, out, "── age") - assert.NotContains(t, out, "── root-user") + assert.NotContains(t, out, "── user") assert.Contains(t, out, "Running 1 checks") } @@ -1234,7 +1230,7 @@ func TestRenderAllJSON_WithFailures(t *testing.T) { results := []output.CheckResult{ {Check: checkAge, Image: "nginx:latest", Passed: true, Message: "Image is recent"}, - {Check: checkRootUser, Image: "nginx:latest", Passed: false, Message: "Image runs as root"}, + {Check: checkUser, Image: "nginx:latest", Passed: false, Message: "Image runs as root"}, {Check: checkSize, Image: "nginx:latest", Passed: false, Message: "check failed with error: some error", Error: "some error"}, } @@ -1377,49 +1373,6 @@ func TestRunAll_UserFailsMinUID(t *testing.T) { assert.Contains(t, out, "below minimum") } -func TestRunAll_UserAndRootUserBothPass(t *testing.T) { - resetAllGlobals(t) - includeChecks = "root-user,user" - - // Non-root user with high UID -> both root-user and user pass - imageRef := createTestImage(t, testImageOptions{ - user: "1000", - }) - - out := captureStdout(t, func() { - err := runAll(allCmd, imageRef) - require.NoError(t, err) - }) - - assert.Equal(t, ValidationSucceeded, Result) - assert.Contains(t, out, "── root-user") - assert.Contains(t, out, "── user") - assert.Contains(t, out, "Running 2 checks") -} - -func TestRunAll_UserFailsButRootUserPasses(t *testing.T) { - resetAllGlobals(t) - includeChecks = "root-user,user" - userMinUID = 1000 - - // UID 500: non-root (root-user passes), but below min-uid (user fails) - imageRef := createTestImage(t, testImageOptions{ - user: "500", - }) - - // Mark min-uid as changed - require.NoError(t, allCmd.Flags().Set("min-uid", "1000")) - - out := captureStdout(t, func() { - err := runAll(allCmd, imageRef) - require.NoError(t, err) - }) - - assert.Equal(t, ValidationFailed, Result) - assert.Contains(t, out, "── root-user") - assert.Contains(t, out, "── user") -} - func TestRunAll_UserSkipped(t *testing.T) { resetAllGlobals(t) skipChecks = "registry,healthcheck,labels,platform,user" diff --git a/cmd/check-image/commands/render.go b/cmd/check-image/commands/render.go index 8624c80..f51e365 100644 --- a/cmd/check-image/commands/render.go +++ b/cmd/check-image/commands/render.go @@ -28,7 +28,6 @@ var textRenderers = map[string]func(*output.CheckResult){ checkSize: renderSizeText, checkPorts: renderPortsText, checkRegistry: renderRegistryText, - checkRootUser: renderRootUserText, checkSecrets: renderSecretsText, checkHealthcheck: renderHealthcheckText, checkLabels: renderLabelsText, @@ -128,11 +127,6 @@ func renderRegistryText(r *output.CheckResult) { fmt.Println(statusPrefix(r.Passed) + r.Message) } -func renderRootUserText(r *output.CheckResult) { - fmt.Println(headerStyle.Render(fmt.Sprintf("Checking if image %s is configured to run as a non-root user", r.Image))) - fmt.Println(statusPrefix(r.Passed) + r.Message) -} - func renderSecretsText(r *output.CheckResult) { d := mustDetails[output.SecretsDetails](r) fmt.Println(headerStyle.Render(fmt.Sprintf("Checking secrets in image %s", r.Image))) diff --git a/cmd/check-image/commands/render_test.go b/cmd/check-image/commands/render_test.go index 65d5bf0..5fd6ebd 100644 --- a/cmd/check-image/commands/render_test.go +++ b/cmd/check-image/commands/render_test.go @@ -77,16 +77,6 @@ func TestRenderResult_TextMode(t *testing.T) { }, expectedParts: []string{"Checking registry", "docker.io/nginx:latest", "Image registry: docker.io", "Registry is trusted"}, }, - { - name: "Root user check", - result: &output.CheckResult{ - Check: checkRootUser, - Image: "nginx:latest", - Passed: true, - Message: "Image runs as non-root user", - }, - expectedParts: []string{"Checking if image nginx:latest", "non-root user", "Image runs as non-root user"}, - }, { name: "Secrets check", result: &output.CheckResult{ @@ -493,38 +483,6 @@ func TestRenderRegistryText_Skipped(t *testing.T) { assert.NotContains(t, captured, "Image registry:") } -func TestRenderRootUserText_NonRoot(t *testing.T) { - result := &output.CheckResult{ - Check: checkRootUser, - Image: "nginx:latest", - Passed: true, - Message: "Image runs as non-root user", - } - - captured := captureStdout(t, func() { - renderRootUserText(result) - }) - - assert.Contains(t, captured, "Checking if image nginx:latest is configured to run as a non-root user") - assert.Contains(t, captured, "Image runs as non-root user") -} - -func TestRenderRootUserText_Root(t *testing.T) { - result := &output.CheckResult{ - Check: checkRootUser, - Image: "old-app:v1", - Passed: false, - Message: "Image runs as root user", - } - - captured := captureStdout(t, func() { - renderRootUserText(result) - }) - - assert.Contains(t, captured, "Checking if image old-app:v1 is configured to run as a non-root user") - assert.Contains(t, captured, "Image runs as root user") -} - func TestRenderSecretsText_NoSecrets(t *testing.T) { result := &output.CheckResult{ Check: checkSecrets, diff --git a/cmd/check-image/commands/root_user.go b/cmd/check-image/commands/root_user.go deleted file mode 100644 index 592ccda..0000000 --- a/cmd/check-image/commands/root_user.go +++ /dev/null @@ -1,70 +0,0 @@ -package commands - -import ( - "context" - "strings" - - "github.com/jarfernandez/check-image/internal/imageutil" - "github.com/jarfernandez/check-image/internal/output" - "github.com/spf13/cobra" -) - -var rootUserCmd = &cobra.Command{ - Use: "root-user image", - Short: "Validate that the image is configured to run the container as a non-root user", - Long: `Validate that the image is configured to run the container as a non-root user. - -` + imageArgFormatsDoc, - Example: ` check-image root-user nginx:latest - check-image root-user oci:/path/to/layout:1.0 - check-image root-user oci-archive:/path/to/image.tar:latest - check-image root-user docker-archive:/path/to/image.tar:tag`, - Args: cobra.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - ctx := cmd.Context() - return runCheckCmd(checkRootUser, runRootUser, ctx, args[0], OutputFmt) - }, -} - -func init() { - rootCmd.AddCommand(rootUserCmd) -} - -func runRootUser(ctx context.Context, imageName string) (*output.CheckResult, error) { - _, config, cleanup, err := imageutil.GetImageAndConfig(ctx, imageName) - if err != nil { - return nil, err - } - defer cleanup() - - isNonRoot := !isRootUser(config.Config.User) - - var msg string - if isNonRoot { - msg = "Image is configured to run as a non-root user" - } else { - msg = "Image is not configured to run as a non-root user" - } - - return &output.CheckResult{ - Check: checkRootUser, - Image: imageName, - Passed: isNonRoot, - Message: msg, - Details: output.RootUserDetails{ - User: config.Config.User, - }, - }, nil -} - -// isRootUser reports whether the user value represents the root account. -// It checks for the name "root", empty string (Docker default is root), and UID 0 -// in both plain "0" and "0:group" formats, since UID 0 is root regardless of name. -func isRootUser(user string) bool { - if user == "" || user == "root" { - return true - } - // Check for UID 0 (e.g., "0", "0:0", "0:somegroup") - userFields := strings.SplitN(user, ":", 2) - return userFields[0] == "0" -} diff --git a/cmd/check-image/commands/root_user_test.go b/cmd/check-image/commands/root_user_test.go deleted file mode 100644 index c89eb80..0000000 --- a/cmd/check-image/commands/root_user_test.go +++ /dev/null @@ -1,117 +0,0 @@ -package commands - -import ( - "context" - "testing" - "time" - - "github.com/jarfernandez/check-image/internal/output" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestRootUserCommand(t *testing.T) { - // Test that root-user command exists and has correct properties - assert.NotNil(t, rootUserCmd) - assert.Equal(t, "root-user image", rootUserCmd.Use) - assert.Contains(t, rootUserCmd.Short, "non-root user") - - // Test that it requires exactly 1 argument - assert.NotNil(t, rootUserCmd.Args) - err := rootUserCmd.Args(rootUserCmd, []string{}) - assert.Error(t, err) - - err = rootUserCmd.Args(rootUserCmd, []string{"image"}) - assert.NoError(t, err) - - err = rootUserCmd.Args(rootUserCmd, []string{"image1", "image2"}) - assert.Error(t, err) -} - -func TestRunRootUser(t *testing.T) { - tests := []struct { - name string - user string - expectedPass bool - expectedMsg string - }{ - { - name: "Non-root user (UID)", - user: "1000", - expectedPass: true, - expectedMsg: "Image is configured to run as a non-root user", - }, - { - name: "Non-root user (username)", - user: "appuser", - expectedPass: true, - expectedMsg: "Image is configured to run as a non-root user", - }, - { - name: "Non-root user (UID:GID)", - user: "1000:1000", - expectedPass: true, - expectedMsg: "Image is configured to run as a non-root user", - }, - { - name: "Root user", - user: "root", - expectedPass: false, - expectedMsg: "Image is not configured to run as a non-root user", - }, - { - name: "Root user by UID 0", - user: "0", - expectedPass: false, - expectedMsg: "Image is not configured to run as a non-root user", - }, - { - name: "Root user by UID 0 with GID", - user: "0:0", - expectedPass: false, - expectedMsg: "Image is not configured to run as a non-root user", - }, - { - name: "Root user by UID 0 with named group", - user: "0:somegroup", - expectedPass: false, - expectedMsg: "Image is not configured to run as a non-root user", - }, - { - name: "Empty user (defaults to root)", - user: "", - expectedPass: false, - expectedMsg: "Image is not configured to run as a non-root user", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Create test image with specific user - imageRef := createTestImage(t, testImageOptions{ - user: tt.user, - created: time.Now(), - }) - - // Run command - result, err := runRootUser(context.Background(), imageRef) - require.NoError(t, err) - - // Assert on struct - assert.Equal(t, "root-user", result.Check) - assert.Equal(t, imageRef, result.Image) - assert.Equal(t, tt.expectedPass, result.Passed) - assert.Equal(t, tt.expectedMsg, result.Message) - - details, ok := result.Details.(output.RootUserDetails) - require.True(t, ok) - assert.Equal(t, tt.user, details.User) - }) - } -} - -func TestRunRootUser_InvalidImage(t *testing.T) { - // Test with invalid image reference - _, err := runRootUser(context.Background(), "nonexistent:image") - require.Error(t, err) -} diff --git a/cmd/check-image/commands/styles.go b/cmd/check-image/commands/styles.go index 8cc0d5d..c8d99ce 100644 --- a/cmd/check-image/commands/styles.go +++ b/cmd/check-image/commands/styles.go @@ -86,10 +86,7 @@ func sectionHeader(name string) string { const leftPrefix = "── " const rightPrefix = " " - rightLen := width - len([]rune(leftPrefix)) - len([]rune(name)) - len([]rune(rightPrefix)) - if rightLen < 2 { - rightLen = 2 - } + rightLen := max(width-len([]rune(leftPrefix))-len([]rune(name))-len([]rune(rightPrefix)), 2) line := leftPrefix + name + rightPrefix + strings.Repeat("─", rightLen) return sectionStyle.Render(line) diff --git a/cmd/check-image/commands/user.go b/cmd/check-image/commands/user.go index 9257854..dc17c55 100644 --- a/cmd/check-image/commands/user.go +++ b/cmd/check-image/commands/user.go @@ -23,7 +23,7 @@ var userCmd = &cobra.Command{ Short: "Validate that the image user meets security requirements", Long: `Validate that the image user meets security requirements. -Without any flags or policy file, performs a basic non-root check (same as root-user). +Without any flags or policy file, performs a basic non-root check. With flags or a policy file, enforces UID ranges, blocked usernames, and numeric UID requirements. ` + imageArgFormatsDoc, diff --git a/cmd/check-image/commands/user_test.go b/cmd/check-image/commands/user_test.go index 9a8d2c1..82cd6fc 100644 --- a/cmd/check-image/commands/user_test.go +++ b/cmd/check-image/commands/user_test.go @@ -96,6 +96,12 @@ func TestRunUser_BasicNonRoot(t *testing.T) { expectedPass: false, expectedMsg: "Image user does not meet requirements", }, + { + name: "root UID 0 with named group", + user: "0:somegroup", + expectedPass: false, + expectedMsg: "Image user does not meet requirements", + }, { name: "empty user defaults to root", user: "", @@ -137,20 +143,20 @@ func TestRunUser_WithPolicy(t *testing.T) { { name: "UID in range passes", user: "1000", - policy: &user.Policy{MinUID: ptrUintCmd(1000), MaxUID: ptrUintCmd(65534)}, + policy: &user.Policy{MinUID: new(uint(1000)), MaxUID: new(uint(65534))}, expectedPass: true, }, { name: "UID below minimum fails", user: "500", - policy: &user.Policy{MinUID: ptrUintCmd(1000)}, + policy: &user.Policy{MinUID: new(uint(1000))}, expectedPass: false, violationRules: []string{"min-uid"}, }, { name: "UID above maximum fails", user: "70000", - policy: &user.Policy{MaxUID: ptrUintCmd(65534)}, + policy: &user.Policy{MaxUID: new(uint(65534))}, expectedPass: false, violationRules: []string{"max-uid"}, }, @@ -164,14 +170,14 @@ func TestRunUser_WithPolicy(t *testing.T) { { name: "require numeric with username fails", user: "appuser", - policy: &user.Policy{RequireNumeric: ptrBoolCmd(true)}, + policy: &user.Policy{RequireNumeric: new(true)}, expectedPass: false, violationRules: []string{"require-numeric"}, }, { name: "require numeric with UID passes", user: "1000", - policy: &user.Policy{RequireNumeric: ptrBoolCmd(true)}, + policy: &user.Policy{RequireNumeric: new(true)}, expectedPass: true, }, } @@ -210,10 +216,10 @@ func TestRunUser_DetailsIncludePolicyConstraints(t *testing.T) { }) policy := &user.Policy{ - MinUID: ptrUintCmd(1000), - MaxUID: ptrUintCmd(65534), + MinUID: new(uint(1000)), + MaxUID: new(uint(65534)), BlockedUsers: []string{"daemon"}, - RequireNumeric: ptrBoolCmd(true), + RequireNumeric: new(true), } result, err := runUser(context.Background(), imageRef, policy) @@ -503,7 +509,7 @@ func TestRunUser_WithPolicyFile_Stdin(t *testing.T) { func TestRunUser_CLIOverridesPolicy(t *testing.T) { // Policy says min-uid: 2000, but CLI override lowers to 500 - basePolicy := &user.Policy{MinUID: ptrUintCmd(2000)} + basePolicy := &user.Policy{MinUID: new(uint(2000))} // Override min-uid to 500 overridden := uint(500) @@ -517,7 +523,7 @@ func TestRunUser_CLIOverridesPolicy(t *testing.T) { func TestRunUser_JSONOutput(t *testing.T) { imageRef := createTestImage(t, testImageOptions{user: "500", created: time.Now()}) - policy := &user.Policy{MinUID: ptrUintCmd(1000), MaxUID: ptrUintCmd(65534)} + policy := &user.Policy{MinUID: new(uint(1000)), MaxUID: new(uint(65534))} result, err := runUser(context.Background(), imageRef, policy) require.NoError(t, err) @@ -574,7 +580,7 @@ func TestRunUser_JSONOutput_Passing(t *testing.T) { func TestRunUser_TextOutput_WithViolations(t *testing.T) { imageRef := createTestImage(t, testImageOptions{user: "500", created: time.Now()}) - policy := &user.Policy{MinUID: ptrUintCmd(1000)} + policy := &user.Policy{MinUID: new(uint(1000))} result, err := runUser(context.Background(), imageRef, policy) require.NoError(t, err) @@ -842,11 +848,3 @@ func TestUserCommand_RunE(t *testing.T) { assert.Contains(t, out, "numeric") }) } - -func ptrUintCmd(v uint) *uint { - return &v -} - -func ptrBoolCmd(v bool) *bool { - return &v -} diff --git a/config/config-inline.json b/config/config-inline.json index 3484c7e..a7e76fe 100644 --- a/config/config-inline.json +++ b/config/config-inline.json @@ -15,7 +15,6 @@ "trusted-registries": ["index.docker.io", "docker.io", "ghcr.io", "gcr.io", "quay.io"] } }, - "root-user": {}, "secrets": { "secrets-policy": { "check-env-vars": true, diff --git a/config/config-inline.yaml b/config/config-inline.yaml index b9be859..fa10de8 100644 --- a/config/config-inline.yaml +++ b/config/config-inline.yaml @@ -14,7 +14,6 @@ checks: - ghcr.io - gcr.io - quay.io - root-user: {} secrets: secrets-policy: check-env-vars: true diff --git a/config/config.json b/config/config.json index d86baa1..46c4c76 100644 --- a/config/config.json +++ b/config/config.json @@ -13,7 +13,6 @@ "registry": { "registry-policy": "config/registry-policy.json" }, - "root-user": {}, "secrets": { "secrets-policy": "config/secrets-policy.json", "skip-env-vars": false, diff --git a/config/config.yaml b/config/config.yaml index 7747df1..08c13e2 100644 --- a/config/config.yaml +++ b/config/config.yaml @@ -8,7 +8,6 @@ checks: allowed-ports: "@config/allowed-ports.yaml" registry: registry-policy: config/registry-policy.yaml - root-user: {} secrets: secrets-policy: config/secrets-policy.yaml skip-env-vars: false diff --git a/internal/output/format_test.go b/internal/output/format_test.go index b48f023..3da643f 100644 --- a/internal/output/format_test.go +++ b/internal/output/format_test.go @@ -107,7 +107,7 @@ func TestRenderJSON(t *testing.T) { Passed: false, Checks: []CheckResult{ {Check: "age", Image: "nginx:latest", Passed: true, Message: "ok"}, - {Check: "root-user", Image: "nginx:latest", Passed: false, Message: "fail"}, + {Check: "user", Image: "nginx:latest", Passed: false, Message: "fail"}, }, Summary: Summary{ Total: 6, diff --git a/internal/output/results.go b/internal/output/results.go index f4f0371..af66690 100644 --- a/internal/output/results.go +++ b/internal/output/results.go @@ -46,11 +46,6 @@ type RegistryDetails struct { Skipped bool `json:"skipped,omitempty"` } -// RootUserDetails holds details for the root-user check. -type RootUserDetails struct { - User string `json:"user"` -} - // SecretsDetails holds details for the secrets check. type SecretsDetails struct { EnvVarFindings []EnvVarFinding `json:"env-var-findings,omitempty"` diff --git a/internal/secrets/detector_test.go b/internal/secrets/detector_test.go index fa86a7d..3e3a0d5 100644 --- a/internal/secrets/detector_test.go +++ b/internal/secrets/detector_test.go @@ -693,7 +693,7 @@ func TestCheckFilesInLayers_CorruptedLayer(t *testing.T) { func TestCheckFilesInLayers_CancelledContext(t *testing.T) { // Create image with multiple layers img := empty.Image - for i := 0; i < 3; i++ { + for i := range 3 { layer := createLayerWithFiles(t, map[string]string{ "/app/file" + string(rune('a'+i)) + ".txt": "content", }) diff --git a/internal/user/policy_test.go b/internal/user/policy_test.go index 0a7cebc..c66d2ca 100644 --- a/internal/user/policy_test.go +++ b/internal/user/policy_test.go @@ -187,29 +187,29 @@ func TestPolicyValidate(t *testing.T) { }, { name: "min-uid only", - policy: Policy{MinUID: ptrUint(1000)}, + policy: Policy{MinUID: new(uint(1000))}, }, { name: "max-uid only", - policy: Policy{MaxUID: ptrUint(65534)}, + policy: Policy{MaxUID: new(uint(65534))}, }, { name: "min-uid equals max-uid", - policy: Policy{MinUID: ptrUint(1000), MaxUID: ptrUint(1000)}, + policy: Policy{MinUID: new(uint(1000)), MaxUID: new(uint(1000))}, }, { name: "min-uid less than max-uid", - policy: Policy{MinUID: ptrUint(1000), MaxUID: ptrUint(65534)}, + policy: Policy{MinUID: new(uint(1000)), MaxUID: new(uint(65534))}, }, { name: "min-uid exceeds max-uid", - policy: Policy{MinUID: ptrUint(65534), MaxUID: ptrUint(1000)}, + policy: Policy{MinUID: new(uint(65534)), MaxUID: new(uint(1000))}, wantErr: true, errContains: "min-uid (65534) must not exceed max-uid (1000)", }, { name: "min-uid zero with max-uid", - policy: Policy{MinUID: ptrUint(0), MaxUID: ptrUint(65534)}, + policy: Policy{MinUID: new(uint(0)), MaxUID: new(uint(65534))}, }, } @@ -238,7 +238,3 @@ func TestLoadUserPolicy_MinExceedsMax(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "min-uid (65534) must not exceed max-uid (1000)") } - -func ptrUint(v uint) *uint { - return &v -} diff --git a/internal/user/validator_test.go b/internal/user/validator_test.go index 34b94e3..8683f3f 100644 --- a/internal/user/validator_test.go +++ b/internal/user/validator_test.go @@ -30,14 +30,14 @@ func TestParseUser(t *testing.T) { input: "1000", userPart: "1000", isNumeric: true, - uid: ptrUint64(1000), + uid: new(uint64(1000)), }, { name: "UID zero", input: "0", userPart: "0", isNumeric: true, - uid: ptrUint64(0), + uid: new(uint64(0)), }, { name: "UID with GID", @@ -45,7 +45,7 @@ func TestParseUser(t *testing.T) { userPart: "1000", groupPart: "1000", isNumeric: true, - uid: ptrUint64(1000), + uid: new(uint64(1000)), }, { name: "username with group", @@ -73,7 +73,7 @@ func TestParseUser(t *testing.T) { userPart: "0", groupPart: "somegroup", isNumeric: true, - uid: ptrUint64(0), + uid: new(uint64(0)), }, } @@ -190,27 +190,27 @@ func TestValidateUser_WithPolicy(t *testing.T) { { name: "UID in range passes", user: "1000", - policy: &Policy{MinUID: ptrUint(1000), MaxUID: ptrUint(65534)}, + policy: &Policy{MinUID: new(uint(1000)), MaxUID: new(uint(65534))}, passed: true, }, { name: "UID below minimum", user: "500", - policy: &Policy{MinUID: ptrUint(1000)}, + policy: &Policy{MinUID: new(uint(1000))}, passed: false, violations: []string{"min-uid"}, }, { name: "UID above maximum", user: "70000", - policy: &Policy{MaxUID: ptrUint(65534)}, + policy: &Policy{MaxUID: new(uint(65534))}, passed: false, violations: []string{"max-uid"}, }, { name: "UID below minimum and above maximum", user: "500", - policy: &Policy{MinUID: ptrUint(1000), MaxUID: ptrUint(400)}, + policy: &Policy{MinUID: new(uint(1000)), MaxUID: new(uint(400))}, passed: false, violations: []string{"min-uid", "max-uid"}, }, @@ -230,65 +230,65 @@ func TestValidateUser_WithPolicy(t *testing.T) { { name: "require numeric with username", user: "appuser", - policy: &Policy{RequireNumeric: ptrBool(true)}, + policy: &Policy{RequireNumeric: new(true)}, passed: false, violations: []string{"require-numeric"}, }, { name: "require numeric with UID", user: "1000", - policy: &Policy{RequireNumeric: ptrBool(true)}, + policy: &Policy{RequireNumeric: new(true)}, passed: true, }, { name: "require numeric false with username", user: "appuser", - policy: &Policy{RequireNumeric: ptrBool(false)}, + policy: &Policy{RequireNumeric: new(false)}, passed: true, }, { name: "username not affected by UID range", user: "appuser", - policy: &Policy{MinUID: ptrUint(1000), MaxUID: ptrUint(65534)}, + policy: &Policy{MinUID: new(uint(1000)), MaxUID: new(uint(65534))}, passed: true, }, { name: "username blocked and require-numeric", user: "daemon", - policy: &Policy{BlockedUsers: []string{"daemon"}, RequireNumeric: ptrBool(true)}, + policy: &Policy{BlockedUsers: []string{"daemon"}, RequireNumeric: new(true)}, passed: false, violations: []string{"require-numeric", "blocked-user"}, }, { name: "UID at exact minimum boundary", user: "1000", - policy: &Policy{MinUID: ptrUint(1000)}, + policy: &Policy{MinUID: new(uint(1000))}, passed: true, }, { name: "UID at exact maximum boundary", user: "65534", - policy: &Policy{MaxUID: ptrUint(65534)}, + policy: &Policy{MaxUID: new(uint(65534))}, passed: true, }, { name: "UID one below minimum", user: "999", - policy: &Policy{MinUID: ptrUint(1000)}, + policy: &Policy{MinUID: new(uint(1000))}, passed: false, violations: []string{"min-uid"}, }, { name: "UID one above maximum", user: "65535", - policy: &Policy{MaxUID: ptrUint(65534)}, + policy: &Policy{MaxUID: new(uint(65534))}, passed: false, violations: []string{"max-uid"}, }, { name: "UID with group and policy passes", user: "1000:1000", - policy: &Policy{MinUID: ptrUint(1000), MaxUID: ptrUint(65534)}, + policy: &Policy{MinUID: new(uint(1000)), MaxUID: new(uint(65534))}, passed: true, }, { @@ -323,7 +323,7 @@ func TestValidateUser_WithPolicy(t *testing.T) { func TestValidateUser_AlwaysEnforcedRegardlessOfPolicy(t *testing.T) { // Even with a permissive policy, root checks are always enforced - policy := &Policy{MinUID: ptrUint(0), MaxUID: ptrUint(65534)} + policy := &Policy{MinUID: new(uint(0)), MaxUID: new(uint(65534))} t.Run("empty user fails even with policy", func(t *testing.T) { result := ValidateUser("", policy) @@ -355,13 +355,13 @@ func TestValidateUser_ViolationMessages(t *testing.T) { }) t.Run("min-uid violation message", func(t *testing.T) { - result := ValidateUser("500", &Policy{MinUID: ptrUint(1000)}) + result := ValidateUser("500", &Policy{MinUID: new(uint(1000))}) require.Len(t, result.Violations, 1) assert.Equal(t, "UID 500 is below minimum 1000", result.Violations[0].Message) }) t.Run("max-uid violation message", func(t *testing.T) { - result := ValidateUser("70000", &Policy{MaxUID: ptrUint(65534)}) + result := ValidateUser("70000", &Policy{MaxUID: new(uint(65534))}) require.Len(t, result.Violations, 1) assert.Equal(t, "UID 70000 is above maximum 65534", result.Violations[0].Message) }) @@ -373,16 +373,8 @@ func TestValidateUser_ViolationMessages(t *testing.T) { }) t.Run("require-numeric violation message", func(t *testing.T) { - result := ValidateUser("appuser", &Policy{RequireNumeric: ptrBool(true)}) + result := ValidateUser("appuser", &Policy{RequireNumeric: new(true)}) require.Len(t, result.Violations, 1) assert.Equal(t, `user "appuser" must be a numeric UID`, result.Violations[0].Message) }) } - -func ptrUint64(v uint64) *uint64 { - return &v -} - -func ptrBool(v bool) *bool { - return &v -}