Skip to content

Commit a039753

Browse files
EdwardAngertEdwardAngert
authored
Merge branch 'main' into 16875-git-workspace-auth
2 parents 5b39e95 + 3f3e201 commit a039753

File tree

323 files changed

+12056
-3378
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

323 files changed

+12056
-3378
lines changed

‎.github/.linkspector.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,6 @@ ignorePatterns:
2323
- pattern: "wiki.ubuntu.com"
2424
- pattern: "mutagen.io"
2525
- pattern: "docs.github.com"
26+
- pattern: "claude.ai"
2627
aliveStatusCodes:
2728
- 200

‎.github/actions/setup-tf/action.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,5 @@ runs:
77
- name: Install Terraform
88
uses: hashicorp/setup-terraform@b9cd54a3c349d3f38e8881555d616ced269862dd # v3.1.2
99
with:
10-
terraform_version: 1.11.2
10+
terraform_version: 1.11.3
1111
terraform_wrapper: false

‎.github/workflows/ci.yaml

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -677,8 +677,8 @@ jobs:
677677
variant:
678678
- premium: false
679679
name: test-e2e
680-
- premium: true
681-
name: test-e2e-premium
680+
#- premium: true
681+
# name: test-e2e-premium
682682
# Skip test-e2e on forks as they don't have access to CI secrets
683683
if: (needs.changes.outputs.go == 'true' || needs.changes.outputs.ts == 'true' || needs.changes.outputs.ci == 'true' || github.ref == 'refs/heads/main') && !(github.event.pull_request.head.repo.fork)
684684
timeout-minutes: 20
@@ -1180,6 +1180,34 @@ jobs:
11801180
done
11811181
fi
11821182
1183+
- name: SBOM Generation and Attestation
1184+
if: github.ref == 'refs/heads/main'
1185+
continue-on-error: true
1186+
env:
1187+
COSIGN_EXPERIMENTAL: 1
1188+
run: |
1189+
set -euxo pipefail
1190+
1191+
# Define image base and tags
1192+
IMAGE_BASE="ghcr.io/coder/coder-preview"
1193+
TAGS=("${{ steps.build-docker.outputs.tag }}" "main" "latest")
1194+
1195+
# Generate and attest SBOM for each tag
1196+
for tag in "${TAGS[@]}"; do
1197+
IMAGE="${IMAGE_BASE}:${tag}"
1198+
SBOM_FILE="coder_sbom_${tag//[:\/]/_}.spdx.json"
1199+
1200+
echo "Generating SBOM for image: ${IMAGE}"
1201+
syft "${IMAGE}" -o spdx-json > "${SBOM_FILE}"
1202+
1203+
echo "Attesting SBOM to image: ${IMAGE}"
1204+
cosign clean --force=true "${IMAGE}"
1205+
cosign attest --type spdxjson \
1206+
--predicate "${SBOM_FILE}" \
1207+
--yes \
1208+
"${IMAGE}"
1209+
done
1210+
11831211
# GitHub attestation provides SLSA provenance for the Docker images, establishing a verifiable
11841212
# record that these images were built in GitHub Actions with specific inputs and environment.
11851213
# This complements our existing cosign attestations which focus on SBOMs.

‎.github/workflows/release.yaml

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,39 @@ jobs:
496496
env:
497497
CODER_BASE_IMAGE_TAG: ${{ steps.image-base-tag.outputs.tag }}
498498

499+
- name: SBOM Generation and Attestation
500+
if: ${{ !inputs.dry_run }}
501+
env:
502+
COSIGN_EXPERIMENTAL: "1"
503+
run: |
504+
set -euxo pipefail
505+
506+
# Generate SBOM for multi-arch image with version in filename
507+
echo "Generating SBOM for multi-arch image: ${{ steps.build_docker.outputs.multiarch_image }}"
508+
syft "${{ steps.build_docker.outputs.multiarch_image }}" -o spdx-json > coder_${{ steps.version.outputs.version }}_sbom.spdx.json
509+
510+
# Attest SBOM to multi-arch image
511+
echo "Attesting SBOM to multi-arch image: ${{ steps.build_docker.outputs.multiarch_image }}"
512+
cosign clean --force=true "${{ steps.build_docker.outputs.multiarch_image }}"
513+
cosign attest --type spdxjson \
514+
--predicate coder_${{ steps.version.outputs.version }}_sbom.spdx.json \
515+
--yes \
516+
"${{ steps.build_docker.outputs.multiarch_image }}"
517+
518+
# If latest tag was created, also attest it
519+
if [[ "${{ steps.build_docker.outputs.created_latest_tag }}" == "true" ]]; then
520+
latest_tag="$(./scripts/image_tag.sh --version latest)"
521+
echo "Generating SBOM for latest image: ${latest_tag}"
522+
syft "${latest_tag}" -o spdx-json > coder_latest_sbom.spdx.json
523+
524+
echo "Attesting SBOM to latest image: ${latest_tag}"
525+
cosign clean --force=true "${latest_tag}"
526+
cosign attest --type spdxjson \
527+
--predicate coder_latest_sbom.spdx.json \
528+
--yes \
529+
"${latest_tag}"
530+
fi
531+
499532
- name: GitHub Attestation for Docker image
500533
id: attest_main
501534
if: ${{ !inputs.dry_run }}
@@ -612,16 +645,27 @@ jobs:
612645
fi
613646
declare -p publish_args
614647
648+
# Build the list of files to publish
649+
files=(
650+
./build/*_installer.exe
651+
./build/*.zip
652+
./build/*.tar.gz
653+
./build/*.tgz
654+
./build/*.apk
655+
./build/*.deb
656+
./build/*.rpm
657+
./coder_${{ steps.version.outputs.version }}_sbom.spdx.json
658+
)
659+
660+
# Only include the latest SBOM file if it was created
661+
if [[ "${{ steps.build_docker.outputs.created_latest_tag }}" == "true" ]]; then
662+
files+=(./coder_latest_sbom.spdx.json)
663+
fi
664+
615665
./scripts/release/publish.sh \
616666
"${publish_args[@]}" \
617667
--release-notes-file "$CODER_RELEASE_NOTES_FILE" \
618-
./build/*_installer.exe \
619-
./build/*.zip \
620-
./build/*.tar.gz \
621-
./build/*.tgz \
622-
./build/*.apk \
623-
./build/*.deb \
624-
./build/*.rpm
668+
"${files[@]}"
625669
env:
626670
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
627671
CODER_GPG_RELEASE_KEY_BASE64: ${{ secrets.GPG_RELEASE_KEY_BASE64 }}
@@ -663,6 +707,15 @@ jobs:
663707
./build/*.apk
664708
./build/*.deb
665709
./build/*.rpm
710+
./coder_${{ steps.version.outputs.version }}_sbom.spdx.json
711+
retention-days: 7
712+
713+
- name: Upload latest sbom artifact to actions (if dry-run)
714+
if: inputs.dry_run && steps.build_docker.outputs.created_latest_tag == 'true'
715+
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
716+
with:
717+
name: latest-sbom-artifact
718+
path: ./coder_latest_sbom.spdx.json
666719
retention-days: 7
667720

668721
- name: Send repository-dispatch event

‎.golangci.yaml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ linters-settings:
164164
- name: unnecessary-stmt
165165
- name: unreachable-code
166166
- name: unused-parameter
167+
exclude: "**/*_test.go"
167168
- name: unused-receiver
168169
- name: var-declaration
169170
- name: var-naming
@@ -195,8 +196,6 @@ issues:
195196
- errcheck
196197
- forcetypeassert
197198
- exhaustruct # This is unhelpful in tests.
198-
- revive # TODO(JonA): disabling in order to update golangci-lint
199-
- gosec # TODO(JonA): disabling in order to update golangci-lint
200199
- path: scripts/*
201200
linters:
202201
- exhaustruct

‎.vscode/settings.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,5 +57,8 @@
5757
"[css][html][markdown][yaml]": {
5858
"editor.defaultFormatter": "esbenp.prettier-vscode"
5959
},
60-
"typos.config": ".github/workflows/typos.toml"
60+
"typos.config": ".github/workflows/typos.toml",
61+
"[markdown]": {
62+
"editor.defaultFormatter": "DavidAnson.vscode-markdownlint"
63+
}
6164
}

‎Makefile

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -581,7 +581,8 @@ GEN_FILES := \
581581
$(TAILNETTEST_MOCKS) \
582582
coderd/database/pubsub/psmock/psmock.go \
583583
agent/agentcontainers/acmock/acmock.go \
584-
agent/agentcontainers/dcspec/dcspec_gen.go
584+
agent/agentcontainers/dcspec/dcspec_gen.go \
585+
coderd/httpmw/loggermock/loggermock.go
585586

586587
# all gen targets should be added here and to gen/mark-fresh
587588
gen: gen/db gen/golden-files $(GEN_FILES)
@@ -630,6 +631,7 @@ gen/mark-fresh:
630631
coderd/database/pubsub/psmock/psmock.go \
631632
agent/agentcontainers/acmock/acmock.go \
632633
agent/agentcontainers/dcspec/dcspec_gen.go \
634+
coderd/httpmw/loggermock/loggermock.go \
633635
"
634636

635637
for file in $$files; do
@@ -669,6 +671,10 @@ agent/agentcontainers/acmock/acmock.go: agent/agentcontainers/containers.go
669671
go generate ./agent/agentcontainers/acmock/
670672
touch "$@"
671673

674+
coderd/httpmw/loggermock/loggermock.go: coderd/httpmw/logger.go
675+
go generate ./coderd/httpmw/loggermock/
676+
touch "$@"
677+
672678
agent/agentcontainers/dcspec/dcspec_gen.go: \
673679
node_modules/.installed \
674680
agent/agentcontainers/dcspec/devContainer.base.schema.json \

‎agent/agent.go

Lines changed: 54 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -907,7 +907,7 @@ func (a *agent) run() (retErr error) {
907907
defer func() {
908908
cErr := aAPI.DRPCConn().Close()
909909
if cErr != nil {
910-
a.logger.Debug(a.hardCtx, "error closing drpc connection", slog.Error(err))
910+
a.logger.Debug(a.hardCtx, "error closing drpc connection", slog.Error(cErr))
911911
}
912912
}()
913913

@@ -1186,9 +1186,9 @@ func (a *agent) createOrUpdateNetwork(manifestOK, networkOK *checkpoint) func(co
11861186
network := a.network
11871187
a.closeMutex.Unlock()
11881188
if network == nil {
1189-
keySeed, err := WorkspaceKeySeed(manifest.WorkspaceID, manifest.AgentName)
1189+
keySeed, err := SSHKeySeed(manifest.OwnerName, manifest.WorkspaceName, manifest.AgentName)
11901190
if err != nil {
1191-
return xerrors.Errorf("generate seed from workspace id: %w", err)
1191+
return xerrors.Errorf("generate SSH key seed: %w", err)
11921192
}
11931193
// use the graceful context here, because creating the tailnet is not itself tied to the
11941194
// agent API.
@@ -1408,7 +1408,7 @@ func (a *agent) createTailnet(
14081408
if rPTYServeErr != nil &&
14091409
a.gracefulCtx.Err() == nil &&
14101410
!strings.Contains(rPTYServeErr.Error(), "use of closed network connection") {
1411-
a.logger.Error(ctx, "error serving reconnecting PTY", slog.Error(err))
1411+
a.logger.Error(ctx, "error serving reconnecting PTY", slog.Error(rPTYServeErr))
14121412
}
14131413
}); err != nil {
14141414
return nil, err
@@ -1518,14 +1518,11 @@ func (a *agent) runCoordinator(ctx context.Context, tClient tailnetproto.DRPCTai
15181518
a.logger.Info(ctx, "connected to coordination RPC")
15191519

15201520
// This allows the Close() routine to wait for the coordinator to gracefully disconnect.
1521-
a.closeMutex.Lock()
1522-
if a.isClosed() {
1523-
return nil
1521+
disconnected := a.setCoordDisconnected()
1522+
if disconnected == nil {
1523+
return nil // already closed by something else
15241524
}
1525-
disconnected := make(chan struct{})
1526-
a.coordDisconnected = disconnected
15271525
defer close(disconnected)
1528-
a.closeMutex.Unlock()
15291526

15301527
ctrl := tailnet.NewAgentCoordinationController(a.logger, network)
15311528
coordination := ctrl.New(coordinate)
@@ -1547,6 +1544,17 @@ func (a *agent) runCoordinator(ctx context.Context, tClient tailnetproto.DRPCTai
15471544
return <-errCh
15481545
}
15491546

1547+
func (a *agent) setCoordDisconnected() chan struct{} {
1548+
a.closeMutex.Lock()
1549+
defer a.closeMutex.Unlock()
1550+
if a.isClosed() {
1551+
return nil
1552+
}
1553+
disconnected := make(chan struct{})
1554+
a.coordDisconnected = disconnected
1555+
return disconnected
1556+
}
1557+
15501558
// runDERPMapSubscriber runs a coordinator and returns if a reconnect should occur.
15511559
func (a *agent) runDERPMapSubscriber(ctx context.Context, tClient tailnetproto.DRPCTailnetClient24, network *tailnet.Conn) error {
15521560
defer a.logger.Debug(ctx, "disconnected from derp map RPC")
@@ -1773,15 +1781,22 @@ func (a *agent) Close() error {
17731781
a.setLifecycle(codersdk.WorkspaceAgentLifecycleShuttingDown)
17741782

17751783
// Attempt to gracefully shut down all active SSH connections and
1776-
// stop accepting new ones.
1777-
err := a.sshServer.Shutdown(a.hardCtx)
1784+
// stop accepting new ones. If all processes have not exited after 5
1785+
// seconds, we just log it and move on as it's more important to run
1786+
// the shutdown scripts. A typical shutdown time for containers is
1787+
// 10 seconds, so this still leaves a bit of time to run the
1788+
// shutdown scripts in the worst-case.
1789+
sshShutdownCtx, sshShutdownCancel := context.WithTimeout(a.hardCtx, 5*time.Second)
1790+
defer sshShutdownCancel()
1791+
err := a.sshServer.Shutdown(sshShutdownCtx)
17781792
if err != nil {
1779-
a.logger.Error(a.hardCtx, "ssh server shutdown", slog.Error(err))
1780-
}
1781-
err = a.sshServer.Close()
1782-
if err != nil {
1783-
a.logger.Error(a.hardCtx, "ssh server close", slog.Error(err))
1793+
if errors.Is(err, context.DeadlineExceeded) {
1794+
a.logger.Warn(sshShutdownCtx, "ssh server shutdown timeout", slog.Error(err))
1795+
} else {
1796+
a.logger.Error(sshShutdownCtx, "ssh server shutdown", slog.Error(err))
1797+
}
17841798
}
1799+
17851800
// wait for SSH to shut down before the general graceful cancel, because
17861801
// this triggers a disconnect in the tailnet layer, telling all clients to
17871802
// shut down their wireguard tunnels to us. If SSH sessions are still up,
@@ -2061,12 +2076,31 @@ func PrometheusMetricsHandler(prometheusRegistry *prometheus.Registry, logger sl
20612076
})
20622077
}
20632078

2064-
// WorkspaceKeySeed converts a WorkspaceID UUID and agent name to an int64 hash.
2079+
// SSHKeySeed converts an owner userName, workspaceName and agentName to an int64 hash.
20652080
// This uses the FNV-1a hash algorithm which provides decent distribution and collision
20662081
// resistance for string inputs.
2067-
func WorkspaceKeySeed(workspaceID uuid.UUID, agentName string) (int64, error) {
2082+
//
2083+
// Why owner username, workspace name, and agent name? These are the components that are used in hostnames for the
2084+
// workspace over SSH, and so we want the workspace to have a stable key with respect to these. We don't use the
2085+
// respective UUIDs. The workspace UUID would be different if you delete and recreate a workspace with the same name.
2086+
// The agent UUID is regenerated on each build. Since Coder's Tailnet networking is handling the authentication, we
2087+
// should not be showing users warnings about host SSH keys.
2088+
func SSHKeySeed(userName, workspaceName, agentName string) (int64, error) {
20682089
h := fnv.New64a()
2069-
_, err := h.Write(workspaceID[:])
2090+
_, err := h.Write([]byte(userName))
2091+
if err != nil {
2092+
return 42, err
2093+
}
2094+
// null separators between strings so that (dog, foodstuff) is distinct from (dogfood, stuff)
2095+
_, err = h.Write([]byte{0})
2096+
if err != nil {
2097+
return 42, err
2098+
}
2099+
_, err = h.Write([]byte(workspaceName))
2100+
if err != nil {
2101+
return 42, err
2102+
}
2103+
_, err = h.Write([]byte{0})
20702104
if err != nil {
20712105
return 42, err
20722106
}

‎agent/agent_test.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ func TestAgent_Stats_Magic(t *testing.T) {
190190
s, ok := <-stats
191191
t.Logf("got stats: ok=%t, ConnectionCount=%d, RxBytes=%d, TxBytes=%d, SessionCountVSCode=%d, ConnectionMedianLatencyMS=%f",
192192
ok, s.ConnectionCount, s.RxBytes, s.TxBytes, s.SessionCountVscode, s.ConnectionMedianLatencyMs)
193-
return ok && s.ConnectionCount > 0 && s.RxBytes > 0 && s.TxBytes > 0 &&
193+
return ok &&
194194
// Ensure that the connection didn't count as a "normal" SSH session.
195195
// This was a special one, so it should be labeled specially in the stats!
196196
s.SessionCountVscode == 1 &&
@@ -258,8 +258,7 @@ func TestAgent_Stats_Magic(t *testing.T) {
258258
s, ok := <-stats
259259
t.Logf("got stats with conn open: ok=%t, ConnectionCount=%d, SessionCountJetBrains=%d",
260260
ok, s.ConnectionCount, s.SessionCountJetbrains)
261-
return ok && s.ConnectionCount > 0 &&
262-
s.SessionCountJetbrains == 1
261+
return ok && s.SessionCountJetbrains == 1
263262
}, testutil.WaitLong, testutil.IntervalFast,
264263
"never saw stats with conn open",
265264
)

‎agent/agentscripts/agentscripts_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,13 +102,16 @@ func TestEnv(t *testing.T) {
102102

103103
func TestTimeout(t *testing.T) {
104104
t.Parallel()
105+
if runtime.GOOS == "darwin" {
106+
t.Skip("this test is flaky on macOS, see https://github.com/coder/internal/issues/329")
107+
}
105108
runner := setup(t, nil)
106109
defer runner.Close()
107110
aAPI := agenttest.NewFakeAgentAPI(t, testutil.Logger(t), nil, nil)
108111
err := runner.Init([]codersdk.WorkspaceAgentScript{{
109112
LogSourceID: uuid.New(),
110113
Script: "sleep infinity",
111-
Timeout: time.Millisecond,
114+
Timeout: 100 * time.Millisecond,
112115
}}, aAPI.ScriptCompleted)
113116
require.NoError(t, err)
114117
require.ErrorIs(t, runner.Execute(context.Background(), agentscripts.ExecuteAllScripts), agentscripts.ErrTimeout)

0 commit comments

Comments
 (0)