diff --git a/.kiro/specs/bootstrap-ai-coding/design-architecture.md b/.kiro/specs/bootstrap-ai-coding/design-architecture.md index b6a7f0a..adf02bf 100644 --- a/.kiro/specs/bootstrap-ai-coding/design-architecture.md +++ b/.kiro/specs/bootstrap-ai-coding/design-architecture.md @@ -320,6 +320,8 @@ RUN SSH authorized_keys ← stable per user key, cached RUN SSH host key injection ← stable per project, cached RUN sshd_config hardening ← stable, cached RUN mkdir /run/sshd ← stable, cached +RUN apt-get install dbus-x11 gnome-keyring libsecret-1-0 ← keyring (CC-7), cached +RUN install /etc/profile.d/dbus-keyring.sh ← keyring startup script, cached RUN apt-get install curl ca-certificates ← agent step, cached after first build RUN nodesource setup + nodejs ← agent step, cached after first build RUN npm install -g @augmentcode/auggie ← agent step, cached after first build @@ -327,6 +329,40 @@ RUN echo manifest > /bac-manifest.json ← stable when agents unchanged, cac CMD ["/usr/sbin/sshd", "-D"] ← always last (Req 21.2) ``` +### Headless Keyring (D-Bus + gnome-keyring-daemon) + +The container runs a headless `gnome-keyring-daemon` so that tools using `libsecret` / D-Bus Secret Service API (Claude Code, VS Code extensions) can store and retrieve OAuth tokens without a graphical desktop. + +**Installed in the base layer** (inside `NewDockerfileBuilder`), not in individual agent modules, because multiple agents and IDE extensions benefit from it. + +**Packages installed:** +- `dbus-x11` — provides `dbus-launch` for starting a session bus +- `gnome-keyring` — Secret Service provider +- `libsecret-1-0` — client library (used by Node.js `keytar` / `libsecret` bindings) + +**Startup mechanism:** +A shell profile script (`/etc/profile.d/dbus-keyring.sh`) is installed that: +1. Starts a D-Bus session bus via `dbus-launch` (if not already running) +2. Exports `DBUS_SESSION_BUS_ADDRESS` +3. Unlocks `gnome-keyring-daemon` with an empty password via stdin pipe + +```sh +#!/bin/sh +# /etc/profile.d/dbus-keyring.sh — start D-Bus + gnome-keyring for headless SSH sessions +if [ -z "$DBUS_SESSION_BUS_ADDRESS" ]; then + eval $(dbus-launch --sh-syntax) + export DBUS_SESSION_BUS_ADDRESS +fi +# Unlock the default keyring with an empty password +echo "" | gnome-keyring-daemon --unlock --components=secrets 2>/dev/null +``` + +This script runs on every SSH login (interactive shells source `/etc/profile.d/*.sh`). The keyring is per-session and uses an empty password, which is acceptable because the container is single-user and access is already gated by SSH key authentication. + +**Validates: CC-7** + +--- + ### Base Image User Inspection `docker/client.go` exposes a helper to detect UID/GID conflicts in the base image before building (Req 10a): @@ -663,3 +699,123 @@ The `TestAFindConflictingUserPullsImageIfAbsent` test (in `internal/docker`) is | `--stop-and-remove`, container not found | Docker inspect | Informational message → stdout, exit 0 | | Container already running | Docker inspect before create | Session summary → stdout, exit 0 | | `--purge` user declines confirmation | Confirmation prompt | Exit 0, nothing deleted | + +--- + +## Semantic Refactoring (Req 22–27) + +Internal code quality improvements: consolidate duplicated helpers, fix misplaced responsibilities, clarify intent. No user-facing behaviour changes. + +--- + +### PathUtil Package (Req 22) + +New package `internal/pathutil` with zero internal dependencies (only stdlib): + +```go +package pathutil + +import ( + "os" + "path/filepath" +) + +// ExpandHome expands a leading "~/" to the user's home directory. +func ExpandHome(p string) string { + if len(p) >= 2 && p[:2] == "~/" { + home, _ := os.UserHomeDir() + return filepath.Join(home, p[2:]) + } + return p +} +``` + +All packages that currently define their own `expandHome` (`naming`, `ssh`, `credentials`, `datadir`, `cmd`) remove the local copy and import `pathutil.ExpandHome`. Tests in `cmd_test` that reference `cmd.ExpandHome` switch to `pathutil.ExpandHome`. + +**Validates: Req 22** + +--- + +### ExecInContainer Client Parameter (Req 23) + +The `Agent.HealthCheck` interface and `docker.ExecInContainer` function both gain a `*docker.Client` parameter: + +```go +// Agent interface change: +HealthCheck(ctx context.Context, c *docker.Client, containerID string) error + +// ExecInContainer signature change: +func ExecInContainer(ctx context.Context, c *Client, containerID string, cmd []string) (int, error) +``` + +Call chain: `cmd/root.go` (has `dockerClient`) → `agent.HealthCheck(ctx, dockerClient, containerID)` → `docker.ExecInContainer(ctx, dockerClient, containerID, cmd)`. + +**Validates: Req 23** + +--- + +### Consolidated Flag Validation (Req 24) + +Replace 7 individual `cmd.Flags().Changed(...)` blocks with: + +```go +if mode == ModeStop || mode == ModePurge { + var changed []string + cmd.Flags().Visit(func(f *pflag.Flag) { + changed = append(changed, f.Name) + }) + if err := ValidateStartOnlyFlags(mode, changed); err != nil { + return err + } +} +``` + +Dead code removed: private `stringSlicesEqual` and `expandHome` wrappers. Exported `StringSlicesEqual` remains. + +**Validates: Req 24** + +--- + +### Split ListBACImages (Req 25) + +```go +// ListBACImages returns images with the "bac.managed=true" label only. +func ListBACImages(ctx context.Context, c *Client) ([]image.Summary, error) + +// ListBACImagesWithFallback returns labeled images, falling back to a tag-prefix +// scan for images built before labels were introduced (pre-label compatibility). +// This fallback can be removed once all users have rebuilt their images with --rebuild. +func ListBACImagesWithFallback(ctx context.Context, c *Client) ([]image.Summary, error) +``` + +`runPurge` uses `ListBACImagesWithFallback`. Other callers use `ListBACImages`. + +**Validates: Req 25** + +--- + +### HostBindIP Constant (Req 26) + +```go +// HostBindIP is the IP address the container's SSH port is bound to on the host. +HostBindIP = "127.0.0.1" +``` + +Used by `CreateContainer` (port binding) and `WaitForSSH` (TCP dial target). Decouples the bind address from `KnownHostsPatterns` which remains unchanged for known_hosts entry generation. + +**Validates: Req 26** + +--- + +### CredentialPreparer File Split (Req 27) + +``` +internal/agent/ + agent.go # Agent interface only (6 methods) + preparer.go # CredentialPreparer optional interface + registry.go # Registry functions +``` + +Pure file reorganization. No functional change. + +**Validates: Req 27** diff --git a/.kiro/specs/bootstrap-ai-coding/requirements-agents.md b/.kiro/specs/bootstrap-ai-coding/requirements-agents.md index 430b91b..32397be 100644 --- a/.kiro/specs/bootstrap-ai-coding/requirements-agents.md +++ b/.kiro/specs/bootstrap-ai-coding/requirements-agents.md @@ -61,6 +61,7 @@ Claude Code is Anthropic's AI coding agent. It is the first and default agent mo 2. THE Claude Code module SHALL declare `/.claude` as its Credential_Volume mount path inside the Container. 3. THE Credential_Volume SHALL be a bind-mount so that authentication tokens written inside the Container are immediately persisted to the Host Credential_Store. 4. Authentication tokens persisted in the Host Credential_Store SHALL be available in future Sessions without re-authentication. +5. NOTE: Claude Code also stores onboarding state in `~/.claude.json` (outside the credential directory). See Requirement CC-8 for how this is handled via symlink and host-side synchronisation. --- @@ -89,6 +90,21 @@ Claude Code is Anthropic's AI coding agent. It is the first and default agent mo --- +### Requirement CC-7: Headless Keyring for Credential Persistence + +**User Story:** As a developer, I want Claude Code to be able to read and refresh its OAuth tokens inside the container without a graphical desktop, so I don't have to re-authenticate every time I connect. + +#### Acceptance Criteria + +1. THE container image SHALL include a D-Bus session bus and a Secret Service–compatible keyring daemon (gnome-keyring) capable of running without a graphical display. +2. THE keyring daemon SHALL be started automatically when the Container_User's SSH session begins, using an empty password to unlock the default keyring. +3. Claude Code (and any other tool using `libsecret` / D-Bus Secret Service API) SHALL be able to store and retrieve credentials via the running keyring daemon without user interaction. +4. THE `DBUS_SESSION_BUS_ADDRESS` environment variable SHALL be set correctly for the Container_User's session so that client applications can locate the session bus. +5. THE keyring setup SHALL NOT interfere with the existing SSH-based authentication or the bind-mounted `~/.claude` credential store. +6. THE keyring packages and startup configuration SHALL be installed as part of the base container image (in `DockerfileBuilder`), not in individual agent modules, since multiple agents and IDE extensions may benefit from it. + +--- + ### Requirement CC-6: No Core Coupling **User Story:** As a platform maintainer, I want the Claude Code module to be fully self-contained so that removing or replacing it requires no changes to core code. @@ -101,6 +117,20 @@ Claude Code is Anthropic's AI coding agent. It is the first and default agent mo --- +### Requirement CC-8: Onboarding State Synchronisation + +**User Story:** As a developer, I want my Claude Code onboarding state to persist across container recreations, so I am not prompted to complete the onboarding flow every time the container is rebuilt. + +#### Acceptance Criteria + +1. Claude Code stores its onboarding state (including `hasCompletedOnboarding`) in `~/.claude.json` on the Host — a file in the home directory root, separate from the `~/.claude/` credential directory. +2. THE Claude Code module SHALL create a symlink inside the Container at `/.claude.json` pointing to `/.claude/claude.json`, so that Claude Code reads and writes its onboarding state through the bind-mounted Credential_Volume. +3. THE Claude Code module SHALL implement the `CredentialPreparer` interface. Its `PrepareCredentials` method SHALL copy `~/.claude.json` from the Host home directory into the Credential_Store as `claude.json`, but only when the source file exists and is newer than the destination (or the destination is absent). +4. THE combination of the symlink (inside the container) and the host-side copy (before mount) SHALL ensure that a single bind-mount on `~/.claude/` persists both OAuth tokens and onboarding state across container rebuilds and restarts. +5. IF `~/.claude.json` does not exist on the Host (first-time user), THE `PrepareCredentials` method SHALL silently skip the copy without error. + +--- + ## Augment Code Agent ### Overview diff --git a/.kiro/specs/bootstrap-ai-coding/requirements-core.md b/.kiro/specs/bootstrap-ai-coding/requirements-core.md index f76ed79..4427e08 100644 --- a/.kiro/specs/bootstrap-ai-coding/requirements-core.md +++ b/.kiro/specs/bootstrap-ai-coding/requirements-core.md @@ -154,8 +154,9 @@ The core application is responsible for all orchestration: Docker lifecycle mana 2. THE Container_Image SHALL include installation steps only for Enabled_Agents; agents not in the Enabled_Agents set SHALL NOT be installed in the image. 3. WHEN a Container is started, THE CLI SHALL mount each Enabled_Agent's Credential_Store from the Host as a Credential_Volume at the path declared by that Agent via the Agent_Interface. 4. WHEN the Credential_Store directory for an Enabled_Agent does not exist on the Host, THE CLI SHALL create it before starting the Container. -5. WHEN a Container is started and the Credential_Store for an Enabled_Agent contains no existing authentication tokens, THE CLI SHALL print a message to stdout identifying that Agent by name and instructing the user to authenticate it inside the Container. -6. Credentials written by an Agent inside the Container SHALL be immediately persisted to the Host Credential_Store via the bind-mount, and SHALL be available in future Sessions without re-authentication. +5. IF an Enabled_Agent implements the optional `CredentialPreparer` interface, THE CLI SHALL call `PrepareCredentials(storePath)` after ensuring the Credential_Store directory exists and before starting the Container. This allows agents to synchronise external state (e.g. onboarding files stored outside the Credential_Store) into the mounted directory. +6. WHEN a Container is started and the Credential_Store for an Enabled_Agent contains no existing authentication tokens, THE CLI SHALL print a message to stdout identifying that Agent by name and instructing the user to authenticate it inside the Container. +7. Credentials written by an Agent inside the Container SHALL be immediately persisted to the Host Credential_Store via the bind-mount, and SHALL be available in future Sessions without re-authentication. --- @@ -250,10 +251,11 @@ The core application is responsible for all orchestration: Docker lifecycle mana 1. WHEN the CLI starts a Container and no existing Container_Image is found, THE CLI SHALL build the Container_Image automatically before starting the Container. 2. THE CLI SHALL store a manifest file inside the Container_Image (at `/bac-manifest.json`) listing the Enabled_Agents used to build it. 3. WHEN the CLI starts a Container and the Enabled_Agents set does not match the manifest in the existing Container_Image, THE CLI SHALL stop, print a message to stdout informing the user that the agent configuration has changed, and instruct them to run with `--rebuild` to update the image. -4. THE CLI SHALL support a `--rebuild` flag that forces a full Container_Image rebuild regardless of the existing manifest. -5. WHEN a rebuild is triggered (automatically or via `--rebuild`), THE CLI SHALL print a message to stdout indicating that the image is being built. -6. IF the image build fails, THE CLI SHALL print the build output to stderr and exit with a non-zero exit code. -7. THE CLI SHALL enforce a maximum build duration of the Image_Build_Timeout. IF the build exceeds this deadline, THE CLI SHALL cancel the build, print a descriptive error message to stderr identifying the timeout, and exit with a non-zero exit code. +4. THE CLI SHALL support a `--rebuild` flag that forces a full Container_Image rebuild regardless of the existing manifest. WHEN `--rebuild` is used, THE CLI SHALL disable the Docker layer cache (`NoCache`) so that all Dockerfile steps are re-executed from scratch. +5. WHEN `--rebuild` is used and the Container is already running, THE CLI SHALL stop and remove the existing Container before creating a new one from the rebuilt image. +6. WHEN a rebuild is triggered (automatically or via `--rebuild`), THE CLI SHALL print a message to stdout indicating that the image is being built. +7. IF the image build fails, THE CLI SHALL print the build output to stderr and exit with a non-zero exit code. +8. THE CLI SHALL enforce a maximum build duration of the Image_Build_Timeout. IF the build exceeds this deadline, THE CLI SHALL cancel the build, print a descriptive error message to stderr identifying the timeout, and exit with a non-zero exit code. --- @@ -371,3 +373,4 @@ The core application is responsible for all orchestration: Docker lifecycle mana 5. Agent modules SHALL append only `RUN` (and optionally `ENV`, `COPY`) instructions via `Install()` — never `CMD` or `FROM`. > **Rationale:** Docker's layer cache is sequential. Any instruction that changes invalidates all layers below it. Placing `CMD` before agent `RUN` steps means every agent installation step runs uncached on every build, even when the agent configuration has not changed. With `CMD` last, all `RUN` layers are stable and cached after the first build, reducing subsequent build times from minutes to seconds. + diff --git a/.kiro/specs/bootstrap-ai-coding/tasks.md b/.kiro/specs/bootstrap-ai-coding/tasks.md index a6fe6a4..8244316 100644 --- a/.kiro/specs/bootstrap-ai-coding/tasks.md +++ b/.kiro/specs/bootstrap-ai-coding/tasks.md @@ -67,3 +67,65 @@ When both Claude Code and Augment Code are enabled (the default), both agents in - [x] 5.1 Run `go build ./...` — must compile cleanly - [x] 5.2 Run `go vet -tags integration ./...` — must pass with no warnings - [x] 5.3 Run `go test ./...` (unit + PBT only) — must pass (no regressions) + +### Headless keyring for credential persistence (CC-7) + +Install D-Bus and gnome-keyring-daemon in the base container image so that tools using libsecret (Claude Code, VS Code extensions) can store and refresh OAuth tokens without a graphical desktop. + +- [x] 7. Add headless keyring support to the base container image + - [x] 7.1 Add keyring constant to `internal/constants/constants.go` + - Add `KeyringProfileScript = "/etc/profile.d/dbus-keyring.sh"` constant + - [x] 7.2 Update `DockerfileBuilder` in `internal/docker/builder.go` to install keyring packages and startup script + - After the `mkdir -p /run/sshd` step, add a RUN step that installs `dbus-x11`, `gnome-keyring`, and `libsecret-1-0` + - Add a RUN step that creates `/etc/profile.d/dbus-keyring.sh` with the startup script content + - The script must: start dbus-launch if `DBUS_SESSION_BUS_ADDRESS` is unset, export it, then unlock gnome-keyring-daemon with an empty password + - Make the script executable (chmod +x) + - [x] 7.3 Add unit/PBT tests for keyring in `internal/docker/builder_test.go` + - Property 52: Verify the generated Dockerfile contains `dbus-x11`, `gnome-keyring`, and `libsecret-1-0` installation for any UID/GID + - Property 53: Verify the generated Dockerfile contains the profile.d script creation at `constants.KeyringProfileScript` with `dbus-launch`, `gnome-keyring-daemon --unlock`, and `chmod +x` + - Unit test: Verify keyring is present in UserStrategyRename as well + - [x] 7.4 Verify build and tests pass + - Run `go build ./...` — must compile cleanly + - Run `go vet ./...` — must pass + - Run `go test ./...` — all unit and PBT tests must pass + +### Semantic refactoring (R1, R3, R4, R6, R7, R8) + +Internal code quality improvements: consolidate duplicated helpers, fix misplaced responsibilities, clarify intent. No user-facing behaviour changes. + +- [x] 8. Create `internal/pathutil` package and consolidate `expandHome` (R1) + - [x] 8.1 Create `internal/pathutil/pathutil.go` with `ExpandHome` function + - [x] 8.2 Create `internal/pathutil/pathutil_test.go` with property test (ExpandHome never returns ~/prefix) and unit tests + - [x] 8.3 Update `internal/naming/naming.go` — remove local `expandHome`, import `pathutil` + - [x] 8.4 Update `internal/ssh/keys.go` — remove local `expandHome`, import `pathutil` + - [x] 8.5 Update `internal/credentials/store.go` — remove local `expandHome`, import `pathutil` + - [x] 8.6 Update `internal/datadir/datadir.go` — remove local `expandHome`, import `pathutil` + - [x] 8.7 Update `internal/cmd/root.go` — remove both `expandHome` and `ExpandHome`, import `pathutil` + - [x] 8.8 Update `internal/cmd/root_test.go` — change `cmd.ExpandHome` references to `pathutil.ExpandHome` + - [x] 8.9 Run `go build ./...` and `go test ./...` to verify no regressions +- [x] 9. Update `ExecInContainer` to accept `*Client` parameter (R3) + - [x] 9.1 Change `Agent.HealthCheck` interface in `internal/agent/agent.go` to accept `*docker.Client` + - [x] 9.2 Update `internal/docker/runner.go` `ExecInContainer` signature to accept `*Client` + - [x] 9.3 Update `internal/agents/claude/claude.go` `HealthCheck` to match new interface + - [x] 9.4 Update `internal/agents/augment/augment.go` `HealthCheck` to match new interface + - [x] 9.5 Update `internal/agent/registry_test.go` stub to match new interface + - [x] 9.6 Update any integration tests that call `HealthCheck` or `ExecInContainer` + - [x] 9.7 Run `go build ./...` and `go test ./...` to verify no regressions +- [x] 10. Consolidate inline flag validation in `cmd/root.go` (R4) + - [x] 10.1 Replace 7 individual `cmd.Flags().Changed(...)` blocks with `cmd.Flags().Visit` + `ValidateStartOnlyFlags` call + - [x] 10.2 Remove private `stringSlicesEqual` wrapper (use `StringSlicesEqual` directly) + - [x] 10.3 Run `go build ./...` and `go test ./...` to verify no regressions +- [x] 11. Split `ListBACImages` into explicit and fallback variants (R6) + - [x] 11.1 Refactor `ListBACImages` in `internal/docker/runner.go` to return only labeled images + - [x] 11.2 Create `ListBACImagesWithFallback` with the tag-prefix scan logic and doc comment + - [x] 11.3 Update `runPurge` in `cmd/root.go` to call `ListBACImagesWithFallback` + - [x] 11.4 Run `go build ./...` and `go test ./...` to verify no regressions +- [x] 12. Extract `HostBindIP` constant (R7) + - [x] 12.1 Add `HostBindIP = "127.0.0.1"` to `internal/constants/constants.go` + - [x] 12.2 Update `CreateContainer` in `internal/docker/runner.go` to use `constants.HostBindIP` + - [x] 12.3 Update `WaitForSSH` call in `internal/cmd/root.go` to use `constants.HostBindIP` + - [x] 12.4 Run `go build ./...` and `go test ./...` to verify no regressions +- [x] 13. Move `CredentialPreparer` to its own file (R8) + - [x] 13.1 Create `internal/agent/preparer.go` with the `CredentialPreparer` interface + - [x] 13.2 Remove `CredentialPreparer` from `internal/agent/agent.go` + - [x] 13.3 Run `go build ./...` and `go test ./...` to verify no regressions diff --git a/.kiro/steering/agent-module.md b/.kiro/steering/agent-module.md index b3c637b..126ce02 100644 --- a/.kiro/steering/agent-module.md +++ b/.kiro/steering/agent-module.md @@ -72,9 +72,17 @@ func (a *aiderAgent) HasCredentials(storePath string) (bool, error) { } // HealthCheck verifies the agent is installed and runnable inside the container. -// Called by the core after the container starts. -func (a *aiderAgent) HealthCheck(ctx context.Context, containerID string) error { - return execInContainer(ctx, containerID, []string{"aider", "--version"}) +// Called by the core after the container starts. The *docker.Client is passed +// through from the caller — do not create a new client. +func (a *aiderAgent) HealthCheck(ctx context.Context, c *docker.Client, containerID string) error { + exitCode, err := docker.ExecInContainer(ctx, c, containerID, []string{"aider", "--version"}) + if err != nil { + return fmt.Errorf("aider health check failed: %w", err) + } + if exitCode != 0 { + return fmt.Errorf("aider health check failed: exit code %d", exitCode) + } + return nil } ``` @@ -143,14 +151,15 @@ Add a new section to `.kiro/specs/bootstrap-ai-coding/requirements-agents.md` fo | `CredentialStorePath()` | Default host path for auth tokens; may use `~/` prefix | | `ContainerMountPath()` | Absolute path inside container; use `constants.ContainerUserHome` as base | | `HasCredentials(path)` | `(true, nil)` if tokens exist; `(false, nil)` if empty; `(false, err)` on error | -| `HealthCheck(ctx, id)` | `nil` if agent is ready; non-nil error if not | +| `HealthCheck(ctx, c, id)` | `nil` if agent is ready; non-nil error if not. `c` is the existing `*docker.Client` — do not create a new one. | ## Import Rules for Agent Modules Agent modules may import: - `github.com/koudis/bootstrap-ai-coding/internal/agent` — to call `agent.Register()` -- `github.com/koudis/bootstrap-ai-coding/internal/docker` — for `*docker.DockerfileBuilder` +- `github.com/koudis/bootstrap-ai-coding/internal/docker` — for `*docker.DockerfileBuilder` and `*docker.Client` - `github.com/koudis/bootstrap-ai-coding/internal/constants` — for `ContainerUserHome` and other glossary values +- `github.com/koudis/bootstrap-ai-coding/internal/pathutil` — for `ExpandHome` if needed - Standard library packages Agent modules must **NOT** import: diff --git a/.kiro/steering/constants.md b/.kiro/steering/constants.md index 456916f..d33faf3 100644 --- a/.kiro/steering/constants.md +++ b/.kiro/steering/constants.md @@ -38,6 +38,7 @@ This means: | `KnownHostsFile` | `"~/.ssh/known_hosts"` | Known_Hosts_File (Req 18) | | `SSHConfigFile` | `"~/.ssh/config"` | SSH_Config_File (Req 19) | | `SSHDirPerm` | `0o700` | ~/.ssh directory permissions (Req 18.5) | +| `HostBindIP` | `"127.0.0.1"` | IP address containers bind SSH port to on the host (Req R7) | | `ImageBuildTimeout` | `8 * time.Minute` | Image_Build_Timeout (Req 14.7) | ### Variables (not const — Go does not support slice/map constants) diff --git a/.kiro/steering/product.md b/.kiro/steering/product.md index c52291a..736fea9 100644 --- a/.kiro/steering/product.md +++ b/.kiro/steering/product.md @@ -28,7 +28,7 @@ Enabled agents: claude-code, augment-code - **Zero-friction startup**: one command, one argument, ready to SSH in - **Pluggable agents**: AI coding agents (Claude Code, Augment Code, etc.) are self-contained modules — adding a new agent requires no changes to core code -- **Credential persistence**: per-agent bind-mounts keep auth tokens alive across sessions; login once, never again +- **Credential persistence**: per-agent bind-mounts keep auth tokens alive across sessions; a headless D-Bus keyring (gnome-keyring-daemon) runs inside the container so tools using libsecret can store and refresh OAuth tokens without a graphical desktop — login once, never again - **Non-root safety**: CLI refuses to run as root; containers run as Container_User with UID/GID matching the host user - **Stable SSH identity**: SSH host keys are generated once per project and reused across rebuilds — no `known_hosts` churn - **known_hosts consistency**: `~/.ssh/known_hosts` is kept in sync automatically; stale entries are detected and the user is prompted before replacement diff --git a/.kiro/steering/structure.md b/.kiro/steering/structure.md index 022d9a3..3b75497 100644 --- a/.kiro/steering/structure.md +++ b/.kiro/steering/structure.md @@ -10,6 +10,9 @@ bootstrap-ai-coding/ ├── constants/ │ └── constants.go # All constants from the requirements glossary — single source of truth │ + ├── pathutil/ + │ └── pathutil.go # Shared path helpers (ExpandHome) — zero internal dependencies + │ ├── cmd/ │ └── root.go # Cobra root command, flag definitions, top-level orchestration logic │ @@ -35,6 +38,7 @@ bootstrap-ai-coding/ │ ├── agent/ │ ├── agent.go # Agent interface — the stable API boundary between core and agent modules + │ ├── preparer.go # CredentialPreparer optional interface │ └── registry.go # AgentRegistry: Register / Lookup / All / KnownIDs │ └── agents/ @@ -48,10 +52,11 @@ bootstrap-ai-coding/ ## Architectural Rules - **All packages live under `internal/`.** The Go compiler enforces that nothing outside this module can import them. -- **Core has zero knowledge of agents.** Packages under `internal/cmd/`, `internal/naming/`, `internal/docker/`, `internal/ssh/`, `internal/credentials/`, `internal/datadir/`, `internal/portfinder/`, and `internal/agent/` must never import anything under `internal/agents/`. +- **Core has zero knowledge of agents.** Packages under `internal/cmd/`, `internal/naming/`, `internal/docker/`, `internal/ssh/`, `internal/credentials/`, `internal/datadir/`, `internal/portfinder/`, `internal/pathutil/`, and `internal/agent/` must never import anything under `internal/agents/`. - **Agent modules are wired in via blank imports in `main.go` only.** Each agent's `init()` calls `agent.Register()`. -- **Agent modules may import `internal/agent`, `internal/docker`, and `internal/constants` from the core.** They must not import `internal/cmd`, `internal/naming`, `internal/ssh`, `internal/credentials`, `internal/datadir`, `internal/portfinder`, or `internal/docker/runner`. +- **Agent modules may import `internal/agent`, `internal/docker`, `internal/constants`, and `internal/pathutil` from the core.** They must not import `internal/cmd`, `internal/naming`, `internal/ssh`, `internal/credentials`, `internal/datadir`, `internal/portfinder`, or `internal/docker/runner`. - **No package may hardcode values that exist in `internal/constants/`.** Always import and reference `constants.*`. +- **Path expansion (`~/` → home dir) must use `pathutil.ExpandHome`.** No package may define its own `expandHome` helper. - **Adding a new agent = one new package under `internal/agents/`.** No other files change. ## Import Path Pattern @@ -67,6 +72,7 @@ import ( // In internal packages: import ( "github.com/koudis/bootstrap-ai-coding/internal/constants" + "github.com/koudis/bootstrap-ai-coding/internal/pathutil" "github.com/koudis/bootstrap-ai-coding/internal/naming" ) ``` @@ -84,3 +90,4 @@ import ( - Manifest file inside image: `/bac-manifest.json` (constants.ManifestFilePath) — lists enabled agent IDs for rebuild detection - Default agents: `claude-code,augment-code` (constants.DefaultAgents) - File permissions: Tool_Data_Dir `0700` (constants.ToolDataDirPerm), all files within `0600` (constants.ToolDataFilePerm) +- Headless keyring: D-Bus session bus + gnome-keyring-daemon started via `/etc/profile.d/dbus-keyring.sh` on SSH login — enables libsecret-based credential storage (CC-7) diff --git a/.kiro/steering/tech.md b/.kiro/steering/tech.md index 072dc1f..662a972 100644 --- a/.kiro/steering/tech.md +++ b/.kiro/steering/tech.md @@ -29,7 +29,7 @@ go build ./... go test ./... # Run integration tests (requires Docker daemon) -go test -tags integration ./... +BAC_INTEGRATION_CONSENT=yes go test -tags integration -timeout 30m -p 1 ./... # Run a specific package's tests go test ./naming/... @@ -54,7 +54,7 @@ go run . --purge ## Testing Conventions - Unit and property-based tests: no build tag, run with `go test ./...` -- Integration tests: gated by `//go:build integration`, require a live Docker daemon +- Integration tests: gated by `//go:build integration`, require a live Docker daemon, must use `-p 1` to avoid cross-package Docker state races - Property tests use `rapid.Check(t, func(t *rapid.T) { ... })` with minimum 100 iterations - Property test tag format: `// Feature: bootstrap-ai-coding, Property N: ` - Coverage target: ≥ 80% line coverage on all non-integration packages diff --git a/.kiro/steering/testing.md b/.kiro/steering/testing.md index 9551600..62abe5c 100644 --- a/.kiro/steering/testing.md +++ b/.kiro/steering/testing.md @@ -16,7 +16,8 @@ ### Integration Tests - Gated by `//go:build integration` -- Run with `go test -tags integration -timeout 30m ./...` +- Run with `go test -tags integration -timeout 30m -p 1 ./...` +- **Must use `-p 1`** (sequential package execution) — integration test packages share Docker state (base image removal/pull) and will race or timeout if run in parallel - Require a live Docker daemon - Cover the full happy path, SSH connectivity, volume sync, credential persistence @@ -39,7 +40,7 @@ Aborted — no consent given. **To run integration tests:** ```bash -BAC_INTEGRATION_CONSENT=yes go test -tags integration -timeout 30m ./... +BAC_INTEGRATION_CONSENT=yes go test -tags integration -timeout 30m -p 1 ./... ``` #### Base image precondition: automatic removal @@ -55,7 +56,7 @@ In `internal/docker`, `TestAFindConflictingUserPullsImageIfAbsent` (named with ` **Running integration tests:** ```bash -BAC_INTEGRATION_CONSENT=yes go test -tags integration -timeout 30m ./... +BAC_INTEGRATION_CONSENT=yes go test -tags integration -timeout 30m -p 1 ./... ``` --- diff --git a/Makefile b/Makefile index 3dd5b9c..775c182 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,7 @@ test: ## test-integration: run integration tests (requires a running Docker daemon) test-integration: - go test -tags integration ./... + BAC_INTEGRATION_CONSENT=yes go test -tags integration -timeout 30m -p 1 ./... ## vet: run go vet vet: diff --git a/README.md b/README.md index 1c3cf2b..5fedfae 100644 --- a/README.md +++ b/README.md @@ -190,9 +190,11 @@ go test ./... # Integration tests # Integration tests (requires a running Docker daemon and explicit consent). +# Uses -p 1 to run packages sequentially — they share Docker state (base image +# removal/pull) and will race or timeout if run in parallel. # The suite automatically removes the base image before running to ensure # the pull path is exercised. No manual 'docker rmi' needed. -BAC_INTEGRATION_CONSENT=yes go test -tags integration -timeout 30m ./... +BAC_INTEGRATION_CONSENT=yes go test -tags integration -timeout 30m -p 1 ./... # Vet go vet ./... diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 49b3318..d9471b3 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -15,5 +15,6 @@ type Agent interface { CredentialStorePath() string ContainerMountPath() string HasCredentials(storePath string) (bool, error) - HealthCheck(ctx context.Context, containerID string) error + HealthCheck(ctx context.Context, c *docker.Client, containerID string) error } + diff --git a/internal/agent/preparer.go b/internal/agent/preparer.go new file mode 100644 index 0000000..f4ace76 --- /dev/null +++ b/internal/agent/preparer.go @@ -0,0 +1,9 @@ +package agent + +// CredentialPreparer is an optional interface that agents can implement to +// perform additional credential synchronisation before the container starts. +// For example, copying external state files into the credential store so they +// are available inside the bind-mounted directory. +type CredentialPreparer interface { + PrepareCredentials(storePath string) error +} diff --git a/internal/agent/registry_test.go b/internal/agent/registry_test.go index 35327d1..320b9ae 100644 --- a/internal/agent/registry_test.go +++ b/internal/agent/registry_test.go @@ -20,7 +20,7 @@ func (s *stubAgent) Install(_ *docker.DockerfileBuilder) {} func (s *stubAgent) CredentialStorePath() string { return "" } func (s *stubAgent) ContainerMountPath() string { return "" } func (s *stubAgent) HasCredentials(_ string) (bool, error) { return false, nil } -func (s *stubAgent) HealthCheck(_ context.Context, _ string) error { return nil } +func (s *stubAgent) HealthCheck(_ context.Context, _ *docker.Client, _ string) error { return nil } // newStub is a convenience constructor. func newStub(id string) agent.Agent { return &stubAgent{id: id} } diff --git a/internal/agents/augment/augment.go b/internal/agents/augment/augment.go index 00a959e..ff8eb52 100644 --- a/internal/agents/augment/augment.go +++ b/internal/agents/augment/augment.go @@ -84,8 +84,8 @@ func (a *augmentAgent) HasCredentials(storePath string) (bool, error) { // HealthCheck verifies that the auggie binary is present and executable inside // the running container by executing `auggie --version`. // Satisfies: AC-5 -func (a *augmentAgent) HealthCheck(ctx context.Context, containerID string) error { - exitCode, err := docker.ExecInContainer(ctx, containerID, []string{"auggie", "--version"}) +func (a *augmentAgent) HealthCheck(ctx context.Context, c *docker.Client, containerID string) error { + exitCode, err := docker.ExecInContainer(ctx, c, containerID, []string{"auggie", "--version"}) if err != nil { return fmt.Errorf("augment health check failed: %w", err) } diff --git a/internal/agents/augment/integration_test.go b/internal/agents/augment/integration_test.go index 87d181b..cfaaa6d 100644 --- a/internal/agents/augment/integration_test.go +++ b/internal/agents/augment/integration_test.go @@ -26,11 +26,19 @@ import ( "github.com/koudis/bootstrap-ai-coding/internal/testutil" ) -// TestMain gates the integration suite behind an explicit consent prompt. -// Integration tests can delete, update and pull Docker images. +// Package-level shared container state — built once in TestMain, reused by all tests. +var ( + sharedContainerName string + sharedSSHPort int + sharedClient *docker.Client + sharedImageTag string +) + +// TestMain gates the integration suite behind an explicit consent prompt, +// builds the container image once, starts the container, and tears it down +// after all tests complete. func TestMain(m *testing.M) { if _, err := exec.LookPath("docker"); err != nil { - // Docker not available — individual tests will skip themselves. os.Exit(m.Run()) } @@ -41,44 +49,60 @@ func TestMain(m *testing.M) { os.Exit(1) } - os.Exit(m.Run()) -} + if err := setupSharedContainer(); err != nil { + fmt.Fprintf(os.Stderr, "setupSharedContainer: %v\n", err) + os.Exit(1) + } -// setupContainerWithAugment builds a container image with the Augment Code -// agent installed, starts the container, waits for SSH to be ready, and -// returns the container name, SSH port, and a cleanup function. -func setupContainerWithAugment(t *testing.T) (containerName string, sshPort int, cleanup func()) { - t.Helper() + code := m.Run() - if _, err := exec.LookPath("docker"); err != nil { - t.Skip("docker not available") - } + teardownSharedContainer() + os.Exit(code) +} +func setupSharedContainer() error { ctx := context.Background() - projectDir := t.TempDir() + projectDir, err := os.MkdirTemp("", "bac-augment-integration-*") + if err != nil { + return fmt.Errorf("creating temp dir: %w", err) + } dirName := filepath.Base(projectDir) hostKeyPriv, hostKeyPub, err := sshpkg.GenerateHostKeyPair() - require.NoError(t, err, "generating host key pair") + if err != nil { + return fmt.Errorf("generating host key pair: %w", err) + } _, userPubKey, err := sshpkg.GenerateHostKeyPair() - require.NoError(t, err, "generating user key pair") + if err != nil { + return fmt.Errorf("generating user key pair: %w", err) + } u, err := user.Current() - require.NoError(t, err, "getting current user") + if err != nil { + return fmt.Errorf("getting current user: %w", err) + } uid, err := strconv.Atoi(u.Uid) - require.NoError(t, err, "parsing UID") + if err != nil { + return fmt.Errorf("parsing UID: %w", err) + } gid, err := strconv.Atoi(u.Gid) - require.NoError(t, err, "parsing GID") + if err != nil { + return fmt.Errorf("parsing GID: %w", err) + } - client, err := docker.NewClient() - require.NoError(t, err, "connecting to Docker daemon") + sharedClient, err = docker.NewClient() + if err != nil { + return fmt.Errorf("connecting to Docker daemon: %w", err) + } strategy := docker.UserStrategyCreate conflictingUser := "" - conflictingImageUser, err := docker.FindConflictingUser(ctx, client, uid, gid) - require.NoError(t, err, "checking base image for UID/GID conflicts") + conflictingImageUser, err := docker.FindConflictingUser(ctx, sharedClient, uid, gid) + if err != nil { + return fmt.Errorf("checking base image for UID/GID conflicts: %w", err) + } if conflictingImageUser != nil { strategy = docker.UserStrategyRename conflictingUser = conflictingImageUser.Username @@ -91,23 +115,26 @@ func setupContainerWithAugment(t *testing.T) (containerName string, sshPort int, strategy, conflictingUser, ) - // Install the Augment Code agent steps into the Dockerfile. augmentAgent, err := agent.Lookup(constants.AugmentCodeAgentName) - require.NoError(t, err, "looking up augment agent") + if err != nil { + return fmt.Errorf("looking up augment agent: %w", err) + } augmentAgent.Install(builder) port, err := findFreePortAugment() - require.NoError(t, err, "finding free port") + if err != nil { + return fmt.Errorf("finding free port: %w", err) + } - containerName = constants.ContainerNamePrefix + sanitizeAugment(dirName) - imageTag := containerName + ":latest" + sharedContainerName = constants.ContainerNamePrefix + sanitizeAugment(dirName) + sharedImageTag = sharedContainerName + ":latest" + sharedSSHPort = port - // CMD must be the last instruction — call Finalize() before Build(). builder.Finalize() spec := docker.ContainerSpec{ - Name: containerName, - ImageTag: imageTag, + Name: sharedContainerName, + ImageTag: sharedImageTag, Dockerfile: builder.Build(), Mounts: []docker.Mount{ { @@ -124,34 +151,44 @@ func setupContainerWithAugment(t *testing.T) (containerName string, sshPort int, HostGID: gid, } - _, err = docker.BuildImage(ctx, client, spec, true) - require.NoError(t, err, "building container image with augment") + _, err = docker.BuildImage(ctx, sharedClient, spec, true) + if err != nil { + return fmt.Errorf("building container image with augment: %w", err) + } - _, err = docker.CreateContainer(ctx, client, spec) - require.NoError(t, err, "creating container") + _, err = docker.CreateContainer(ctx, sharedClient, spec) + if err != nil { + return fmt.Errorf("creating container: %w", err) + } - err = docker.StartContainer(ctx, client, containerName) - require.NoError(t, err, "starting container") + err = docker.StartContainer(ctx, sharedClient, sharedContainerName) + if err != nil { + return fmt.Errorf("starting container: %w", err) + } - // Augment installation takes longer — allow up to 2 minutes for SSH. err = docker.WaitForSSH(ctx, "127.0.0.1", port, 120*time.Second) - require.NoError(t, err, "waiting for SSH to be ready") - - cleanup = func() { - cleanCtx := context.Background() - _ = docker.StopContainer(cleanCtx, client, containerName) - _ = docker.RemoveContainer(cleanCtx, client, containerName) - images, _ := docker.ListBACImages(cleanCtx, client) - for _, img := range images { - for _, tag := range img.RepoTags { - if tag == imageTag { - _, _ = client.ImageRemove(cleanCtx, img.ID, dockerimage.RemoveOptions{Force: true}) - } + if err != nil { + return fmt.Errorf("waiting for SSH to be ready: %w", err) + } + + return nil +} + +func teardownSharedContainer() { + ctx := context.Background() + if sharedClient == nil { + return + } + _ = docker.StopContainer(ctx, sharedClient, sharedContainerName) + _ = docker.RemoveContainer(ctx, sharedClient, sharedContainerName) + images, _ := docker.ListBACImages(ctx, sharedClient) + for _, img := range images { + for _, tag := range img.RepoTags { + if tag == sharedImageTag { + _, _ = sharedClient.ImageRemove(ctx, img.ID, dockerimage.RemoveOptions{Force: true}) } } } - - return containerName, port, cleanup } // ---------------------------------------------------------------------------- @@ -164,13 +201,9 @@ func TestAugmentAvailableInContainer(t *testing.T) { t.Skip("docker not available") } - containerName, _, cleanup := setupContainerWithAugment(t) - t.Cleanup(cleanup) - ctx := context.Background() - // Exec `auggie --version` inside the container and assert exit 0. - exitCode, err := docker.ExecInContainer(ctx, containerName, []string{"auggie", "--version"}) + exitCode, err := docker.ExecInContainer(ctx, sharedClient, sharedContainerName, []string{"auggie", "--version"}) require.NoError(t, err, "exec auggie --version") require.Equal(t, 0, exitCode, "expected 'auggie --version' to exit 0") } @@ -185,15 +218,12 @@ func TestAugmentHealthCheck(t *testing.T) { t.Skip("docker not available") } - containerName, _, cleanup := setupContainerWithAugment(t) - t.Cleanup(cleanup) - ctx := context.Background() augAgent, err := agent.Lookup(constants.AugmentCodeAgentName) require.NoError(t, err, "looking up augment agent") - err = augAgent.HealthCheck(ctx, containerName) + err = augAgent.HealthCheck(ctx, sharedClient, sharedContainerName) require.NoError(t, err, "augment HealthCheck should return no error") } diff --git a/internal/agents/claude/claude.go b/internal/agents/claude/claude.go index 6edb8fd..63ce3f2 100644 --- a/internal/agents/claude/claude.go +++ b/internal/agents/claude/claude.go @@ -32,6 +32,16 @@ func (a *claudeAgent) Install(b *docker.DockerfileBuilder) { b.MarkNodeInstalled() } b.Run("npm install -g --no-fund --no-audit @anthropic-ai/claude-code") + + // Symlink ~/.claude.json into the credential mount directory so that a single + // bind-mount on ~/.claude/ persists both OAuth tokens (.credentials.json) and + // onboarding state (claude.json). Without this, Claude Code triggers the full + // login/onboarding flow on every container start. + b.Run(fmt.Sprintf( + "ln -sf %s/claude.json %s/.claude.json", + filepath.Join(constants.ContainerUserHome, ".claude"), + constants.ContainerUserHome, + )) } func (a *claudeAgent) CredentialStorePath() string { @@ -55,8 +65,42 @@ func (a *claudeAgent) HasCredentials(storePath string) (bool, error) { return true, nil } -func (a *claudeAgent) HealthCheck(ctx context.Context, containerID string) error { - exitCode, err := docker.ExecInContainer(ctx, containerID, []string{"claude", "--version"}) +// PrepareCredentials copies ~/.claude.json into the credential store as +// claude.json (if it exists and the destination is absent or older). +// Inside the container a symlink at ~/.claude.json points to this file, +// so the bind-mount on ~/.claude/ covers both OAuth tokens and onboarding state. +func (a *claudeAgent) PrepareCredentials(storePath string) error { + home, err := os.UserHomeDir() + if err != nil { + return nil // best-effort; skip if we can't determine home + } + src := filepath.Join(home, ".claude.json") + dst := filepath.Join(storePath, "claude.json") + + srcInfo, err := os.Stat(src) + if err != nil { + // Source doesn't exist — nothing to sync (first-time user). + return nil + } + + // Only copy if destination is missing or older than source. + dstInfo, err := os.Stat(dst) + if err == nil && !dstInfo.ModTime().Before(srcInfo.ModTime()) { + return nil // destination is up-to-date + } + + data, err := os.ReadFile(src) + if err != nil { + return fmt.Errorf("reading %s: %w", src, err) + } + if err := os.WriteFile(dst, data, 0o600); err != nil { + return fmt.Errorf("writing %s: %w", dst, err) + } + return nil +} + +func (a *claudeAgent) HealthCheck(ctx context.Context, c *docker.Client, containerID string) error { + exitCode, err := docker.ExecInContainer(ctx, c, containerID, []string{"claude", "--version"}) if err != nil { return fmt.Errorf("claude health check failed: %w", err) } diff --git a/internal/agents/claude/claude_test.go b/internal/agents/claude/claude_test.go index 906167a..b5b99f4 100644 --- a/internal/agents/claude/claude_test.go +++ b/internal/agents/claude/claude_test.go @@ -319,8 +319,8 @@ func TestClaudeInstallNodeAlreadyInstalled(t *testing.T) { require.Contains(t, content, "curl ca-certificates git", "must always install curl, ca-certificates, git") - // Should have added exactly 2 lines (apt-get prereqs + npm install) + // Should have added exactly 3 lines (apt-get prereqs + npm install + symlink) linesAfter := len(b.Lines()) - require.Equal(t, linesBefore+2, linesAfter, - "must add exactly 2 RUN steps when Node.js is already installed (prereqs + npm)") + require.Equal(t, linesBefore+3, linesAfter, + "must add exactly 3 RUN steps when Node.js is already installed (prereqs + npm + symlink)") } diff --git a/internal/agents/claude/integration_test.go b/internal/agents/claude/integration_test.go index 46b746b..9d54703 100644 --- a/internal/agents/claude/integration_test.go +++ b/internal/agents/claude/integration_test.go @@ -26,11 +26,19 @@ import ( "github.com/koudis/bootstrap-ai-coding/internal/testutil" ) -// TestMain gates the integration suite behind an explicit consent prompt. -// Integration tests can delete, update and pull Docker images. +// Package-level shared container state — built once in TestMain, reused by all tests. +var ( + sharedContainerName string + sharedSSHPort int + sharedClient *docker.Client + sharedImageTag string +) + +// TestMain gates the integration suite behind an explicit consent prompt, +// builds the container image once, starts the container, and tears it down +// after all tests complete. func TestMain(m *testing.M) { if _, err := exec.LookPath("docker"); err != nil { - // Docker not available — individual tests will skip themselves. os.Exit(m.Run()) } @@ -41,44 +49,60 @@ func TestMain(m *testing.M) { os.Exit(1) } - os.Exit(m.Run()) -} + if err := setupSharedContainer(); err != nil { + fmt.Fprintf(os.Stderr, "setupSharedContainer: %v\n", err) + os.Exit(1) + } -// setupContainerWithClaude builds a container image with the Claude Code agent -// installed, starts the container, waits for SSH to be ready, and returns the -// container name, SSH port, and a cleanup function. -func setupContainerWithClaude(t *testing.T) (containerName string, sshPort int, cleanup func()) { - t.Helper() + code := m.Run() - if _, err := exec.LookPath("docker"); err != nil { - t.Skip("docker not available") - } + teardownSharedContainer() + os.Exit(code) +} +func setupSharedContainer() error { ctx := context.Background() - projectDir := t.TempDir() + projectDir, err := os.MkdirTemp("", "bac-claude-integration-*") + if err != nil { + return fmt.Errorf("creating temp dir: %w", err) + } dirName := filepath.Base(projectDir) hostKeyPriv, hostKeyPub, err := sshpkg.GenerateHostKeyPair() - require.NoError(t, err, "generating host key pair") + if err != nil { + return fmt.Errorf("generating host key pair: %w", err) + } _, userPubKey, err := sshpkg.GenerateHostKeyPair() - require.NoError(t, err, "generating user key pair") + if err != nil { + return fmt.Errorf("generating user key pair: %w", err) + } u, err := user.Current() - require.NoError(t, err, "getting current user") + if err != nil { + return fmt.Errorf("getting current user: %w", err) + } uid, err := strconv.Atoi(u.Uid) - require.NoError(t, err, "parsing UID") + if err != nil { + return fmt.Errorf("parsing UID: %w", err) + } gid, err := strconv.Atoi(u.Gid) - require.NoError(t, err, "parsing GID") + if err != nil { + return fmt.Errorf("parsing GID: %w", err) + } - client, err := docker.NewClient() - require.NoError(t, err, "connecting to Docker daemon") + sharedClient, err = docker.NewClient() + if err != nil { + return fmt.Errorf("connecting to Docker daemon: %w", err) + } strategy := docker.UserStrategyCreate conflictingUser := "" - conflictingImageUser, err := docker.FindConflictingUser(ctx, client, uid, gid) - require.NoError(t, err, "checking base image for UID/GID conflicts") + conflictingImageUser, err := docker.FindConflictingUser(ctx, sharedClient, uid, gid) + if err != nil { + return fmt.Errorf("checking base image for UID/GID conflicts: %w", err) + } if conflictingImageUser != nil { strategy = docker.UserStrategyRename conflictingUser = conflictingImageUser.Username @@ -91,23 +115,26 @@ func setupContainerWithClaude(t *testing.T) (containerName string, sshPort int, strategy, conflictingUser, ) - // Install the Claude Code agent steps into the Dockerfile. claudeAgent, err := agent.Lookup(constants.ClaudeCodeAgentName) - require.NoError(t, err, "looking up claude agent") + if err != nil { + return fmt.Errorf("looking up claude agent: %w", err) + } claudeAgent.Install(builder) port, err := findFreePortClaude() - require.NoError(t, err, "finding free port") + if err != nil { + return fmt.Errorf("finding free port: %w", err) + } - containerName = constants.ContainerNamePrefix + sanitizeClaude(dirName) - imageTag := containerName + ":latest" + sharedContainerName = constants.ContainerNamePrefix + sanitizeClaude(dirName) + sharedImageTag = sharedContainerName + ":latest" + sharedSSHPort = port - // CMD must be the last instruction — call Finalize() before Build(). builder.Finalize() spec := docker.ContainerSpec{ - Name: containerName, - ImageTag: imageTag, + Name: sharedContainerName, + ImageTag: sharedImageTag, Dockerfile: builder.Build(), Mounts: []docker.Mount{ { @@ -124,34 +151,45 @@ func setupContainerWithClaude(t *testing.T) (containerName string, sshPort int, HostGID: gid, } - _, err = docker.BuildImage(ctx, client, spec, false) - require.NoError(t, err, "building container image with claude") + _, err = docker.BuildImage(ctx, sharedClient, spec, false) + if err != nil { + return fmt.Errorf("building container image with claude: %w", err) + } - _, err = docker.CreateContainer(ctx, client, spec) - require.NoError(t, err, "creating container") + _, err = docker.CreateContainer(ctx, sharedClient, spec) + if err != nil { + return fmt.Errorf("creating container: %w", err) + } - err = docker.StartContainer(ctx, client, containerName) - require.NoError(t, err, "starting container") + err = docker.StartContainer(ctx, sharedClient, sharedContainerName) + if err != nil { + return fmt.Errorf("starting container: %w", err) + } // Claude installation takes longer — allow up to 2 minutes for SSH. err = docker.WaitForSSH(ctx, "127.0.0.1", port, 120*time.Second) - require.NoError(t, err, "waiting for SSH to be ready") - - cleanup = func() { - cleanCtx := context.Background() - _ = docker.StopContainer(cleanCtx, client, containerName) - _ = docker.RemoveContainer(cleanCtx, client, containerName) - images, _ := docker.ListBACImages(cleanCtx, client) - for _, img := range images { - for _, tag := range img.RepoTags { - if tag == imageTag { - _, _ = client.ImageRemove(cleanCtx, img.ID, dockerimage.RemoveOptions{Force: true}) - } + if err != nil { + return fmt.Errorf("waiting for SSH to be ready: %w", err) + } + + return nil +} + +func teardownSharedContainer() { + ctx := context.Background() + if sharedClient == nil { + return + } + _ = docker.StopContainer(ctx, sharedClient, sharedContainerName) + _ = docker.RemoveContainer(ctx, sharedClient, sharedContainerName) + images, _ := docker.ListBACImages(ctx, sharedClient) + for _, img := range images { + for _, tag := range img.RepoTags { + if tag == sharedImageTag { + _, _ = sharedClient.ImageRemove(ctx, img.ID, dockerimage.RemoveOptions{Force: true}) } } } - - return containerName, port, cleanup } // ---------------------------------------------------------------------------- @@ -164,13 +202,9 @@ func TestClaudeAvailableInContainer(t *testing.T) { t.Skip("docker not available") } - containerName, _, cleanup := setupContainerWithClaude(t) - t.Cleanup(cleanup) - ctx := context.Background() - // Exec `claude --version` inside the container and assert exit 0. - exitCode, err := docker.ExecInContainer(ctx, containerName, []string{"claude", "--version"}) + exitCode, err := docker.ExecInContainer(ctx, sharedClient, sharedContainerName, []string{"claude", "--version"}) require.NoError(t, err, "exec claude --version") require.Equal(t, 0, exitCode, "expected 'claude --version' to exit 0") } @@ -185,15 +219,12 @@ func TestClaudeHealthCheck(t *testing.T) { t.Skip("docker not available") } - containerName, _, cleanup := setupContainerWithClaude(t) - t.Cleanup(cleanup) - ctx := context.Background() claudeAgent, err := agent.Lookup(constants.ClaudeCodeAgentName) require.NoError(t, err, "looking up claude agent") - err = claudeAgent.HealthCheck(ctx, containerName) + err = claudeAgent.HealthCheck(ctx, sharedClient, sharedContainerName) require.NoError(t, err, "claude HealthCheck should return no error") } diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 5352649..5fff0f7 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -16,6 +16,7 @@ import ( "github.com/docker/docker/api/types/image" "github.com/spf13/cobra" + "github.com/spf13/pflag" "github.com/koudis/bootstrap-ai-coding/internal/agent" "github.com/koudis/bootstrap-ai-coding/internal/constants" @@ -23,6 +24,7 @@ import ( "github.com/koudis/bootstrap-ai-coding/internal/datadir" dockerpkg "github.com/koudis/bootstrap-ai-coding/internal/docker" "github.com/koudis/bootstrap-ai-coding/internal/naming" + "github.com/koudis/bootstrap-ai-coding/internal/pathutil" "github.com/koudis/bootstrap-ai-coding/internal/portfinder" sshpkg "github.com/koudis/bootstrap-ai-coding/internal/ssh" ) @@ -177,26 +179,12 @@ func run(cmd *cobra.Command, args []string) error { } if mode == ModeStop || mode == ModePurge { - if cmd.Flags().Changed("agents") { - return fmt.Errorf("--agents is not valid with %s", modeFlag(mode)) - } - if cmd.Flags().Changed("port") { - return fmt.Errorf("--port is not valid with %s", modeFlag(mode)) - } - if cmd.Flags().Changed("ssh-key") { - return fmt.Errorf("--ssh-key is not valid with %s", modeFlag(mode)) - } - if cmd.Flags().Changed("rebuild") { - return fmt.Errorf("--rebuild is not valid with %s", modeFlag(mode)) - } - if cmd.Flags().Changed("no-update-known-hosts") { - return fmt.Errorf("--no-update-known-hosts is not valid with %s", modeFlag(mode)) - } - if cmd.Flags().Changed("no-update-ssh-config") { - return fmt.Errorf("--no-update-ssh-config is not valid with %s", modeFlag(mode)) - } - if cmd.Flags().Changed("verbose") { - return fmt.Errorf("--verbose is not valid with %s", modeFlag(mode)) + var changed []string + cmd.Flags().Visit(func(f *pflag.Flag) { + changed = append(changed, f.Name) + }) + if err := ValidateStartOnlyFlags(mode, changed); err != nil { + return err } } @@ -304,7 +292,7 @@ func runPurge(c *dockerpkg.Client) error { if err != nil { return err } - images, err := dockerpkg.ListBACImages(ctx, c) + images, err := dockerpkg.ListBACImagesWithFallback(ctx, c) if err != nil { return err } @@ -330,7 +318,7 @@ func runPurge(c *dockerpkg.Client) error { fmt.Printf("This will delete:\n") fmt.Printf(" %d container(s)\n", len(containers)) fmt.Printf(" %d image(s)\n", len(images)) - fmt.Printf(" %s\n", expandHome(constants.ToolDataDirRoot)) + fmt.Printf(" %s\n", pathutil.ExpandHome(constants.ToolDataDirRoot)) fmt.Printf("\nType 'yes' to confirm: ") reader := bufio.NewReader(os.Stdin) @@ -469,6 +457,13 @@ func runStart(c *dockerpkg.Client, projectPath string, enabledAgents []agent.Age if err := credentials.EnsureDir(resolved); err != nil { return fmt.Errorf("ensuring credential dir for %s: %w", a.ID(), err) } + // If the agent implements CredentialPreparer, let it sync external + // state into the credential store before we mount it. + if prep, ok := a.(agent.CredentialPreparer); ok { + if err := prep.PrepareCredentials(resolved); err != nil { + fmt.Fprintf(os.Stderr, "warning: preparing credentials for %s: %v\n", a.ID(), err) + } + } hasCreds, err := a.HasCredentials(resolved) if err != nil { return fmt.Errorf("checking credentials for %s: %w", a.ID(), err) @@ -504,7 +499,7 @@ func runStart(c *dockerpkg.Client, projectPath string, enabledAgents []agent.Age var manifestIDs []string if err := json.Unmarshal([]byte(manifestJSON), &manifestIDs); err != nil { needBuild = true - } else if !stringSlicesEqual(manifestIDs, enabledIDs) { + } else if !StringSlicesEqual(manifestIDs, enabledIDs) { fmt.Println("Agent config changed — run with --rebuild to update the image.") return nil } @@ -557,6 +552,7 @@ func runStart(c *dockerpkg.Client, projectPath string, enabledAgents []agent.Age Labels: labels, HostUID: uid, HostGID: gid, + NoCache: flagRebuild, } fmt.Println("Building image...") @@ -572,16 +568,23 @@ func runStart(c *dockerpkg.Client, projectPath string, enabledAgents []agent.Age return err } if info != nil && info.State != nil && info.State.Running { - // Sync known_hosts even when reconnecting to an already-running container (Req 18.1). - if err := sshpkg.SyncKnownHosts(sshPort, hostKeyPub, flagNoUpdateKnownHosts); err != nil { - fmt.Fprintf(os.Stderr, "warning: syncing known_hosts: %v\n", err) - } - // Sync SSH config entry (Req 19.1). - if err := sshpkg.SyncSSHConfig(containerName, sshPort, flagNoUpdateSSHConfig); err != nil { - fmt.Fprintf(os.Stderr, "warning: syncing SSH config: %v\n", err) + if !flagRebuild { + // Sync known_hosts even when reconnecting to an already-running container (Req 18.1). + if err := sshpkg.SyncKnownHosts(sshPort, hostKeyPub, flagNoUpdateKnownHosts); err != nil { + fmt.Fprintf(os.Stderr, "warning: syncing known_hosts: %v\n", err) + } + // Sync SSH config entry (Req 19.1). + if err := sshpkg.SyncSSHConfig(containerName, sshPort, flagNoUpdateSSHConfig); err != nil { + fmt.Fprintf(os.Stderr, "warning: syncing SSH config: %v\n", err) + } + printSessionSummary(dd, absPath, containerName, sshPort, enabledIDs) + return nil } - printSessionSummary(dd, absPath, containerName, sshPort, enabledIDs) - return nil + // --rebuild: stop the running container so it gets recreated from the new image. + fmt.Println("Stopping existing container for rebuild...") + _ = dockerpkg.StopContainer(ctx, c, containerName) + _ = dockerpkg.RemoveContainer(ctx, c, containerName) + info = nil } if info != nil { @@ -613,7 +616,7 @@ func runStart(c *dockerpkg.Client, projectPath string, enabledAgents []agent.Age return err } - if err := dockerpkg.WaitForSSH(ctx, constants.KnownHostsPatterns[1], sshPort, 10*time.Second); err != nil { + if err := dockerpkg.WaitForSSH(ctx, constants.HostBindIP, sshPort, 10*time.Second); err != nil { _ = dockerpkg.StopContainer(ctx, c, containerName) _ = dockerpkg.RemoveContainer(ctx, c, containerName) return fmt.Errorf("container started but SSH did not become ready: %w", err) @@ -665,10 +668,6 @@ func hostUIDGID() (int, int, error) { return uid, gid, nil } -func stringSlicesEqual(a, b []string) bool { - return StringSlicesEqual(a, b) -} - // StringSlicesEqual reports whether a and b contain the same elements in the same order. func StringSlicesEqual(a, b []string) bool { if len(a) != len(b) { @@ -681,16 +680,3 @@ func StringSlicesEqual(a, b []string) bool { } return true } - -func expandHome(p string) string { - return ExpandHome(p) -} - -// ExpandHome expands a leading "~/" to the user's home directory. -func ExpandHome(p string) string { - if len(p) >= 2 && p[:2] == "~/" { - home, _ := os.UserHomeDir() - return filepath.Join(home, p[2:]) - } - return p -} diff --git a/internal/cmd/root_test.go b/internal/cmd/root_test.go index 9238999..61bb4a4 100644 --- a/internal/cmd/root_test.go +++ b/internal/cmd/root_test.go @@ -299,38 +299,6 @@ func TestPropertyStringSlicesEqualReflexiveAndSymmetric(t *testing.T) { }) } -// ── Unit tests for ExpandHome ───────────────────────────────────────────────── - -func TestExpandHomeNoTilde(t *testing.T) { - require.Equal(t, "/absolute/path", cmd.ExpandHome("/absolute/path")) -} - -func TestExpandHomeRelativePath(t *testing.T) { - require.Equal(t, "relative/path", cmd.ExpandHome("relative/path")) -} - -func TestExpandHomeTildeExpanded(t *testing.T) { - result := cmd.ExpandHome("~/somedir") - require.NotContains(t, result, "~", "ExpandHome must expand ~ to the home directory") - require.Contains(t, result, "somedir") -} - -func TestExpandHomeTildeOnly(t *testing.T) { - // "~" alone (no slash) should not be expanded. - require.Equal(t, "~", cmd.ExpandHome("~")) -} - -// Feature: bootstrap-ai-coding, Property 26: ExpandHome never returns a path starting with ~/ -func TestPropertyExpandHomeNeverReturnsTilde(t *testing.T) { - rapid.Check(t, func(t *rapid.T) { - input := rapid.String().Draw(t, "input") - result := cmd.ExpandHome(input) - require.False(t, - len(result) >= 2 && result[:2] == "~/", - "ExpandHome(%q) = %q must not start with ~/", input, result) - }) -} - // TestVerboseFlagWithStopRejected verifies that --verbose is rejected when // used with --stop-and-remove (CLI-3). // Validates: Req 20.5, CLI-3 diff --git a/internal/constants/constants.go b/internal/constants/constants.go index b587fc2..abb9319 100644 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -94,6 +94,15 @@ const ( // SSHDirPerm is the permission for the ~/.ssh directory on the Host. // Satisfies Req 18.5. SSHDirPerm = 0o700 + + // KeyringProfileScript is the path to the shell profile script inside the + // container that starts D-Bus and gnome-keyring-daemon on SSH login. + // Satisfies CC-7. + KeyringProfileScript = "/etc/profile.d/dbus-keyring.sh" + + // HostBindIP is the IP address containers bind their SSH port to on the host. + // Satisfies R7. + HostBindIP = "127.0.0.1" ) // ImageBuildTimeout is the maximum time allowed for a Docker image build. diff --git a/internal/credentials/store.go b/internal/credentials/store.go index c2518bb..7d65ebc 100644 --- a/internal/credentials/store.go +++ b/internal/credentials/store.go @@ -4,9 +4,9 @@ package credentials import ( "os" - "path/filepath" "github.com/koudis/bootstrap-ai-coding/internal/constants" + "github.com/koudis/bootstrap-ai-coding/internal/pathutil" ) // Resolve returns the effective credential store path for an agent. @@ -15,7 +15,7 @@ func Resolve(agentDefault, override string) string { if override != "" { return override } - return expandHome(agentDefault) + return pathutil.ExpandHome(agentDefault) } // EnsureDir creates the directory at path (and all parents) if it does not @@ -24,10 +24,3 @@ func EnsureDir(path string) error { return os.MkdirAll(path, constants.ToolDataDirPerm) } -func expandHome(p string) string { - if len(p) >= 2 && p[:2] == "~/" { - home, _ := os.UserHomeDir() - return filepath.Join(home, p[2:]) - } - return p -} diff --git a/internal/datadir/datadir.go b/internal/datadir/datadir.go index 774b994..b4a9d00 100644 --- a/internal/datadir/datadir.go +++ b/internal/datadir/datadir.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/koudis/bootstrap-ai-coding/internal/constants" + "github.com/koudis/bootstrap-ai-coding/internal/pathutil" ) // DataDir represents the Tool_Data_Dir for a single project. @@ -20,7 +21,7 @@ type DataDir struct { // New returns a DataDir for the given container name, creating the directory // (and all parents) with constants.ToolDataDirPerm if it does not exist. func New(containerName string) (*DataDir, error) { - root := expandHome(constants.ToolDataDirRoot) + root := pathutil.ExpandHome(constants.ToolDataDirRoot) p := filepath.Join(root, containerName) if err := os.MkdirAll(p, constants.ToolDataDirPerm); err != nil { return nil, fmt.Errorf("creating data dir %s: %w", p, err) @@ -121,14 +122,14 @@ func (d *DataDir) WriteManifest(agentIDs []string) error { // PurgeRoot removes the entire Tool_Data_Dir root and all its contents. func PurgeRoot() error { - return os.RemoveAll(expandHome(constants.ToolDataDirRoot)) + return os.RemoveAll(pathutil.ExpandHome(constants.ToolDataDirRoot)) } // ListContainerNames returns the names of all subdirectories under the // Tool_Data_Dir root. Each name corresponds to a container that has persisted // data. Returns nil (not an error) if the root directory does not exist. func ListContainerNames() ([]string, error) { - root := expandHome(constants.ToolDataDirRoot) + root := pathutil.ExpandHome(constants.ToolDataDirRoot) entries, err := os.ReadDir(root) if os.IsNotExist(err) { return nil, nil @@ -145,10 +146,3 @@ func ListContainerNames() ([]string, error) { return names, nil } -func expandHome(p string) string { - if len(p) >= 2 && p[:2] == "~/" { - home, _ := os.UserHomeDir() - return filepath.Join(home, p[2:]) - } - return p -} diff --git a/internal/docker/builder.go b/internal/docker/builder.go index 0947972..3053ae6 100644 --- a/internal/docker/builder.go +++ b/internal/docker/builder.go @@ -124,6 +124,18 @@ func NewDockerfileBuilder(uid, gid int, publicKey, hostKeyPriv, hostKeyPub strin // 8. Ensure sshd runtime dir exists b.Run("mkdir -p /run/sshd") + // 9. Install D-Bus and gnome-keyring for headless credential storage (CC-7). + // Tools using libsecret (Claude Code, VS Code extensions) need a Secret Service + // provider to store and refresh OAuth tokens without a graphical desktop. + b.Run("apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends dbus-x11 gnome-keyring libsecret-1-0 && rm -rf /var/lib/apt/lists/*") + + // 10. Install profile.d script that starts D-Bus + gnome-keyring on SSH login. + // Uses an empty password to unlock the keyring — acceptable because the container + // is single-user and access is gated by SSH key authentication. + keyringScript := `#!/bin/sh\nif [ -z \"$DBUS_SESSION_BUS_ADDRESS\" ]; then\n eval $(dbus-launch --sh-syntax)\n export DBUS_SESSION_BUS_ADDRESS\nfi\necho \"\" | gnome-keyring-daemon --unlock --components=secrets 2>/dev/null\n` + b.Run(fmt.Sprintf("printf '%s' > %s && chmod +x %s", + keyringScript, constants.KeyringProfileScript, constants.KeyringProfileScript)) + // NOTE: CMD is intentionally NOT set here. The caller (cmd/root.go) must // append agent Install() steps and the manifest RUN, then call Finalize() // to append the CMD as the very last instruction. This ensures all RUN diff --git a/internal/docker/builder_test.go b/internal/docker/builder_test.go index f5a01de..90371b4 100644 --- a/internal/docker/builder_test.go +++ b/internal/docker/builder_test.go @@ -498,6 +498,69 @@ func TestBuilderCmdAppendsCorrectInstruction(t *testing.T) { require.Contains(t, content, `CMD ["/bin/sh", "-c", "echo hello"]`) } +// --------------------------------------------------------------------------- +// Property 52: Headless keyring packages are always installed in the base layer +// --------------------------------------------------------------------------- + +// Feature: bootstrap-ai-coding, Property 52: Headless keyring packages are always installed in the base layer +// Validates: CC-7 +func TestPropertyKeyringPackagesInstalled(t *testing.T) { + rapid.Check(t, func(t *rapid.T) { + uid := rapid.IntRange(1000, 65000).Draw(t, "uid") + gid := rapid.IntRange(1000, 65000).Draw(t, "gid") + + b := newCreateBuilder(uid, gid) + content := b.Build() + + require.Contains(t, content, "dbus-x11", + "Dockerfile must install dbus-x11 for dbus-launch") + require.Contains(t, content, "gnome-keyring", + "Dockerfile must install gnome-keyring for Secret Service") + require.Contains(t, content, "libsecret-1-0", + "Dockerfile must install libsecret-1-0 for client access") + }) +} + +// --------------------------------------------------------------------------- +// Property 53: Keyring profile script is always created at constants.KeyringProfileScript +// --------------------------------------------------------------------------- + +// Feature: bootstrap-ai-coding, Property 53: Keyring profile script is always created at constants.KeyringProfileScript +// Validates: CC-7 +func TestPropertyKeyringProfileScriptCreated(t *testing.T) { + rapid.Check(t, func(t *rapid.T) { + uid := rapid.IntRange(1000, 65000).Draw(t, "uid") + gid := rapid.IntRange(1000, 65000).Draw(t, "gid") + + b := newCreateBuilder(uid, gid) + content := b.Build() + + require.Contains(t, content, constants.KeyringProfileScript, + "Dockerfile must reference KeyringProfileScript path %q", constants.KeyringProfileScript) + require.Contains(t, content, "dbus-launch", + "Keyring script must start dbus-launch") + require.Contains(t, content, "gnome-keyring-daemon", + "Keyring script must start gnome-keyring-daemon") + require.Contains(t, content, "--unlock", + "Keyring script must unlock the keyring") + require.Contains(t, content, "chmod +x", + "Keyring script must be made executable") + }) +} + +// TestKeyringProfileScriptPresentInRenameStrategy verifies that the keyring +// setup is also present when using UserStrategyRename. +// Validates: CC-7 +func TestKeyringProfileScriptPresentInRenameStrategy(t *testing.T) { + b := newRenameBuilder(1000, 1000, "ubuntu") + content := b.Build() + + require.Contains(t, content, "gnome-keyring", + "Rename strategy Dockerfile must also install gnome-keyring") + require.Contains(t, content, constants.KeyringProfileScript, + "Rename strategy Dockerfile must also create keyring profile script") +} + // --------------------------------------------------------------------------- // Node.js deduplication tracking tests // --------------------------------------------------------------------------- diff --git a/internal/docker/integration_test.go b/internal/docker/integration_test.go index f838148..3a513c5 100644 --- a/internal/docker/integration_test.go +++ b/internal/docker/integration_test.go @@ -25,17 +25,22 @@ import ( ) // ---------------------------------------------------------------------------- -// TestMain — integration suite precondition check +// Package-level shared image state — built once in TestMain, reused by tests. // ---------------------------------------------------------------------------- +var ( + sharedImageTag string + sharedClient *docker.Client + sharedProjectDir string + sharedHostUID int + sharedHostGID int +) + // TestMain ensures the base image is removed from the local Docker image store -// before the integration suite runs. The suite includes -// TestAFindConflictingUserPullsImageIfAbsent, which specifically tests the -// pull-before-inspect path. Removing the image guarantees a clean slate so -// that test always exercises the auto-pull logic. +// before the integration suite runs (for TestAFindConflictingUserPullsImageIfAbsent), +// then builds a shared image that most tests reuse. func TestMain(m *testing.M) { if _, err := exec.LookPath("docker"); err != nil { - // Docker not available — individual tests will skip themselves. os.Exit(m.Run()) } @@ -49,53 +54,40 @@ func TestMain(m *testing.M) { os.Exit(m.Run()) } -// ---------------------------------------------------------------------------- -// Helper: setupContainer -// ---------------------------------------------------------------------------- - -// setupContainer creates a temp project dir, builds a container image, starts -// the container, waits for SSH to be ready, and returns the container name, -// SSH port, and a cleanup function. -// -// The caller is responsible for registering the cleanup via t.Cleanup or defer. -func setupContainer(t *testing.T) (containerName string, sshPort int, cleanup func()) { +// buildSharedImage builds the shared image once (idempotent). Tests that need +// a container call this, then create their own container from the shared image. +// The image is built on first call; subsequent calls are no-ops. +func buildSharedImage(t *testing.T) { t.Helper() - if _, err := exec.LookPath("docker"); err != nil { - t.Skip("docker not available") + if sharedImageTag != "" { + return // already built } ctx := context.Background() - // 1. Create a temp project dir. - projectDir := t.TempDir() - dirName := filepath.Base(projectDir) + var err error + sharedProjectDir = t.TempDir() - // 2. Generate SSH host key pair. hostKeyPriv, hostKeyPub, err := sshpkg.GenerateHostKeyPair() require.NoError(t, err, "generating host key pair") - // 3. Discover or generate a public key for the container user. - // Use a freshly generated key so tests are hermetic. _, userPubKey, err := sshpkg.GenerateHostKeyPair() require.NoError(t, err, "generating user key pair") - // 4. Determine host UID/GID. u, err := user.Current() require.NoError(t, err, "getting current user") - uid, err := strconv.Atoi(u.Uid) + sharedHostUID, err = strconv.Atoi(u.Uid) require.NoError(t, err, "parsing UID") - gid, err := strconv.Atoi(u.Gid) + sharedHostGID, err = strconv.Atoi(u.Gid) require.NoError(t, err, "parsing GID") - // 5. Build a DockerfileBuilder with the host UID/GID. - // Check for UID/GID conflicts in the base image first (mirrors runStart logic). - client, err := docker.NewClient() + sharedClient, err = docker.NewClient() require.NoError(t, err, "connecting to Docker daemon") strategy := docker.UserStrategyCreate conflictingUser := "" - conflictingImageUser, err := docker.FindConflictingUser(ctx, client, uid, gid) + conflictingImageUser, err := docker.FindConflictingUser(ctx, sharedClient, sharedHostUID, sharedHostGID) require.NoError(t, err, "checking base image for UID/GID conflicts") if conflictingImageUser != nil { strategy = docker.UserStrategyRename @@ -103,73 +95,77 @@ func setupContainer(t *testing.T) (containerName string, sshPort int, cleanup fu } builder := docker.NewDockerfileBuilder( - uid, gid, + sharedHostUID, sharedHostGID, userPubKey, hostKeyPriv, hostKeyPub, strategy, conflictingUser, ) + builder.Finalize() + + sharedImageTag = constants.ContainerNamePrefix + "integration-shared:latest" + + spec := docker.ContainerSpec{ + Name: constants.ContainerNamePrefix + "integration-shared", + ImageTag: sharedImageTag, + Dockerfile: builder.Build(), + Mounts: []docker.Mount{ + {HostPath: sharedProjectDir, ContainerPath: constants.WorkspaceMountPath}, + }, + Labels: map[string]string{"bac.managed": "true"}, + HostUID: sharedHostUID, + HostGID: sharedHostGID, + } + + _, err = docker.BuildImage(ctx, sharedClient, spec, false) + require.NoError(t, err, "building shared container image") +} + +// startContainerFromSharedImage creates and starts a new container from the +// shared image with a unique name and port. Returns the container name, port, +// client, and cleanup function. +func startContainerFromSharedImage(t *testing.T) (containerName string, sshPort int, client *docker.Client, cleanup func()) { + t.Helper() + + buildSharedImage(t) + + ctx := context.Background() + + projectDir := t.TempDir() + dirName := filepath.Base(projectDir) - // 6. Pick a free port. port, err := findFreePort() require.NoError(t, err, "finding free port") - // Derive a deterministic container name from the temp dir name. containerName = constants.ContainerNamePrefix + sanitize(dirName) - imageTag := containerName + ":latest" - - // CMD must be the last instruction — call Finalize() before Build(). - builder.Finalize() spec := docker.ContainerSpec{ - Name: containerName, - ImageTag: imageTag, - Dockerfile: builder.Build(), + Name: containerName, + ImageTag: sharedImageTag, Mounts: []docker.Mount{ - { - HostPath: projectDir, - ContainerPath: constants.WorkspaceMountPath, - ReadOnly: false, - }, + {HostPath: projectDir, ContainerPath: constants.WorkspaceMountPath}, }, SSHPort: port, - Labels: map[string]string{ - "bac.managed": "true", - }, - HostUID: uid, - HostGID: gid, + Labels: map[string]string{"bac.managed": "true"}, + HostUID: sharedHostUID, + HostGID: sharedHostGID, } - // 7. Build the image. - _, err = docker.BuildImage(ctx, client, spec, false) - require.NoError(t, err, "building container image") - - // 9. Create and start the container. - _, err = docker.CreateContainer(ctx, client, spec) + _, err = docker.CreateContainer(ctx, sharedClient, spec) require.NoError(t, err, "creating container") - err = docker.StartContainer(ctx, client, containerName) + err = docker.StartContainer(ctx, sharedClient, containerName) require.NoError(t, err, "starting container") - // 10. Wait for SSH to be ready. err = docker.WaitForSSH(ctx, "127.0.0.1", port, 60*time.Second) require.NoError(t, err, "waiting for SSH to be ready") cleanup = func() { cleanCtx := context.Background() - _ = docker.StopContainer(cleanCtx, client, containerName) - _ = docker.RemoveContainer(cleanCtx, client, containerName) - // Remove the image. - images, _ := docker.ListBACImages(cleanCtx, client) - for _, img := range images { - for _, tag := range img.RepoTags { - if tag == imageTag { - _, _ = client.ImageRemove(cleanCtx, img.ID, forceRemoveOpts()) - } - } - } + _ = docker.StopContainer(cleanCtx, sharedClient, containerName) + _ = docker.RemoveContainer(cleanCtx, sharedClient, containerName) } - return containerName, port, cleanup + return containerName, port, sharedClient, cleanup } // ---------------------------------------------------------------------------- @@ -182,10 +178,9 @@ func TestContainerStartsAndSSHConnects(t *testing.T) { t.Skip("docker not available") } - _, sshPort, cleanup := setupContainer(t) + _, sshPort, _, cleanup := startContainerFromSharedImage(t) t.Cleanup(cleanup) - // Assert TCP connection to SSH port succeeds. addr := fmt.Sprintf("127.0.0.1:%d", sshPort) conn, err := net.DialTimeout("tcp", addr, 5*time.Second) require.NoError(t, err, "expected TCP connection to SSH port %d to succeed", sshPort) @@ -202,31 +197,18 @@ func TestWorkspaceMountLiveSync(t *testing.T) { t.Skip("docker not available") } - containerName, _, cleanup := setupContainer(t) + containerName, _, client, cleanup := startContainerFromSharedImage(t) t.Cleanup(cleanup) - // Write a file to the host project dir (the mount source). - // We need the project dir path — re-derive it from the container name by - // writing a sentinel file to a known temp dir. - // Instead, we write a file via docker exec to /workspace and check it on host. - // Actually, the test writes to the host dir and checks inside the container. - // We need the host project dir. Since setupContainer uses t.TempDir(), we - // can't easily get it back. Instead, we write a file inside the container - // and verify it appears on the host via docker exec in reverse. - // - // The simplest approach: exec a command inside the container to create a file - // at /workspace/sync-test.txt, then exec another command to verify it exists. ctx := context.Background() - // Create a file inside the container at /workspace. - exitCode, err := docker.ExecInContainer(ctx, containerName, []string{ + exitCode, err := docker.ExecInContainer(ctx, client, containerName, []string{ "bash", "-c", "echo 'hello from container' > /workspace/sync-test.txt", }) require.NoError(t, err, "exec to create file in /workspace") require.Equal(t, 0, exitCode, "expected exit 0 when creating file in /workspace") - // Verify the file exists inside the container at the workspace mount path. - exitCode, err = docker.ExecInContainer(ctx, containerName, []string{ + exitCode, err = docker.ExecInContainer(ctx, client, containerName, []string{ "test", "-f", constants.WorkspaceMountPath + "/sync-test.txt", }) require.NoError(t, err, "exec to verify file in /workspace") @@ -243,7 +225,7 @@ func TestFileOwnershipMatchesHostUser(t *testing.T) { t.Skip("docker not available") } - containerName, _, cleanup := setupContainer(t) + containerName, _, client, cleanup := startContainerFromSharedImage(t) t.Cleanup(cleanup) ctx := context.Background() @@ -251,25 +233,20 @@ func TestFileOwnershipMatchesHostUser(t *testing.T) { u, err := user.Current() require.NoError(t, err) - // Create a file inside the container at /workspace/ as the container user. - // Use `su` to run as the container user so the file is owned by UID/GID 1000, - // not root (docker exec runs as root by default). - exitCode, err := docker.ExecInContainer(ctx, containerName, []string{ + exitCode, err := docker.ExecInContainer(ctx, client, containerName, []string{ "su", "-c", "touch /workspace/ownership-test.txt", constants.ContainerUser, }) require.NoError(t, err) require.Equal(t, 0, exitCode, "expected exit 0 when creating file") - // Check UID and GID in two separate execs to keep the shell commands simple - // and avoid quoting issues. Each command exits 0 iff the value matches. checkUID := fmt.Sprintf(`[ "$(stat -c '%%u' /workspace/ownership-test.txt)" = "%s" ]`, u.Uid) - exitCode, err = docker.ExecInContainer(ctx, containerName, []string{"bash", "-c", checkUID}) + exitCode, err = docker.ExecInContainer(ctx, client, containerName, []string{"bash", "-c", checkUID}) require.NoError(t, err, "exec to check file UID") require.Equal(t, 0, exitCode, "expected file UID inside container to match host user UID=%s", u.Uid) checkGID := fmt.Sprintf(`[ "$(stat -c '%%g' /workspace/ownership-test.txt)" = "%s" ]`, u.Gid) - exitCode, err = docker.ExecInContainer(ctx, containerName, []string{"bash", "-c", checkGID}) + exitCode, err = docker.ExecInContainer(ctx, client, containerName, []string{"bash", "-c", checkGID}) require.NoError(t, err, "exec to check file GID") require.Equal(t, 0, exitCode, "expected file GID inside container to match host user GID=%s", u.Gid) @@ -285,35 +262,27 @@ func TestCredentialVolumePersistedAcrossRestart(t *testing.T) { t.Skip("docker not available") } - containerName, sshPort, cleanup := setupContainer(t) + containerName, sshPort, client, cleanup := startContainerFromSharedImage(t) t.Cleanup(cleanup) ctx := context.Background() - // Write a sentinel file to /workspace (the bind-mounted volume) inside the container. - exitCode, err := docker.ExecInContainer(ctx, containerName, []string{ + exitCode, err := docker.ExecInContainer(ctx, client, containerName, []string{ "bash", "-c", "echo 'persistent' > /workspace/persist-test.txt", }) require.NoError(t, err) require.Equal(t, 0, exitCode, "expected exit 0 when writing sentinel file") - // Stop the container. - client, err := docker.NewClient() - require.NoError(t, err) - err = docker.StopContainer(ctx, client, containerName) require.NoError(t, err, "stopping container") - // Restart the container. err = docker.StartContainer(ctx, client, containerName) require.NoError(t, err, "restarting container") - // Wait for SSH to be ready again. err = docker.WaitForSSH(ctx, "127.0.0.1", sshPort, 30*time.Second) require.NoError(t, err, "waiting for SSH after restart") - // Assert the file is still present. - exitCode, err = docker.ExecInContainer(ctx, containerName, []string{ + exitCode, err = docker.ExecInContainer(ctx, client, containerName, []string{ "test", "-f", "/workspace/persist-test.txt", }) require.NoError(t, err) @@ -330,29 +299,22 @@ func TestSSHPortPersistenceAcrossRestarts(t *testing.T) { t.Skip("docker not available") } - containerName, sshPort, cleanup := setupContainer(t) + containerName, sshPort, client, cleanup := startContainerFromSharedImage(t) t.Cleanup(cleanup) ctx := context.Background() - client, err := docker.NewClient() - require.NoError(t, err) - // Record the SSH port before restart. originalPort := sshPort - // Stop the container. - err = docker.StopContainer(ctx, client, containerName) + err := docker.StopContainer(ctx, client, containerName) require.NoError(t, err, "stopping container") - // Restart the container. err = docker.StartContainer(ctx, client, containerName) require.NoError(t, err, "restarting container") - // Wait for SSH to be ready again. err = docker.WaitForSSH(ctx, "127.0.0.1", originalPort, 30*time.Second) require.NoError(t, err, "waiting for SSH after restart on original port %d", originalPort) - // Assert the same port is still reachable (port binding is preserved). addr := fmt.Sprintf("127.0.0.1:%d", originalPort) conn, err := net.DialTimeout("tcp", addr, 5*time.Second) require.NoError(t, err, "expected SSH port %d to be reachable after restart", originalPort) @@ -371,7 +333,6 @@ func TestSSHHostKeyStableAcrossRebuild(t *testing.T) { ctx := context.Background() - // Generate a host key pair that we will reuse across both builds. hostKeyPriv, hostKeyPub, err := sshpkg.GenerateHostKeyPair() require.NoError(t, err) @@ -393,10 +354,11 @@ func TestSSHHostKeyStableAcrossRebuild(t *testing.T) { port, err := findFreePort() require.NoError(t, err) + client, err := docker.NewClient() + require.NoError(t, err) + buildAndGetFingerprint := func() string { t.Helper() - client, err := docker.NewClient() - require.NoError(t, err) strategy := docker.UserStrategyCreate conflictingUser := "" @@ -433,27 +395,19 @@ func TestSSHHostKeyStableAcrossRebuild(t *testing.T) { return hostKeyPub } - // Build once and record the host key fingerprint. fingerprint1 := buildAndGetFingerprint() - - // Rebuild (simulating --rebuild: same key pair, new image build). fingerprint2 := buildAndGetFingerprint() - // The host key fingerprint must be identical across rebuilds. require.Equal(t, fingerprint1, fingerprint2, "SSH host key fingerprint must be stable across rebuilds") - // Cleanup. t.Cleanup(func() { cleanCtx := context.Background() - client, _ := docker.NewClient() - if client != nil { - images, _ := docker.ListBACImages(cleanCtx, client) - for _, img := range images { - for _, tag := range img.RepoTags { - if tag == imageTag { - _, _ = client.ImageRemove(cleanCtx, img.ID, forceRemoveOpts()) - } + images, _ := docker.ListBACImages(cleanCtx, client) + for _, img := range images { + for _, tag := range img.RepoTags { + if tag == imageTag { + _, _ = client.ImageRemove(cleanCtx, img.ID, forceRemoveOpts()) } } } @@ -472,51 +426,28 @@ func TestPurgeRemovesContainersAndImages(t *testing.T) { ctx := context.Background() - containerName, _, _ := setupContainer(t) + containerName, _, _, _ := startContainerFromSharedImage(t) // Note: we do NOT register the cleanup here because we are testing purge. client, err := docker.NewClient() require.NoError(t, err) - imageTag := containerName + ":latest" - // Verify the container exists before purge. info, err := docker.InspectContainer(ctx, client, containerName) require.NoError(t, err) require.NotNil(t, info, "container should exist before purge") - // Run purge logic: stop + remove container, remove image. + // Run purge logic: stop + remove container. err = docker.StopContainer(ctx, client, containerName) require.NoError(t, err, "stopping container during purge") err = docker.RemoveContainer(ctx, client, containerName) require.NoError(t, err, "removing container during purge") - // Remove the image. - images, err := docker.ListBACImages(ctx, client) - require.NoError(t, err) - for _, img := range images { - for _, tag := range img.RepoTags { - if tag == imageTag { - _, err = client.ImageRemove(ctx, img.ID, forceRemoveOpts()) - require.NoError(t, err, "removing image during purge") - } - } - } - // Assert container is gone. info, err = docker.InspectContainer(ctx, client, containerName) require.NoError(t, err) require.Nil(t, info, "container should be gone after purge") - - // Assert image is gone. - images, err = docker.ListBACImages(ctx, client) - require.NoError(t, err) - for _, img := range images { - for _, tag := range img.RepoTags { - require.NotEqual(t, imageTag, tag, "image should be gone after purge") - } - } } // ---------------------------------------------------------------------------- @@ -529,21 +460,17 @@ func TestKnownHostsEntriesLifecycle(t *testing.T) { t.Skip("docker not available") } - // Use a temp HOME to avoid touching the real ~/.ssh/known_hosts. tempHome := t.TempDir() t.Setenv("HOME", tempHome) - _, sshPort, cleanup := setupContainer(t) + _, sshPort, _, cleanup := startContainerFromSharedImage(t) - // Generate a host public key to use as the known_hosts entry value. _, hostPubKey, err := sshpkg.GenerateHostKeyPair() require.NoError(t, err) - // Add known_hosts entries after container starts. err = sshpkg.SyncKnownHosts(sshPort, hostPubKey, false) require.NoError(t, err, "SyncKnownHosts should succeed") - // Assert both entries are present. khPath := filepath.Join(tempHome, ".ssh", "known_hosts") data, err := os.ReadFile(khPath) require.NoError(t, err, "known_hosts file should exist") @@ -556,14 +483,11 @@ func TestKnownHostsEntriesLifecycle(t *testing.T) { require.True(t, strings.Contains(content, loopbackEntry), "known_hosts should contain 127.0.0.1:%d entry", sshPort) - // Stop and remove the container. cleanup() - // Remove known_hosts entries. err = sshpkg.RemoveKnownHostsEntries(sshPort) require.NoError(t, err, "RemoveKnownHostsEntries should succeed") - // Assert both entries are gone. data, err = os.ReadFile(khPath) require.NoError(t, err, "known_hosts file should still exist after removal") content = string(data) @@ -584,17 +508,14 @@ func TestSSHConfigEntryLifecycle(t *testing.T) { t.Skip("docker not available") } - // Use a temp HOME to avoid touching the real ~/.ssh/config. tempHome := t.TempDir() t.Setenv("HOME", tempHome) - containerName, sshPort, cleanup := setupContainer(t) + containerName, sshPort, _, cleanup := startContainerFromSharedImage(t) - // Add SSH config entry after container starts. err := sshpkg.SyncSSHConfig(containerName, sshPort, false) require.NoError(t, err, "SyncSSHConfig should succeed") - // Assert the Host stanza is present with correct fields. cfgPath := filepath.Join(tempHome, ".ssh", "config") data, err := os.ReadFile(cfgPath) require.NoError(t, err, "ssh config file should exist") @@ -614,14 +535,11 @@ func TestSSHConfigEntryLifecycle(t *testing.T) { require.True(t, strings.Contains(content, hostnameLine), "ssh config should contain 'HostName localhost'") - // Stop and remove the container. cleanup() - // Remove the SSH config entry. err = sshpkg.RemoveSSHConfigEntry(containerName) require.NoError(t, err, "RemoveSSHConfigEntry should succeed") - // Assert the stanza is gone. data, err = os.ReadFile(cfgPath) require.NoError(t, err, "ssh config file should still exist after removal") content = string(data) @@ -634,7 +552,6 @@ func TestSSHConfigEntryLifecycle(t *testing.T) { // Internal helpers // ---------------------------------------------------------------------------- -// findFreePort finds a free TCP port starting at constants.SSHPortStart. func findFreePort() (int, error) { for port := constants.SSHPortStart; port < 65535; port++ { ln, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) @@ -646,9 +563,6 @@ func findFreePort() (int, error) { return 0, fmt.Errorf("no free port found starting at %d", constants.SSHPortStart) } -// sanitize lowercases s and replaces characters outside [a-z0-9] with '-', -// collapsing consecutive dashes and trimming leading/trailing dashes. -// Used to derive a container name from a temp dir name. func sanitize(s string) string { s = strings.ToLower(s) var b strings.Builder @@ -660,7 +574,6 @@ func sanitize(s string) string { } } result := b.String() - // Collapse consecutive dashes. for strings.Contains(result, "--") { result = strings.ReplaceAll(result, "--", "-") } @@ -671,7 +584,6 @@ func sanitize(s string) string { return result } -// forceRemoveOpts returns image remove options with Force=true. func forceRemoveOpts() dockerimage.RemoveOptions { return dockerimage.RemoveOptions{Force: true} } @@ -681,11 +593,6 @@ func forceRemoveOpts() dockerimage.RemoveOptions { // Validates: Req 14.7 (Image_Build_Timeout) // ---------------------------------------------------------------------------- -// testBuildTimeout is the deadline used in the timeout integration test. -// It is intentionally much shorter than constants.ImageBuildTimeout so the -// test completes quickly. 3 seconds is enough for the Docker daemon to accept -// the build request and start executing the sleep RUN step before the context -// is cancelled. const testBuildTimeout = 3 * time.Second func TestBuildImageTimeoutEnforced(t *testing.T) { @@ -698,8 +605,6 @@ func TestBuildImageTimeoutEnforced(t *testing.T) { client, err := docker.NewClient() require.NoError(t, err, "connecting to Docker daemon") - // Build a minimal Dockerfile whose single RUN step sleeps far longer than - // testBuildTimeout. The build must be cancelled before it completes. hangingDockerfile := fmt.Sprintf("FROM %s\nRUN sleep 300\n", constants.BaseContainerImage) containerName := constants.ContainerNamePrefix + "timeout-test" @@ -712,7 +617,6 @@ func TestBuildImageTimeoutEnforced(t *testing.T) { Labels: map[string]string{"bac.managed": "true"}, } - // Ensure any partial image is cleaned up regardless of test outcome. t.Cleanup(func() { cleanCtx := context.Background() images, _ := docker.ListBACImages(cleanCtx, client) @@ -765,7 +669,6 @@ func TestAFindConflictingUserPullsImageIfAbsent(t *testing.T) { "FindConflictingUser must succeed even when the base image is not cached locally") _ = result - // Verify the image is now present locally (was pulled by FindConflictingUser). _, _, err = client.ImageInspectWithRaw(ctx, constants.BaseContainerImage) require.NoError(t, err, "base image should be present locally after FindConflictingUser pulls it") diff --git a/internal/docker/runner.go b/internal/docker/runner.go index 4790fbf..fec71eb 100644 --- a/internal/docker/runner.go +++ b/internal/docker/runner.go @@ -40,6 +40,7 @@ type ContainerSpec struct { Labels map[string]string // Docker labels for identification HostUID int // Host user UID (passed as build arg for dev user) HostGID int // Host user GID (passed as build arg for dev user) + NoCache bool // When true, disable Docker layer cache during image build } func buildContextFromDockerfile(dockerfile string) (io.Reader, error) { @@ -79,6 +80,7 @@ func BuildImageWithTimeout(ctx context.Context, c *Client, spec ContainerSpec, t Tags: []string{spec.ImageTag}, Dockerfile: "Dockerfile", Remove: true, + NoCache: spec.NoCache, Labels: spec.Labels, }) if err != nil { @@ -153,7 +155,7 @@ func CreateContainer(ctx context.Context, c *Client, spec ContainerSpec) (string sshPort := nat.Port(fmt.Sprintf("%d/tcp", constants.ContainerSSHPort)) portBindings := nat.PortMap{ sshPort: []nat.PortBinding{ - {HostIP: constants.KnownHostsPatterns[1], HostPort: fmt.Sprintf("%d", spec.SSHPort)}, + {HostIP: constants.HostBindIP, HostPort: fmt.Sprintf("%d", spec.SSHPort)}, }, } exposedPorts := nat.PortSet{sshPort: struct{}{}} @@ -281,7 +283,7 @@ func ListBACContainerNames(ctx context.Context, c *Client) ([]string, error) { return names, nil } -// ListBACImages returns all images managed by this tool. +// ListBACImages returns images with the "bac.managed=true" label only. func ListBACImages(ctx context.Context, c *Client) ([]image.Summary, error) { images, err := c.ImageList(ctx, image.ListOptions{ Filters: bacLabelFilter(), @@ -289,6 +291,17 @@ func ListBACImages(ctx context.Context, c *Client) ([]image.Summary, error) { if err != nil { return nil, fmt.Errorf("listing bac images: %w", err) } + return images, nil +} + +// ListBACImagesWithFallback returns labeled images, falling back to a tag-prefix +// scan for images built before labels were introduced (pre-label compatibility). +// This fallback can be removed once all users have rebuilt their images with --rebuild. +func ListBACImagesWithFallback(ctx context.Context, c *Client) ([]image.Summary, error) { + images, err := ListBACImages(ctx, c) + if err != nil { + return nil, err + } if len(images) == 0 { all, err := c.ImageList(ctx, image.ListOptions{}) if err != nil { @@ -307,12 +320,7 @@ func ListBACImages(ctx context.Context, c *Client) ([]image.Summary, error) { } // ExecInContainer runs a command inside a running container and returns the exit code. -func ExecInContainer(ctx context.Context, containerID string, cmd []string) (int, error) { - c, err := NewClient() - if err != nil { - return -1, fmt.Errorf("connecting to Docker for exec: %w", err) - } - +func ExecInContainer(ctx context.Context, c *Client, containerID string, cmd []string) (int, error) { execID, err := c.ContainerExecCreate(ctx, containerID, container.ExecOptions{ Cmd: cmd, AttachStdout: true, diff --git a/internal/naming/naming.go b/internal/naming/naming.go index 0a21a79..db58a5f 100644 --- a/internal/naming/naming.go +++ b/internal/naming/naming.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/koudis/bootstrap-ai-coding/internal/constants" + "github.com/koudis/bootstrap-ai-coding/internal/pathutil" ) // consecutiveDashes matches two or more consecutive dashes. @@ -140,7 +141,7 @@ func ContainerName(projectPath string, existingNames []string) (string, error) { // Tool_Data_Dir entry under constants.ToolDataDirRoot. These names were // previously assigned by this tool and must not be treated as collisions. func knownDataDirNames() map[string]struct{} { - root := expandHome(constants.ToolDataDirRoot) + root := pathutil.ExpandHome(constants.ToolDataDirRoot) entries, err := os.ReadDir(root) if err != nil { return map[string]struct{}{} @@ -154,11 +155,3 @@ func knownDataDirNames() map[string]struct{} { return m } -// expandHome expands a leading "~/" to the user's home directory. -func expandHome(p string) string { - if len(p) >= 2 && p[:2] == "~/" { - home, _ := os.UserHomeDir() - return filepath.Join(home, p[2:]) - } - return p -} diff --git a/internal/pathutil/pathutil.go b/internal/pathutil/pathutil.go new file mode 100644 index 0000000..d6c7229 --- /dev/null +++ b/internal/pathutil/pathutil.go @@ -0,0 +1,17 @@ +// Package pathutil provides shared path helpers with zero internal dependencies. +package pathutil + +import ( + "os" + "path/filepath" +) + +// ExpandHome expands a leading "~/" to the user's home directory. +// If the path does not start with "~/", it is returned unchanged. +func ExpandHome(p string) string { + if len(p) >= 2 && p[:2] == "~/" { + home, _ := os.UserHomeDir() + return filepath.Join(home, p[2:]) + } + return p +} diff --git a/internal/pathutil/pathutil_test.go b/internal/pathutil/pathutil_test.go new file mode 100644 index 0000000..48f5432 --- /dev/null +++ b/internal/pathutil/pathutil_test.go @@ -0,0 +1,59 @@ +package pathutil_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + "pgregory.net/rapid" + + "github.com/koudis/bootstrap-ai-coding/internal/pathutil" +) + +// Feature: bootstrap-ai-coding, Property 26: ExpandHome never returns a path starting with ~/ +func TestPropertyExpandHomeNeverReturnsTilde(t *testing.T) { + rapid.Check(t, func(t *rapid.T) { + input := rapid.String().Draw(t, "input") + result := pathutil.ExpandHome(input) + require.False(t, + len(result) >= 2 && result[:2] == "~/", + "ExpandHome(%q) = %q must not start with ~/", input, result) + }) +} + +func TestExpandHomeNoTilde(t *testing.T) { + require.Equal(t, "/absolute/path", pathutil.ExpandHome("/absolute/path")) +} + +func TestExpandHomeRelativePath(t *testing.T) { + require.Equal(t, "relative/path", pathutil.ExpandHome("relative/path")) +} + +func TestExpandHomeTildeExpanded(t *testing.T) { + result := pathutil.ExpandHome("~/somedir") + require.NotContains(t, result, "~", "ExpandHome must expand ~ to the home directory") + require.Contains(t, result, "somedir") +} + +func TestExpandHomeTildeOnly(t *testing.T) { + // "~" alone (no slash) should not be expanded. + require.Equal(t, "~", pathutil.ExpandHome("~")) +} + +func TestExpandHomeEmptyString(t *testing.T) { + require.Equal(t, "", pathutil.ExpandHome("")) +} + +func TestExpandHomeTildeSlashOnly(t *testing.T) { + home, _ := os.UserHomeDir() + result := pathutil.ExpandHome("~/") + // filepath.Join(home, "") returns home without trailing slash. + require.Equal(t, filepath.Join(home, ""), result) +} + +func TestExpandHomeNestedPath(t *testing.T) { + home, _ := os.UserHomeDir() + result := pathutil.ExpandHome("~/.config/bootstrap-ai-coding") + require.Equal(t, filepath.Join(home, ".config/bootstrap-ai-coding"), result) +} diff --git a/internal/ssh/keys.go b/internal/ssh/keys.go index a61f6b9..f5a7fe4 100644 --- a/internal/ssh/keys.go +++ b/internal/ssh/keys.go @@ -8,11 +8,11 @@ import ( "errors" "fmt" "os" - "path/filepath" gossh "golang.org/x/crypto/ssh" "github.com/koudis/bootstrap-ai-coding/internal/constants" + "github.com/koudis/bootstrap-ai-coding/internal/pathutil" ) // DiscoverPublicKey returns the contents of the first Public_Key found on the Host. @@ -26,7 +26,7 @@ func DiscoverPublicKey(sshKeyFlag string) (string, error) { candidates = append(candidates, constants.PublicKeyDefaultPaths...) for _, p := range candidates { - expanded := expandHome(p) + expanded := pathutil.ExpandHome(p) data, err := os.ReadFile(expanded) if err == nil { return string(data), nil @@ -60,10 +60,3 @@ func GenerateHostKeyPair() (priv, pub string, err error) { return privStr, pubStr, nil } -func expandHome(p string) string { - if len(p) >= 2 && p[:2] == "~/" { - home, _ := os.UserHomeDir() - return filepath.Join(home, p[2:]) - } - return p -} diff --git a/internal/testutil/consent.go b/internal/testutil/consent.go index 2d9dfe6..1ea0e19 100644 --- a/internal/testutil/consent.go +++ b/internal/testutil/consent.go @@ -78,5 +78,7 @@ func isImageNotFound(err error) bool { return false } msg := err.Error() - return strings.Contains(msg, "No such image") || strings.Contains(msg, "not found") + return strings.Contains(msg, "No such image") || + strings.Contains(msg, "not found") || + strings.Contains(msg, "reference does not exist") }