diff --git a/companions/dap/client.go b/companions/dap/client.go index b8ba6f86d..8c4bda1af 100644 --- a/companions/dap/client.go +++ b/companions/dap/client.go @@ -164,27 +164,69 @@ func (c *companionClient) configurationDone(ctx context.Context) error { func (c *companionClient) Launch(ctx context.Context, program string, args []string, env map[string]string) error { launchArgs := map[string]interface{}{ "program": program, - "args": args, - "env": env, "cwd": "/workspace", "stopOnEntry": false, } - _, err := c.tp.request(ctx, "launch", launchArgs) - if err != nil { - return err + // Nil means no override. JSON null is not a valid argument array or + // environment object for adapters such as debugpy. + if args != nil { + launchArgs["args"] = args } - return c.configurationDone(ctx) + if env != nil { + launchArgs["env"] = env + } + return c.startDebugSession(ctx, "launch", launchArgs) } func (c *companionClient) Attach(ctx context.Context, pid int) error { attachArgs := map[string]interface{}{ "processId": pid, } - _, err := c.tp.request(ctx, "attach", attachArgs) + return c.startDebugSession(ctx, "attach", attachArgs) +} + +// Adapters may defer the launch/attach response until configuration is done. +// Send the start request first, wait for the adapter's initialized event, then +// complete configuration before awaiting the start response. +func (c *companionClient) startDebugSession(ctx context.Context, command string, args map[string]interface{}) error { + ctx, cancel := context.WithCancel(ctx) + defer cancel() + wait, err := c.tp.sendRequest(command, args) if err != nil { return err } - return c.configurationDone(ctx) + result := make(chan error, 1) + go func() { + _, err := wait(ctx) + result <- err + }() + var response <-chan error = result + for { + select { + case err := <-response: + if err != nil { + return err + } + response = nil // Early success still requires initialized/configurationDone. + case <-c.tp.initialized: + if err := c.configurationDone(ctx); err != nil { + return err + } + if response == nil { + return nil + } + select { + case err := <-response: + return err + case <-ctx.Done(): + return ctx.Err() + } + case <-c.tp.done: + return fmt.Errorf("%s: transport closed before configuration completed", command) + case <-ctx.Done(): + return ctx.Err() + } + } } func (c *companionClient) SetBreakpoints(ctx context.Context, file string, lines []int) ([]BreakpointResult, error) { diff --git a/companions/dap/client_test.go b/companions/dap/client_test.go new file mode 100644 index 000000000..3dc8ecd3a --- /dev/null +++ b/companions/dap/client_test.go @@ -0,0 +1,134 @@ +package dap + +import ( + "bufio" + "context" + "encoding/json" + "fmt" + "io" + "net" + "strconv" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// These wire exchanges exercise the maintained client and transport without +// starting an adapter, a companion, or a host service. The real debugpy test +// remains separately required under python_companion_required. +func TestStartDebugSessionProtocol(t *testing.T) { + for _, command := range []string{"launch", "attach"} { + for _, behavior := range []string{"deferred", "early", "rejected", "configuration-error", "disconnect", "cancel"} { + t.Run(command+"/"+behavior, func(t *testing.T) { + var args []string + var env map[string]string + if behavior == "early" { + args = []string{"--example", "value"} + env = map[string]string{"EXAMPLE": "value"} + } + ctx, cancel := context.WithTimeout(t.Context(), 3*time.Second) + defer cancel() + conn, peer := net.Pipe() + t.Cleanup(func() { _ = conn.Close(); _ = peer.Close() }) + client := &companionClient{tp: newTransport(ctx, conn)} + peerDone := make(chan struct{}) + go func() { + defer close(peerDone) + reader := bufio.NewReader(peer) + start := readProtocolRequest(t, reader) + require.Equal(t, command, start.Command) + if command == "launch" { + wire := start.Arguments.(map[string]interface{}) + t.Logf("launch wire arguments: %v", wire) + if args == nil { + require.NotContains(t, wire, "args") + require.NotContains(t, wire, "env") + } else { + require.Equal(t, []interface{}{"--example", "value"}, wire["args"]) + require.Equal(t, map[string]interface{}{"EXAMPLE": "value"}, wire["env"]) + } + } + if behavior == "cancel" { + cancel() + return + } + if behavior == "disconnect" { + _ = peer.Close() + return + } + if behavior == "rejected" { + writeProtocolMessage(t, peer, dapResponse{Type: "response", RequestSeq: start.Seq, Success: false, Message: "start refused"}) + return + } + if behavior == "early" { + writeProtocolMessage(t, peer, dapResponse{Type: "response", RequestSeq: start.Seq, Success: true}) + } + writeProtocolMessage(t, peer, dapEvent{Type: "event", Event: "initialized"}) + configuration := readProtocolRequest(t, reader) + require.Equal(t, "configurationDone", configuration.Command) + writeProtocolMessage(t, peer, dapResponse{Type: "response", RequestSeq: configuration.Seq, + Success: behavior != "configuration-error", Message: "configuration refused"}) + if behavior != "early" && behavior != "configuration-error" { + writeProtocolMessage(t, peer, dapResponse{Type: "response", RequestSeq: start.Seq, Success: true}) + } + }() + var err error + if command == "launch" { + err = client.Launch(ctx, "/workspace/main.py", args, env) + } else { + err = client.Attach(ctx, 42) + } + switch behavior { + case "deferred", "early": + require.NoError(t, err) + case "rejected": + require.ErrorContains(t, err, "start refused") + case "configuration-error": + require.ErrorContains(t, err, "configuration refused") + default: + require.Error(t, err) + } + <-peerDone + }) + } + } +} + +func TestConnectionProbePreservesStartupEvent(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), time.Second) + defer cancel() + conn, peer := net.Pipe() + t.Cleanup(func() { _ = conn.Close(); _ = peer.Close() }) + go writeProtocolMessage(t, peer, dapEvent{Type: "event", Event: "initialized"}) + ready, err := probeConnection(conn) + require.NoError(t, err) + transport := newTransport(ctx, ready) + select { + case <-transport.initialized: + case <-ctx.Done(): + t.Fatal("connection probe discarded the first byte of the startup DAP event") + } +} + +func readProtocolRequest(t *testing.T, reader *bufio.Reader) dapRequest { + t.Helper() + headers, err := (&transport{reader: reader}).readHeader() + require.NoError(t, err) + length, err := strconv.Atoi(headers["Content-Length"]) + require.NoError(t, err) + body := make([]byte, length) + _, err = io.ReadFull(reader, body) + require.NoError(t, err) + var request dapRequest + require.NoError(t, json.Unmarshal(body, &request)) + return request +} + +func writeProtocolMessage(t *testing.T, writer io.Writer, message interface{}) { + t.Helper() + body, err := json.Marshal(message) + require.NoError(t, err) + _, err = fmt.Fprintf(writer, "Content-Length: %d\r\n\r\n%s", len(body), body) + require.NoError(t, err) +} diff --git a/companions/dap/python_integration_test.go b/companions/dap/python_integration_test.go new file mode 100644 index 000000000..f31fc4bef --- /dev/null +++ b/companions/dap/python_integration_test.go @@ -0,0 +1,56 @@ +//go:build python_companion_required + +package dap + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/codefly-dev/core/companions/python" + "github.com/codefly-dev/core/companions/testutil" + "github.com/codefly-dev/core/languages" + "github.com/codefly-dev/core/runners/companion" + "github.com/stretchr/testify/require" +) + +func TestPythonCompanionInitializesAndLaunchesWithoutPip(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), time.Minute) + defer cancel() + testutil.RequireDocker(t, ctx) + img, err := python.CompanionImage(ctx) + require.NoError(t, err) + out, err := exec.CommandContext(ctx, "docker", "image", "inspect", img.FullName()).CombinedOutput() + require.NoError(t, err, "build the selected companion with codefly companion build python: %s", out) + source := t.TempDir() + program, err := os.ReadFile("testdata/python/main.py") + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(source, "main.py"), program, 0o644)) + client, err := NewClient(ctx, languages.PYTHON, source) + require.NoError(t, err) + t.Cleanup(func() { + cleanup, stop := context.WithTimeout(context.Background(), 15*time.Second) + defer stop() + require.NoError(t, client.Close(cleanup)) + }) + require.Equal(t, companion.BackendDocker, client.(*companionClient).runner.Backend()) + terminated := make(chan struct{}, 1) + client.OnTerminated(func() { + select { + case terminated <- struct{}{}: + default: + } + }) + require.NoError(t, client.Launch(ctx, "/workspace/main.py", nil, nil)) + select { + case <-terminated: + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + result, err := os.ReadFile(filepath.Join(source, "result.txt")) + require.NoError(t, err) + require.Equal(t, "debugpy without pip", string(result)) +} diff --git a/companions/dap/testdata/python/main.py b/companions/dap/testdata/python/main.py new file mode 100644 index 000000000..d2567b95e --- /dev/null +++ b/companions/dap/testdata/python/main.py @@ -0,0 +1,8 @@ +import importlib.util +from pathlib import Path + +import debugpy + +assert importlib.util.find_spec("pip") is None +assert importlib.util.find_spec("ensurepip") is None +Path(__file__).with_name("result.txt").write_text("debugpy without pip") diff --git a/companions/dap/transport.go b/companions/dap/transport.go index 4d75b780f..988c00361 100644 --- a/companions/dap/transport.go +++ b/companions/dap/transport.go @@ -2,6 +2,7 @@ package dap import ( "bufio" + "bytes" "context" "encoding/json" "fmt" @@ -58,10 +59,13 @@ type transport struct { reader *bufio.Reader ctx context.Context - mu sync.Mutex - seq int64 - pending map[int]chan *dapResponse - readerErr error // set when the reader goroutine exits with an error + mu sync.Mutex + seq int64 + pending map[int]chan *dapResponse + readerErr error // set when the reader goroutine exits with an error + initialized chan struct{} + initializedOnce sync.Once + done chan struct{} // Event handlers (protected by eventMu). eventMu sync.RWMutex @@ -78,10 +82,12 @@ type transport struct { // causes readMessages to return. This is independent of an explicit close(). func newTransport(ctx context.Context, conn net.Conn) *transport { t := &transport{ - conn: conn, - reader: bufio.NewReader(conn), - ctx: ctx, - pending: make(map[int]chan *dapResponse), + conn: conn, + reader: bufio.NewReader(conn), + ctx: ctx, + pending: make(map[int]chan *dapResponse), + initialized: make(chan struct{}), + done: make(chan struct{}), } go t.readMessages() return t @@ -89,10 +95,26 @@ func newTransport(ctx context.Context, conn net.Conn) *transport { // request sends a DAP request and waits for the matching response. func (t *transport) request(ctx context.Context, command string, arguments interface{}) (*dapResponse, error) { + wait, err := t.sendRequest(command, arguments) + if err != nil { + return nil, err + } + return wait(ctx) +} + +// sendRequest writes the request before returning its response waiter. Launch +// and attach must be sent before configurationDone, but their responses may +// remain pending until configurationDone has been acknowledged. +func (t *transport) sendRequest(command string, arguments interface{}) (func(context.Context) (*dapResponse, error), error) { seq := int(atomic.AddInt64(&t.seq, 1)) ch := make(chan *dapResponse, 1) t.mu.Lock() + if t.readerErr != nil { + err := t.readerErr + t.mu.Unlock() + return nil, fmt.Errorf("%s: transport reader error: %w", command, err) + } t.pending[seq] = ch t.mu.Unlock() @@ -110,25 +132,32 @@ func (t *transport) request(ctx context.Context, command string, arguments inter return nil, err } - select { - case resp, ok := <-ch: - if !ok { - // Channel closed by reader goroutine due to error. + return func(ctx context.Context) (*dapResponse, error) { + defer func() { t.mu.Lock() - err := t.readerErr + delete(t.pending, seq) t.mu.Unlock() - if err != nil { - return nil, fmt.Errorf("%s: transport reader error: %w", command, err) + }() + select { + case resp, ok := <-ch: + if !ok { + // Channel closed by reader goroutine due to error. + t.mu.Lock() + err := t.readerErr + t.mu.Unlock() + if err != nil { + return nil, fmt.Errorf("%s: transport reader error: %w", command, err) + } + return nil, fmt.Errorf("%s: transport closed", command) } - return nil, fmt.Errorf("%s: transport closed", command) - } - if !resp.Success { - return nil, fmt.Errorf("%s: %s", command, resp.Message) + if !resp.Success { + return nil, fmt.Errorf("%s: %s", command, resp.Message) + } + return resp, nil + case <-ctx.Done(): + return nil, ctx.Err() } - return resp, nil - case <-ctx.Done(): - return nil, ctx.Err() - } + }, nil } // send marshals and writes a DAP message with Content-Length framing. @@ -152,6 +181,7 @@ func (t *transport) send(msg interface{}) error { // readMessages reads DAP messages and dispatches responses to pending callers // and events to registered handlers. func (t *transport) readMessages() { + defer close(t.done) // Closing the connection on cancellation unblocks the blocking reads // below so the goroutine exits even when no I/O error occurs on its own. stop := make(chan struct{}) @@ -230,6 +260,8 @@ func (t *transport) dispatchEvent(ev dapEvent) { defer t.eventMu.RUnlock() switch ev.Event { + case "initialized": + t.initializedOnce.Do(func() { close(t.initialized) }) case "stopped": if t.onStopped != nil { var body struct { @@ -289,6 +321,13 @@ func (t *transport) close() error { return t.conn.Close() } +type bufferedConnection struct { + net.Conn + reader io.Reader +} + +func (c *bufferedConnection) Read(p []byte) (int, error) { return c.reader.Read(p) } + // waitForConnection retries TCP connection until the DAP server is actually // ready. A simple TCP dial is NOT enough -- Docker's port proxy accepts // connections before the DAP server inside the container is listening. @@ -310,27 +349,30 @@ func waitForConnection(ctx context.Context, port int) (net.Conn, error) { time.Sleep(500 * time.Millisecond) continue } - // Verify the connection is actually alive by checking it - // doesn't get reset immediately. Docker's port proxy accepts - // connections before the backend is ready and then resets them. - conn.SetReadDeadline(time.Now().Add(500 * time.Millisecond)) - probe := make([]byte, 1) - _, err = conn.Read(probe) - conn.SetReadDeadline(time.Time{}) // clear deadline + ready, err := probeConnection(conn) if err != nil { - if netErr, ok := err.(net.Error); ok && netErr.Timeout() { - // Timeout on read = no data yet = connection is alive. - // DAP server won't send anything until we do. - return conn, nil - } - // EOF or connection reset = Docker proxy forwarded to a - // non-ready backend. Retry. conn.Close() time.Sleep(500 * time.Millisecond) continue } - // Got data (unexpected from DAP), but connection works. + return ready, nil + } + } +} + +// probeConnection detects a reset without discarding startup DAP frames. +func probeConnection(conn net.Conn) (net.Conn, error) { + conn.SetReadDeadline(time.Now().Add(500 * time.Millisecond)) + probe := make([]byte, 1) + n, err := conn.Read(probe) + conn.SetReadDeadline(time.Time{}) + if err != nil { + if netErr, ok := err.(net.Error); ok && netErr.Timeout() { return conn, nil } + return nil, err } + // debugpy sends telemetry immediately on connection. Dropping this byte + // corrupts Content-Length before the protocol reader can initialize. + return &bufferedConnection{Conn: conn, reader: io.MultiReader(bytes.NewReader(probe[:n]), conn)}, nil } diff --git a/companions/python/Dockerfile b/companions/python/Dockerfile index a19e82afc..7c186e654 100644 --- a/companions/python/Dockerfile +++ b/companions/python/Dockerfile @@ -59,7 +59,20 @@ RUN curl -LsSf https://astral.sh/uv/0.5.29/install.sh | env UV_INSTALL_DIR=/usr/ # # --break-system-packages is required because the alpine base marks the system # env as externally managed. -RUN pip install --no-cache-dir --break-system-packages debugpy==1.8.21 +RUN pip install --no-cache-dir --break-system-packages debugpy==1.8.21 && \ + python -m pip uninstall --yes --break-system-packages pip && \ + python -c 'import pathlib, shutil, sysconfig; shutil.rmtree(pathlib.Path(sysconfig.get_path("stdlib")) / "ensurepip")' + +# This runtime uses uv. Remove both installed pip (including its vendored +# dependencies) and the ensurepip bootstrap wheel, which can restore the same +# vulnerable vendors. Keep debugpy and its metadata intact; inventories must +# describe the actual remaining packages, not edited vendor/SBOM declarations. +RUN python -c 'import importlib.util, debugpy.adapter; assert importlib.util.find_spec("pip") is None; assert importlib.util.find_spec("ensurepip") is None' && \ + python -m debugpy.adapter --help && \ + uv venv --offline --python /usr/local/bin/python /tmp/python-runtime-check && \ + uv pip install --no-cache --python /tmp/python-runtime-check/bin/python debugpy==1.8.21 && \ + /tmp/python-runtime-check/bin/python -c 'import debugpy; assert debugpy.__version__ == "1.8.21"' && \ + rm -rf /tmp/python-runtime-check # uv venv convention used by python-fastapi (see DockerEnvPath in its # runtime). Plugins mount /venv so venvs survive container restarts. diff --git a/companions/python/README.md b/companions/python/README.md new file mode 100644 index 000000000..a2da99056 --- /dev/null +++ b/companions/python/README.md @@ -0,0 +1,42 @@ +# Python companion + +The Python runtime companion uses uv to create environments and install +dependencies. It keeps debugpy in the system interpreter because the DAP +companion launches `python -m debugpy.adapter`. Services mount their environment +at `/venv`; `UV_PROJECT_ENVIRONMENT` and `PATH` select it at runtime. + +Version 0.0.8 removes build-only pip after installing debugpy, including pip's +installed vendored dependencies and Python's bundled ensurepip bootstrap wheel. +Both copies in 0.0.7 contain the same vulnerable dependencies; upgrading a +standalone urllib3, setuptools or msgpack package does not replace those copies. +The current upstream pip 26.2.2 wheel still contains them. + +Use `uv venv` and `uv pip` for environment management. This image does not offer +`pip` or `python -m ensurepip`; ordinary `python -m venv` bootstrap behavior +must not be assumed. Debugpy, uv, and their package metadata are retained. +No vendor manifest or SBOM is rewritten to conceal installed code. +Codefly's editable-library and Python-upgrade helpers still invoke pip in their +calling process. Their ordinary host behavior is unchanged; invoking those +pip-dependent commands inside this companion is not a supported replacement +for the host workflow. This image change does not redefine host interpreter +selection. + +Both architecture builds verify pip/ensurepip absence, the debugpy adapter +entrypoint, and uv installation/import in a fresh environment. The tagged +`python_companion_required` DAP test exercises initialize and launch through +Core's maintained client against the selected companion. Publication +uses the [companion procedure](../../docs/runbooks/publish-companions.md). +Downstream security clearance requires rebuilding services and scanning their +exact image digests; a source change or companion tag is not that evidence. +Removal affects the merged runtime filesystem; historical immutable base-layer +blobs still contain their original bytes. + +The DAP client sends launch/attach, waits for the adapter's `initialized` +event, completes configuration, and then collects the start response. Debugpy +defers that response until `configurationDone`; waiting for it first deadlocks. +The connection probe also preserves early adapter telemetry bytes so the +protocol reader receives complete frames. +Launch omits unspecified arguments and environment overrides instead of sending +JSON null, which debugpy rejects. Explicit argument lists and environment maps +are preserved. This behavior belongs to the DAP client; ordinary application +image execution does not use this debugging handshake. diff --git a/companions/python/info.codefly.yaml b/companions/python/info.codefly.yaml index 87fb744f1..abc899f5a 100644 --- a/companions/python/info.codefly.yaml +++ b/companions/python/info.codefly.yaml @@ -1 +1 @@ -version: 0.0.7 +version: 0.0.8 diff --git a/version/info.codefly.yaml b/version/info.codefly.yaml index aee574164..35f234e95 100644 --- a/version/info.codefly.yaml +++ b/version/info.codefly.yaml @@ -1 +1 @@ -version: 0.19.15 +version: 0.19.16