Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions cmd/internal/agentworkspace/setup_gpg.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func fetchAndDecodeKeys(ownerTrustB64 string) ([]byte, []byte, error) {

func configureGPGAgent(gpgConf *gpg.GPGConf) error {
log.Debugf("Stopping container gpg-agent")
if err := gpgConf.StopGpgAgent(); err != nil {
if err := gpg.StopGpgAgent(); err != nil {
return fmt.Errorf("stop container gpg-agent: %w", err)
}

Expand All @@ -135,7 +135,7 @@ func configureGPGAgent(gpgConf *gpg.GPGConf) error {
// Now we again kill the agent and remove the socket to really be sure every
// thing is clean
log.Debugf("Ensure stopping container gpg-agent")
if err := gpgConf.StopGpgAgent(); err != nil {
if err := gpg.StopGpgAgent(); err != nil {
return fmt.Errorf("ensure stopping container gpg-agent: %w", err)
}

Expand All @@ -145,7 +145,7 @@ func configureGPGAgent(gpgConf *gpg.GPGConf) error {
}

log.Debugf("Setup gpg.conf")
if err := gpgConf.SetupGpgConf(); err != nil {
if err := gpg.SetupGpgConf(); err != nil {
return fmt.Errorf("setup gpg.conf: %w", err)
}

Expand Down
7 changes: 5 additions & 2 deletions cmd/workspace/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -745,7 +745,10 @@ func (cmd *SSHCmd) setupGPGAgent(

gitKey := gpg.SigningKey(ctx)

cmd.ReverseForwardPorts = append(cmd.ReverseForwardPorts, gpgExtraSocketPath)
cmd.ReverseForwardPorts = append(
cmd.ReverseForwardPorts,
gpg.ContainerSocketPath+":"+gpgExtraSocketPath,
)
Comment on lines +748 to +751

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm ParsePortSpec's field-assignment order for "A:B" strings.
rg -n "func ParsePortSpec" -A 40 --type=go
rg -n "type Mapping struct" -A 15 --type=go
rg -n "ReverseForwardPorts" -B2 -A5 --type=go -g '!*_test.go'

Repository: devsy-org/devsy

Length of output: 153


🏁 Script executed:

#!/bin/bash
set -u

echo "Tracked files around ssh/devssh/gpg/port:"
git ls-files | rg 'cmd/workspace/ssh\.go|devssh|gpg|port|setup_gpg|ssh\.go$' | sed -n '1,200p'

echo
echo "ssh.go outline around relevant functions:"
if [ -f cmd/workspace/ssh.go ]; then
  wc -l cmd/workspace/ssh.go
  rg -n "ReverseForwardPorts|ParsePortSpec|Mapping|setup-gpg|ReversePortForward|Reverse forwarding" cmd/workspace/ssh.go
fi

Repository: devsy-org/devsy

Length of output: 3381


🏁 Script executed:

#!/bin/bash
set -u

echo "pkg/port/parse.go:"
sed -n '1,220p' pkg/port/parse.go

echo
echo "pkg/port/port.go:"
sed -n '1,180p' pkg/port/port.go

echo
echo "cmd/workspace/ssh.go relevant sections:"
sed -n '1,130p' cmd/workspace/ssh.go
sed -n '430,470p' cmd/workspace/ssh.go
sed -n '730,770p' cmd/workspace/ssh.go

echo
echo "cmd/internal/agentworkspace/setup_gpg.go:"
sed -n '1,220p' cmd/internal/agentworkspace/setup_gpg.go

echo
echo "pkg/gpg/gpg_forwarding.go relevant socket symlink section:"
sed -n '130,200p' pkg/gpg/gpg_forwarding.go

echo
echo "Behavioral check of ParsePortSpec assignments for simple A:B strings:"
python3 - <<'PY'
from pathlib import Path
src = Path("pkg/port/parse.go").read_text()
print("ParsePortSpec source excerpt:")
start = src.index("func ParsePortSpec(s string)")
print(src[start:].split("func ")[0][:2200])
PY

Repository: devsy-org/devsy

Length of output: 16493


Reverse-forward mapping order is correct for GPG forwarding.

ParsePortSpec assigns a 2-part string A:B to mapping.Host.Address = A and mapping.Container.Address = B, so the current gpg.ContainerSocketPath+":"+gpgExtraSocketPath would bind Host = /tmp/S.gpg-agent and Container = detected host socket path, which is the wrong direction. Keep the suggested ordering gpgExtraSocketPath+":"+gpg.ContainerSocketPath if this is still present.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/workspace/ssh.go` around lines 748 - 751, Update the ReverseForwardPorts
mapping in the GPG forwarding setup to use gpgExtraSocketPath as the host side
and gpg.ContainerSocketPath as the container side, preserving the A:B ordering
expected by ParsePortSpec.


// Now we forward the agent socket to the remote, and setup remote gpg to use it
forwardAgent := []string{
Expand All @@ -757,7 +760,7 @@ func (cmd *SSHCmd) setupGPGAgent(
names.Flag(names.OwnerTrust),
ownerTrustArgument,
names.Flag(names.SocketPath),
gpgExtraSocketPath,
gpg.ContainerSocketPath,
}

if log.DebugEnabled() {
Expand Down
31 changes: 19 additions & 12 deletions e2e/framework/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -468,34 +468,41 @@ func (f *Framework) SetupGPG(tmpDir string) error {
return exec.Command("gpg", "-k").Run()
}

func (f *Framework) DevsySSHGpgTestKey(ctx context.Context, workspace string) error {
pubKeyB, err := exec.Command("sh", "-c", "gpg -k --with-colons 2>/dev/null | grep sec | base64 -w0").
Output()
func ImportGpgKey(privateKeyPath string) error {
// #nosec G204 -- test imports a controlled key path
out, err := exec.Command("gpg", "--batch", "--import", privateKeyPath).CombinedOutput()
if err != nil {
return err
return fmt.Errorf("import gpg private key: %s: %w", out, err)
}
if out, err := exec.Command("gpg-connect-agent", "/bye").CombinedOutput(); err != nil {
return fmt.Errorf("start gpg-agent: %s: %w", out, err)
}
return nil
}

// First run to trigger the first forwarding
func (f *Framework) DevsySSHGpgSecretKeyForwarded(
ctx context.Context,
workspace, expectedFingerprint string,
) error {
stdout, _, err := f.ExecCommandCapture(ctx, []string{
cmdWorkspace,
cmdSSH,
names.Flag(names.AgentForwarding),
names.Flag(names.SSHGPGForwarding),
flagCommand,
"gpg -k --with-colons 2>/dev/null |grep sec | base64 -w0", workspace,
"gpg --list-secret-keys --with-colons 2>/dev/null",
workspace,
})
if err != nil {
return err
return fmt.Errorf("list secret keys in container: %w", err)
}

if stdout != string(pubKeyB) {
if !strings.Contains(stdout, expectedFingerprint) {
return fmt.Errorf(
"devsy gpg public key forwarding failed, expected %s, got %s",
string(pubKeyB),
"gpg agent forwarding failed: secret key %s not reachable in container, got: %q",
expectedFingerprint,
stdout,
)
}

return nil
}

Expand Down
39 changes: 39 additions & 0 deletions e2e/tests/ssh/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
const (
osWindows = "windows"

gpgTestKeyFingerprint = "07F681B9FD6C3411F679BFD1F51769DB572DDD3F"

Comment on lines +25 to +26

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check whether a static private key fixture is committed to the repo.
fd -i 'gpg-private' tests/ssh/testdata

Repository: devsy-org/devsy

Length of output: 259


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Repo files count: '
git ls-files | wc -l

printf '\nCandidate ssh.go files:\n'
fd -i '^ssh\.go$|testdata$|gpg-private' . || true

printf '\nMatch gpg-private/fingerprint/gpg-forwarding:\n'
rg -n "gpg-private|gpg-forwarding|gpgTestKeyFingerprint|ImportGpgKey" . || true

Repository: devsy-org/devsy

Length of output: 2428


Don’t commit the GPG test private key fixture.

e2e/tests/ssh/testdata/gpg-forwarding/gpg-private.key is checked into the repo and imported by the e2e flow, so this shouldn’t be categorized as a secrets leak at line 25. Instead, flag this test fixture under the key fixture location as a cleanup: generate the disposable GPG key at setup time or exclude the private fixture from committed testdata.

🧰 Tools
🪛 Betterleaks (1.6.1)

[high] 25-25: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@e2e/tests/ssh/ssh.go` around lines 25 - 26, Remove the committed GPG
private-key fixture from the testdata used by the SSH e2e flow. Update the setup
around gpgTestKeyFingerprint and the GPG forwarding test to generate a
disposable key at runtime, or otherwise exclude the private fixture from
committed testdata while preserving the test’s behavior.

Source: Linters/SAST tools

envSSHAuthSock = "SSH_AUTH_SOCK"

sshOptStrictHostKeyCheckingNo = "StrictHostKeyChecking=no"
Expand Down Expand Up @@ -120,6 +122,43 @@ var _ = ginkgo.Describe("devsy ssh test suite", ginkgo.Label("ssh"), ginkgo.Orde
},
)

ginkgo.It(
"should expose the host GPG secret key in the container via agent forwarding",
ginkgo.Label("gpg"),
ginkgo.SpecTimeout(framework.TimeoutModerate()),
func(ctx ginkgo.SpecContext) {
if runtime.GOOS == osWindows {
ginkgo.Skip("skipping on windows")
}

tempDir, err := framework.CopyToTempDir("tests/ssh/testdata/gpg-forwarding")
framework.ExpectNoError(err)

f := framework.NewDefaultFramework(initialDir + "/bin")
_ = f.DevsyProviderAdd(ctx, "docker")
err = f.DevsyProviderUse(ctx, "docker")
framework.ExpectNoError(err)

ginkgo.DeferCleanup(func(cleanupCtx context.Context) {
_ = f.DevsyWorkspaceDelete(cleanupCtx, tempDir)
framework.CleanupTempDir(initialDir, tempDir)
})

ginkgo.GinkgoT().Setenv("GNUPGHOME", ginkgo.GinkgoT().TempDir())
framework.ExpectNoError(
framework.ImportGpgKey(filepath.Join(tempDir, "gpg-private.key")),
)

err = f.DevsyUp(ctx, tempDir, names.Flag(names.SSHGPGForwarding))
framework.ExpectNoError(err)

sshCtx, cancelSSH := context.WithDeadline(ctx, time.Now().Add(30*time.Second))
defer cancelSSH()
err = f.DevsySSHGpgSecretKeyForwarded(sshCtx, tempDir, gpgTestKeyFingerprint)
framework.ExpectNoError(err)
},
)

ginkgo.It(
"should set up git SSH signature helper and sign a commit",
ginkgo.SpecTimeout(framework.TimeoutLong()),
Expand Down
2 changes: 1 addition & 1 deletion e2e/tests/ssh/testdata/gpg-forwarding/.devcontainer.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{ "image": "ghcr.io/devsy-org/test-images/base:ubuntu" }
{ "image": "ghcr.io/devsy-org/test-images/base:ubuntu", "remoteUser": "vscode" }
3 changes: 2 additions & 1 deletion e2e/tests/ssh/testdata/gpg-forwarding/devcontainer.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{
"name": "Go",
"image": "ghcr.io/devsy-org/test-images/go:1"
"image": "ghcr.io/devsy-org/test-images/go:1",
"remoteUser": "vscode"
}
46 changes: 38 additions & 8 deletions pkg/gpg/forward.go
Original file line number Diff line number Diff line change
@@ -1,19 +1,26 @@
package gpg

import (
"context"
"os"
"os/exec"
"time"

client2 "github.com/devsy-org/devsy/pkg/client"
"github.com/devsy-org/devsy/pkg/flags/names"
"github.com/devsy-org/devsy/pkg/log"
devssh "github.com/devsy-org/devsy/pkg/ssh"
)

// ForwardAgent starts a background SSH connection that forwards the local GPG agent.
func ForwardAgent(client client2.BaseWorkspaceClient) error {
log.Debug("gpg forwarding enabled, performing immediately")
type backoff struct {
min, max time.Duration
}

var forwardRestartBackoff = backoff{min: time.Second, max: 30 * time.Second}

// ForwardAgent starts a supervised background SSH connection that forwards the
// local GPG agent, restarting it until ctx is cancelled.
func ForwardAgent(ctx context.Context, client client2.BaseWorkspaceClient) error {
execPath, err := os.Executable()
if err != nil {
return err
Expand All @@ -32,14 +39,37 @@ func ForwardAgent(client client2.BaseWorkspaceClient) error {

args := buildForwardArgs(remoteUser, client.Context(), client.Workspace())

go func() {
go superviseForward(ctx, execPath, args, forwardRestartBackoff)

return nil
}

func superviseForward(ctx context.Context, execPath string, args []string, b backoff) {
delay := b.min
for {
if ctx.Err() != nil {
return
}

start := time.Now()
//nolint:gosec // execPath comes from os.Executable()
if runErr := exec.Command(execPath, args...).Run(); runErr != nil {
log.Errorf("failure in forwarding gpg-agent: %v", runErr)
runErr := exec.CommandContext(ctx, execPath, args...).Run()
if ctx.Err() != nil {
return
}
}()

return nil
if time.Since(start) >= b.max {
delay = b.min
}
log.Errorf("gpg-agent forward exited (%v); restarting in %s", runErr, delay)

select {
case <-ctx.Done():
return
case <-time.After(delay):
}
delay = min(2*delay, b.max)
}
}

func buildForwardArgs(user, context, workspace string) []string {
Expand Down
49 changes: 49 additions & 0 deletions pkg/gpg/forward_test.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
package gpg

import (
"context"
"os"
"path/filepath"
"strings"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestBuildForwardArgs(t *testing.T) {
Expand All @@ -22,3 +28,46 @@ func TestBuildForwardArgs(t *testing.T) {
}
assert.Equal(t, expected, got)
}

func TestSuperviseForward_RestartsUntilCancelled(t *testing.T) {
runs := filepath.Join(t.TempDir(), "runs")
ctx, cancel := context.WithCancel(context.Background())

done := make(chan struct{})
go func() {
superviseForward(ctx, "/bin/sh", []string{"-c", "printf x >> " + runs},
backoff{min: 10 * time.Millisecond, max: 20 * time.Millisecond})
close(done)
}()

require.Eventually(t, func() bool {
data, _ := os.ReadFile(runs) //nolint:gosec // test path is created by the test
return strings.Count(string(data), "x") >= 2
}, 5*time.Second, 10*time.Millisecond, "forward should be restarted after it exits")

cancel()

select {
case <-done:
case <-time.After(2 * time.Second):
t.Fatal("superviseForward did not stop after ctx cancel")
}
}

func TestSuperviseForward_StopsImmediatelyIfCancelled(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
cancel()

done := make(chan struct{})
go func() {
superviseForward(ctx, "/bin/sh", []string{"-c", "exit 0"},
backoff{min: 10 * time.Millisecond, max: 20 * time.Millisecond})
close(done)
}()

select {
case <-done:
case <-time.After(time.Second):
t.Fatal("superviseForward should return promptly when ctx is already cancelled")
}
}
Loading
Loading