diff --git a/cmd/internal/agentworkspace/logs.go b/cmd/internal/agentworkspace/logs.go index 42bddb892..630db0551 100644 --- a/cmd/internal/agentworkspace/logs.go +++ b/cmd/internal/agentworkspace/logs.go @@ -39,16 +39,6 @@ func NewLogsCmd(flags *flags.GlobalFlags) *cobra.Command { } func (cmd *LogsCmd) Run(ctx context.Context) error { - // `agent workspace logs` returns devcontainer logs from inside the - // workspace container/machine. Reject host invocations explicitly - // to avoid silently reading from the wrong place. - if agent.IsHostAgentInvocation(cmd.AgentDir) { - return fmt.Errorf( - "`devsy internal agent workspace logs` is only valid inside the workspace container or machine", - ) - } - - // get workspace info shouldExit, workspaceInfo, err := agent.ReadAgentWorkspaceInfo( cmd.AgentDir, cmd.Context, @@ -59,8 +49,10 @@ func (cmd *LogsCmd) Run(ctx context.Context) error { } else if shouldExit { return nil } + if workspaceInfo == nil { + return fmt.Errorf("workspace %q not found", cmd.ID) + } - // create new runner runner, err := devcontainer.NewRunner( config.ContainerDevsyHelperLocation, config.DefaultAgentDownloadURL(), @@ -70,6 +62,5 @@ func (cmd *LogsCmd) Run(ctx context.Context) error { return fmt.Errorf("create runner: %w", err) } - // write devcontainer logs to stdout return runner.Logs(ctx, os.Stdout) } diff --git a/cmd/internal/logs_daemon.go b/cmd/internal/logs_daemon.go index 3b5e6eafe..d03ad3efe 100644 --- a/cmd/internal/logs_daemon.go +++ b/cmd/internal/logs_daemon.go @@ -6,7 +6,6 @@ import ( "os" "github.com/devsy-org/devsy/cmd/flags" - "github.com/devsy-org/devsy/pkg/agent" "github.com/devsy-org/devsy/pkg/client" "github.com/devsy-org/devsy/pkg/config" provider2 "github.com/devsy-org/devsy/pkg/provider" @@ -66,8 +65,7 @@ func (cmd *LogsDaemonCmd) Run(ctx context.Context, args []string) error { } command := fmt.Sprintf( - "%s%q internal agent workspace logs-daemon --context %q --id %q", - agent.ContainerAgentEnvPrefix, + "%q internal agent workspace logs-daemon --context %q --id %q", workspaceClient.AgentPath(), workspaceClient.Context(), workspaceClient.Workspace(), diff --git a/cmd/workspace/logs.go b/cmd/workspace/logs.go index 09a2f7462..f871cfede 100644 --- a/cmd/workspace/logs.go +++ b/cmd/workspace/logs.go @@ -124,8 +124,7 @@ func (cmd *LogsCmd) Run(ctx context.Context, args []string) error { defer func() { _ = session.Close() }() agentCommand := fmt.Sprintf( - "%s%q internal agent workspace logs --context %q --id %q", - agent.ContainerAgentEnvPrefix, + "%q internal agent workspace logs --context %q --id %q", client.AgentPath(), client.Context(), client.Workspace(), diff --git a/cmd/workspace/up/agent.go b/cmd/workspace/up/agent.go index e94d157c3..6ed71b150 100644 --- a/cmd/workspace/up/agent.go +++ b/cmd/workspace/up/agent.go @@ -203,8 +203,7 @@ func (cmd *UpCmd) devsyUpMachineSSH( } agentCommand := fmt.Sprintf( - "%s%q internal agent workspace up --workspace-info %q", - agent.ContainerAgentEnvPrefix, + "%q internal agent workspace up --workspace-info %q", client.AgentPath(), workspaceInfo, ) diff --git a/pkg/agent/workspace.go b/pkg/agent/workspace.go index bdb8c569c..bd29b4e36 100644 --- a/pkg/agent/workspace.go +++ b/pkg/agent/workspace.go @@ -35,42 +35,10 @@ var extraSearchLocations = []string{ config.ContainerDataDir + "/agent", } -// EnvAgentInContainer is set to "1" by every SSH command builder that -// launches `devsy agent ...` inside the workspace container or machine. -// It is the single signal used by IsHostAgentInvocation to distinguish a -// container-side invocation from a host-side one. The host never sets it. -const EnvAgentInContainer = "DEVSY_AGENT_IN_CONTAINER" - -// EnvAgentInContainerTrue is the value the env var takes when set. -const EnvAgentInContainerTrue = "1" - -// ContainerAgentEnvPrefix is the inline env-var assignment SSH command -// builders prepend to the shell snippet that launches `devsy agent ...` -// inside the workspace container or machine. POSIX shells parse leading -// `NAME=value cmd ...` as a one-shot environment assignment for `cmd`, -// so this works without requiring the SSH server to honour SetEnv (which -// is often disabled). -const ContainerAgentEnvPrefix = EnvAgentInContainer + "=" + EnvAgentInContainerTrue + " " - -// AgentCommandEnvPrefix omits the container env prefix when the command -// will be executed on the host (provider agent.local=true); prepending it -// there would mislabel the host process and trip IsHostAgentInvocation's -// stale-env warning. -func AgentCommandEnvPrefix(isLocal bool) string { - if isLocal { - return "" - } - return ContainerAgentEnvPrefix -} - var ErrFindAgentHomeFolder = fmt.Errorf("couldn't find devsy home directory") -// GetAgentDaemonLogFolder returns the folder that holds agent-daemon.log. -// The daemon is a container/machine-side process: it is started by the -// inject-and-run path during `devsy up` and the SSH command builder sets -// DEVSY_AGENT_IN_CONTAINER=1. A host-side invocation has no daemon to -// inspect and therefore is rejected explicitly instead of silently -// resolving to the legacy `/agent` glob. +// GetAgentDaemonLogFolder returns the path to the agent daemon log folder, which is only +// available inside the workspace container or machine. func GetAgentDaemonLogFolder(agentFolder string) (string, error) { if IsHostAgentInvocation(agentFolder) { return "", fmt.Errorf( @@ -229,39 +197,14 @@ func GetAgentBinariesDirFromWorkspaceDir(workspaceDir string) (string, error) { return "", os.ErrNotExist } -// IsHostAgentInvocation reports whether this `devsy agent ...` call is -// running on the user's host (as opposed to inside the workspace container -// or machine). The signal is explicit: every SSH command builder that -// launches `devsy agent ...` inside the container sets -// DEVSY_AGENT_IN_CONTAINER=1; the host never sets it. An explicit -// --agent-dir (agentFolder != "") also forces a non-host (legacy/explicit) -// routing, so unit tests and unusual deployments can still pin a folder. -// -// When this returns true, per-workspace agent state lives under the -// canonical PathManager.WorkspaceAgentDir so a single -// os.RemoveAll(WorkspaceDir) wipes it on delete. DEVSY_HOME is NOT -// consulted here — it is purely a host-side config-dir relocation knob -// and must not double as a container marker. -// containerDetector is the package-level seam used by IsHostAgentInvocation -// to detect whether the process is running inside a container. Tests -// override this to make the predicate deterministic across platforms. var containerDetector = isLikelyContainer +// IsHostAgentInvocation determines whether the current invocation is a host-side invocation of the agent. func IsHostAgentInvocation(agentFolder string) bool { if agentFolder != "" { return false } - if os.Getenv(EnvAgentInContainer) != EnvAgentInContainerTrue { - return true - } - if !containerDetector() { - log.Debugf( - "%s=1 is set but no container indicator file found; treating as host invocation", - EnvAgentInContainer, - ) - return true - } - return false + return !containerDetector() } func GetAgentBinariesDir(agentFolder, context, workspaceID string) (string, error) { diff --git a/pkg/agent/workspace_test.go b/pkg/agent/workspace_test.go index 3a5061ab2..039baf80f 100644 --- a/pkg/agent/workspace_test.go +++ b/pkg/agent/workspace_test.go @@ -6,21 +6,6 @@ import ( const explicitAgentDir = "/some/dir" -func TestAgentCommandEnvPrefix(t *testing.T) { - if got := AgentCommandEnvPrefix(true); got != "" { - t.Errorf("AgentCommandEnvPrefix(true) = %q, want \"\"", got) - } - if got := AgentCommandEnvPrefix(false); got != ContainerAgentEnvPrefix { - t.Errorf( - "AgentCommandEnvPrefix(false) = %q, want %q", - got, - ContainerAgentEnvPrefix, - ) - } -} - -// withContainerDetector swaps the package-level container detector for -// the duration of the test, restoring the previous value on cleanup. func withContainerDetector(t *testing.T, fn func() bool) { t.Helper() prev := containerDetector @@ -33,7 +18,6 @@ func withContainerDetector(t *testing.T, fn func() bool) { type hostInvocationCase struct { name string agentFolder string - inContainer string // empty == unset; "1" == container marker containerSeen bool want bool } @@ -41,98 +25,53 @@ type hostInvocationCase struct { func hostInvocationCases() []hostInvocationCase { return []hostInvocationCase{ { - name: "host: no agentFolder, no marker, no indicator", - agentFolder: "", - inContainer: "", - containerSeen: false, - want: true, - }, - { - name: "container: no agentFolder, marker set, indicator present", - agentFolder: "", - inContainer: "1", - containerSeen: true, - want: false, - }, - { - name: "host with stale env: marker set but no indicator", + name: "host: no agentFolder, not in a container", agentFolder: "", - inContainer: "1", containerSeen: false, want: true, }, { - name: "host with rogue indicator but no env", + name: "container: no agentFolder, running in a container", agentFolder: "", - inContainer: "", containerSeen: true, - want: true, - }, - { - name: "explicit agentFolder, no marker (legacy/explicit)", - agentFolder: explicitAgentDir, - inContainer: "", - containerSeen: false, want: false, }, { - name: "explicit agentFolder beats stale env", + name: "explicit agentFolder, not in a container (legacy/explicit)", agentFolder: explicitAgentDir, - inContainer: "1", containerSeen: false, want: false, }, { - name: "explicit agentFolder and marker (container with --agent-dir)", + name: "explicit agentFolder beats container detection", agentFolder: explicitAgentDir, - inContainer: "1", containerSeen: true, want: false, }, } } -// TestIsHostAgentInvocation covers the matrix of -// (agentFolder empty/non-empty) x (env unset/"1") x (container indicator yes/no). func TestIsHostAgentInvocation(t *testing.T) { for _, tc := range hostInvocationCases() { t.Run(tc.name, func(t *testing.T) { - t.Setenv(EnvAgentInContainer, tc.inContainer) withContainerDetector(t, func() bool { return tc.containerSeen }) got := IsHostAgentInvocation(tc.agentFolder) if got != tc.want { t.Fatalf( - "IsHostAgentInvocation(%q) with %s=%q indicator=%v = %v, want %v", - tc.agentFolder, EnvAgentInContainer, tc.inContainer, - tc.containerSeen, got, tc.want, + "IsHostAgentInvocation(%q) with container=%v = %v, want %v", + tc.agentFolder, tc.containerSeen, got, tc.want, ) } }) } } -// TestIsHostAgentInvocation_IgnoresDevsyHome guards the regression that -// setting DEVSY_HOME on the host must NOT flip the predicate to -// "container" — only DEVSY_AGENT_IN_CONTAINER + an indicator does that. func TestIsHostAgentInvocation_IgnoresDevsyHome(t *testing.T) { t.Setenv("DEVSY_HOME", "/custom/devsy/home") - t.Setenv(EnvAgentInContainer, "") withContainerDetector(t, func() bool { return false }) if !IsHostAgentInvocation("") { t.Fatal("IsHostAgentInvocation should still report host when only DEVSY_HOME is set") } } - -// TestIsHostAgentInvocation_NonStandardMarkerValue ensures that only the -// exact "1" string flips the predicate, mirroring the strict equality -// check in the implementation. -func TestIsHostAgentInvocation_NonStandardMarkerValue(t *testing.T) { - t.Setenv(EnvAgentInContainer, "true") - withContainerDetector(t, func() bool { return true }) - - if !IsHostAgentInvocation("") { - t.Fatal("only DEVSY_AGENT_IN_CONTAINER=1 should be honoured; got false for value 'true'") - } -} diff --git a/pkg/client/clientimplementation/workspace_client.go b/pkg/client/clientimplementation/workspace_client.go index 8ae274df9..bd2b49694 100644 --- a/pkg/client/clientimplementation/workspace_client.go +++ b/pkg/client/clientimplementation/workspace_client.go @@ -378,8 +378,7 @@ func (s *workspaceClient) Delete(ctx context.Context, opt client.DeleteOptions) return fmt.Errorf("agent info") } command := fmt.Sprintf( - "%s%q internal agent workspace delete --workspace-info %q", - agent.AgentCommandEnvPrefix(s.agentLocalLocked()), + "%q internal agent workspace delete --workspace-info %q", info.Agent.Path, compressed, ) @@ -487,8 +486,7 @@ func (s *workspaceClient) Stop(ctx context.Context, opt client.StopOptions) erro return fmt.Errorf("agent info") } command := fmt.Sprintf( - "%s%q internal agent workspace stop --workspace-info %q", - agent.AgentCommandEnvPrefix(s.agentLocalLocked()), + "%q internal agent workspace stop --workspace-info %q", info.Agent.Path, compressed, ) @@ -667,8 +665,7 @@ func (s *workspaceClient) getContainerStatus(ctx context.Context) (client.Status return "", fmt.Errorf("get agent info") } command := fmt.Sprintf( - "%s%q internal agent workspace status --workspace-info %q", - agent.AgentCommandEnvPrefix(s.agentLocalLocked()), + "%q internal agent workspace status --workspace-info %q", info.Agent.Path, compressed, ) @@ -1008,8 +1005,7 @@ func buildAgentCommand( agentCommand, workspaceInfo string, ) string { command := fmt.Sprintf( - "%s%q internal agent workspace %s --workspace-info %q", - agent.AgentCommandEnvPrefix(workspaceClient.AgentLocal()), + "%q internal agent workspace %s --workspace-info %q", workspaceClient.AgentPath(), agentCommand, workspaceInfo, diff --git a/pkg/tunnel/container.go b/pkg/tunnel/container.go index 65a0d39d1..5cf6175cf 100644 --- a/pkg/tunnel/container.go +++ b/pkg/tunnel/container.go @@ -146,8 +146,7 @@ func (c *ContainerTunnel) updateConfig(ctx context.Context, sshClient *ssh.Clien // update workspace remotely buf := &bytes.Buffer{} command := fmt.Sprintf( - "%s%q internal agent workspace update-config --workspace-info %q", - agent.ContainerAgentEnvPrefix, + "%q internal agent workspace update-config --workspace-info %q", c.client.AgentPath(), workspaceInfo, ) @@ -243,8 +242,7 @@ func (c *ContainerTunnel) runContainerTunnel(ctx context.Context, opts container defer log.Debugf("Container tunnel exited") command := fmt.Sprintf( - "%s%q internal agent container-tunnel --workspace-info %q", - agent.ContainerAgentEnvPrefix, + "%q internal agent container-tunnel --workspace-info %q", c.client.AgentPath(), opts.workspaceInfo, ) diff --git a/pkg/tunnel/services.go b/pkg/tunnel/services.go index cce6fc8c7..fe149ce9c 100644 --- a/pkg/tunnel/services.go +++ b/pkg/tunnel/services.go @@ -14,7 +14,6 @@ import ( "al.essio.dev/pkg/shellescape" "github.com/devsy-org/api/pkg/devsy" - "github.com/devsy-org/devsy/pkg/agent" "github.com/devsy-org/devsy/pkg/agent/tunnelserver" "github.com/devsy-org/devsy/pkg/config" config2 "github.com/devsy-org/devsy/pkg/devcontainer/config" @@ -150,8 +149,7 @@ func addGitSSHSigningKey( // buildCredentialsCommand builds the credentials server command. func buildCredentialsCommand(ctx context.Context, opts RunServicesOptions) string { command := fmt.Sprintf( - "%s%s internal agent container credentials-server --user %s", - agent.ContainerAgentEnvPrefix, + "%s internal agent container credentials-server --user %s", shellescape.Quote(config.ContainerDevsyHelperLocation), shellescape.Quote(opts.User), )