Skip to content

Commit f6a195e

Browse files
EdwardAngertEdwardAngert
EdwardAngert
authored and
EdwardAngert
committed
Merge remote-tracking branch 'origin/main' into 15503-ui-org-sync
2 parents 4dc7f3f + bc4f9a4 commit f6a195e

File tree

123 files changed

+7161
-1464
lines changed

Some content is hidden

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

123 files changed

+7161
-1464
lines changed

‎.github/workflows/ci.yaml

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ jobs:
188188
189189
# Check for any typos
190190
- name: Check for typos
191-
uses: crate-ci/typos@b74202f74b4346efdbce7801d187ec57b266bac8 # v1.27.3
191+
uses: crate-ci/typos@2872c382bb9668d4baa5eade234dcbc0048ca2cf # v1.28.2
192192
with:
193193
config: .github/workflows/typos.toml
194194

@@ -540,7 +540,7 @@ jobs:
540540
timeout-minutes: 25
541541
steps:
542542
- name: Harden Runner
543-
uses: step-security/harden-runner@91182cccc01eb5e619899d80e4e971d6181294a7 # v2.10.1
543+
uses: step-security/harden-runner@0080882f6c36860b6ba35c610c98ce87d4e2f26f # v2.10.2
544544
with:
545545
egress-policy: audit
546546

@@ -630,11 +630,8 @@ jobs:
630630
working-directory: site
631631

632632
test-e2e:
633-
# test-e2e fails on 2-core 8GB runners, so we use the 4-core 16GB runner
634633
runs-on: ${{ github.repository_owner == 'coder' && 'depot-ubuntu-22.04-4' || 'ubuntu-latest' }}
635634
needs: changes
636-
if: needs.changes.outputs.go == 'true' || needs.changes.outputs.ts == 'true' || needs.changes.outputs.ci == 'true' || github.ref == 'refs/heads/main'
637-
timeout-minutes: 20
638635
strategy:
639636
fail-fast: false
640637
matrix:
@@ -643,6 +640,9 @@ jobs:
643640
name: test-e2e
644641
- premium: true
645642
name: test-e2e-premium
643+
# Skip test-e2e on forks as they don't have access to CI secrets
644+
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)
645+
timeout-minutes: 20
646646
name: ${{ matrix.variant.name }}
647647
steps:
648648
- name: Harden Runner
@@ -666,6 +666,8 @@ jobs:
666666
name: make gen
667667

668668
- run: pnpm build
669+
env:
670+
NODE_OPTIONS: ${{ github.repository_owner == 'coder' && '--max_old_space_size=8192' || '' }}
669671
working-directory: site
670672

671673
- run: pnpm playwright:install
@@ -747,7 +749,7 @@ jobs:
747749
# Prevent excessive build runs on minor version changes
748750
skip: "@(renovate/**|dependabot/**)"
749751
# Run TurboSnap to trace file dependencies to related stories
750-
# and tell chromatic to only take snapshots of relevent stories
752+
# and tell chromatic to only take snapshots of relevant stories
751753
onlyChanged: true
752754
# Avoid uploading single files, because that's very slow
753755
zip: true
@@ -774,7 +776,7 @@ jobs:
774776
workingDir: "./site"
775777
storybookBaseDir: "./site"
776778
# Run TurboSnap to trace file dependencies to related stories
777-
# and tell chromatic to only take snapshots of relevent stories
779+
# and tell chromatic to only take snapshots of relevant stories
778780
onlyChanged: true
779781
# Avoid uploading single files, because that's very slow
780782
zip: true
@@ -1144,7 +1146,7 @@ jobs:
11441146
version: "2.2.1"
11451147

11461148
- name: Get Cluster Credentials
1147-
uses: google-github-actions/get-gke-credentials@206d64b64b0eba0a6e2f25113d044c31776ca8d6 # v2.2.2
1149+
uses: google-github-actions/get-gke-credentials@9025e8f90f2d8e0c3dafc3128cc705a26d992a6a # v2.3.0
11481150
with:
11491151
cluster_name: dogfood-v2
11501152
location: us-central1-a

‎.github/workflows/contrib.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ jobs:
5353
env:
5454
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
5555
# the below token should have repo scope and must be manually added by you in the repository's secret
56-
PERSONAL_ACCESS_TOKEN: ${{ secrets.CDRCOMMUNITY_GITHUB_TOKEN }}
56+
PERSONAL_ACCESS_TOKEN: ${{ secrets.CDRCI2_GITHUB_TOKEN }}
5757
with:
5858
remote-organization-name: "coder"
5959
remote-repository-name: "cla"

‎.github/workflows/scorecard.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,6 @@ jobs:
4747

4848
# Upload the results to GitHub's code scanning dashboard.
4949
- name: "Upload to code-scanning"
50-
uses: github/codeql-action/upload-sarif@f09c1c0a94de965c15400f5634aa42fac8fb8f88 # v3.27.5
50+
uses: github/codeql-action/upload-sarif@aa578102511db1f4524ed59b8cc2bae4f6e88195 # v3.27.6
5151
with:
5252
sarif_file: results.sarif

‎.github/workflows/security.yaml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ jobs:
3838
uses: ./.github/actions/setup-go
3939

4040
- name: Initialize CodeQL
41-
uses: github/codeql-action/init@f09c1c0a94de965c15400f5634aa42fac8fb8f88 # v3.27.5
41+
uses: github/codeql-action/init@aa578102511db1f4524ed59b8cc2bae4f6e88195 # v3.27.6
4242
with:
4343
languages: go, javascript
4444

@@ -48,7 +48,7 @@ jobs:
4848
rm Makefile
4949
5050
- name: Perform CodeQL Analysis
51-
uses: github/codeql-action/analyze@f09c1c0a94de965c15400f5634aa42fac8fb8f88 # v3.27.5
51+
uses: github/codeql-action/analyze@aa578102511db1f4524ed59b8cc2bae4f6e88195 # v3.27.6
5252

5353
- name: Send Slack notification on failure
5454
if: ${{ failure() }}
@@ -144,7 +144,7 @@ jobs:
144144
severity: "CRITICAL,HIGH"
145145

146146
- name: Upload Trivy scan results to GitHub Security tab
147-
uses: github/codeql-action/upload-sarif@f09c1c0a94de965c15400f5634aa42fac8fb8f88 # v3.27.5
147+
uses: github/codeql-action/upload-sarif@aa578102511db1f4524ed59b8cc2bae4f6e88195 # v3.27.6
148148
with:
149149
sarif_file: trivy-results.sarif
150150
category: "Trivy"

‎.golangci.yaml

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,8 +175,6 @@ linters-settings:
175175
- name: modifies-value-receiver
176176
- name: package-comments
177177
- name: range
178-
- name: range-val-address
179-
- name: range-val-in-closure
180178
- name: receiver-naming
181179
- name: redefines-builtin-id
182180
- name: string-of-int
@@ -199,6 +197,10 @@ linters-settings:
199197
govet:
200198
disable:
201199
- loopclosure
200+
gosec:
201+
excludes:
202+
# Implicit memory aliasing of items from a range statement (irrelevant as of Go v1.22)
203+
- G601
202204

203205
issues:
204206
# Rules listed here: https://github.com/securego/gosec#available-rules
@@ -238,7 +240,6 @@ linters:
238240
- errname
239241
- errorlint
240242
- exhaustruct
241-
- exportloopref
242243
- forcetypeassert
243244
- gocritic
244245
# gocyclo is may be useful in the future when we start caring

‎agent/agent.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"tailscale.com/util/clientmetric"
3434

3535
"cdr.dev/slog"
36+
"github.com/coder/coder/v2/agent/agentexec"
3637
"github.com/coder/coder/v2/agent/agentscripts"
3738
"github.com/coder/coder/v2/agent/agentssh"
3839
"github.com/coder/coder/v2/agent/proto"
@@ -80,6 +81,7 @@ type Options struct {
8081
ReportMetadataInterval time.Duration
8182
ServiceBannerRefreshInterval time.Duration
8283
BlockFileTransfer bool
84+
Execer agentexec.Execer
8385
}
8486

8587
type Client interface {
@@ -139,6 +141,10 @@ func New(options Options) Agent {
139141
prometheusRegistry = prometheus.NewRegistry()
140142
}
141143

144+
if options.Execer == nil {
145+
options.Execer = agentexec.DefaultExecer
146+
}
147+
142148
hardCtx, hardCancel := context.WithCancel(context.Background())
143149
gracefulCtx, gracefulCancel := context.WithCancel(hardCtx)
144150
a := &agent{
@@ -171,6 +177,7 @@ func New(options Options) Agent {
171177

172178
prometheusRegistry: prometheusRegistry,
173179
metrics: newAgentMetrics(prometheusRegistry),
180+
execer: options.Execer,
174181
}
175182
// Initially, we have a closed channel, reflecting the fact that we are not initially connected.
176183
// Each time we connect we replace the channel (while holding the closeMutex) with a new one
@@ -239,6 +246,7 @@ type agent struct {
239246
// metrics are prometheus registered metrics that will be collected and
240247
// labeled in Coder with the agent + workspace.
241248
metrics *agentMetrics
249+
execer agentexec.Execer
242250
}
243251

244252
func (a *agent) TailnetConn() *tailnet.Conn {
@@ -247,7 +255,7 @@ func (a *agent) TailnetConn() *tailnet.Conn {
247255

248256
func (a *agent) init() {
249257
// pass the "hard" context because we explicitly close the SSH server as part of graceful shutdown.
250-
sshSrv, err := agentssh.NewServer(a.hardCtx, a.logger.Named("ssh-server"), a.prometheusRegistry, a.filesystem, &agentssh.Config{
258+
sshSrv, err := agentssh.NewServer(a.hardCtx, a.logger.Named("ssh-server"), a.prometheusRegistry, a.filesystem, a.execer, &agentssh.Config{
251259
MaxTimeout: a.sshMaxTimeout,
252260
MOTDFile: func() string { return a.manifest.Load().MOTDFile },
253261
AnnouncementBanners: func() *[]codersdk.BannerConfig { return a.announcementBanners.Load() },

‎agent/agentexec/cli_linux.go

Lines changed: 62 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,16 @@ import (
99
"os"
1010
"os/exec"
1111
"runtime"
12+
"slices"
1213
"strconv"
1314
"strings"
1415
"syscall"
1516

1617
"golang.org/x/sys/unix"
1718
"golang.org/x/xerrors"
19+
"kernel.org/pub/linux/libs/security/libcap/cap"
1820
)
1921

20-
// unset is set to an invalid value for nice and oom scores.
21-
const unset = -2000
22-
2322
// CLI runs the agent-exec command. It should only be called by the cli package.
2423
func CLI() error {
2524
// We lock the OS thread here to avoid a race condition where the nice priority
@@ -51,6 +50,14 @@ func CLI() error {
5150
return xerrors.Errorf("no exec command provided %+v", os.Args)
5251
}
5352

53+
if *oom == unset {
54+
// If an explicit oom score isn't set, we use the default.
55+
*oom, err = defaultOOMScore()
56+
if err != nil {
57+
return xerrors.Errorf("get default oom score: %w", err)
58+
}
59+
}
60+
5461
if *nice == unset {
5562
// If an explicit nice score isn't set, we use the default.
5663
*nice, err = defaultNiceScore()
@@ -59,36 +66,62 @@ func CLI() error {
5966
}
6067
}
6168

62-
if *oom == unset {
63-
// If an explicit oom score isn't set, we use the default.
64-
*oom, err = defaultOOMScore()
65-
if err != nil {
66-
return xerrors.Errorf("get default oom score: %w", err)
67-
}
69+
// We drop effective caps prior to setting dumpable so that we limit the
70+
// impact of someone attempting to hijack the process (i.e. with a debugger)
71+
// to take advantage of the capabilities of the agent process. We encourage
72+
// users to set cap_net_admin on the agent binary for improved networking
73+
// performance and doing so results in the process having its SET_DUMPABLE
74+
// attribute disabled (meaning we cannot adjust the oom score).
75+
err = dropEffectiveCaps()
76+
if err != nil {
77+
printfStdErr("failed to drop effective caps: %v", err)
6878
}
6979

70-
err = unix.Setpriority(unix.PRIO_PROCESS, 0, *nice)
80+
// Set dumpable to 1 so that we can adjust the oom score. If the process
81+
// doesn't have capabilities or has an suid/sgid bit set, this is already
82+
// set.
83+
err = unix.Prctl(unix.PR_SET_DUMPABLE, 1, 0, 0, 0)
7184
if err != nil {
72-
// We alert the user instead of failing the command since it can be difficult to debug
73-
// for a template admin otherwise. It's quite possible (and easy) to set an
74-
// inappriopriate value for niceness.
75-
printfStdErr("failed to adjust niceness to %d for cmd %+v: %v", *nice, args, err)
85+
printfStdErr("failed to set dumpable: %v", err)
7686
}
7787

7888
err = writeOOMScoreAdj(*oom)
7989
if err != nil {
8090
// We alert the user instead of failing the command since it can be difficult to debug
8191
// for a template admin otherwise. It's quite possible (and easy) to set an
8292
// inappriopriate value for oom_score_adj.
83-
printfStdErr("failed to adjust oom score to %d for cmd %+v: %v", *oom, args, err)
93+
printfStdErr("failed to adjust oom score to %d for cmd %+v: %v", *oom, execArgs(os.Args), err)
94+
}
95+
96+
// Set dumpable back to 0 just to be safe. It's not inherited for execve anyways.
97+
err = unix.Prctl(unix.PR_SET_DUMPABLE, 0, 0, 0, 0)
98+
if err != nil {
99+
printfStdErr("failed to unset dumpable: %v", err)
100+
}
101+
102+
err = unix.Setpriority(unix.PRIO_PROCESS, 0, *nice)
103+
if err != nil {
104+
// We alert the user instead of failing the command since it can be difficult to debug
105+
// for a template admin otherwise. It's quite possible (and easy) to set an
106+
// inappriopriate value for niceness.
107+
printfStdErr("failed to adjust niceness to %d for cmd %+v: %v", *nice, args, err)
84108
}
85109

86110
path, err := exec.LookPath(args[0])
87111
if err != nil {
88112
return xerrors.Errorf("look path: %w", err)
89113
}
90114

91-
return syscall.Exec(path, args, os.Environ())
115+
// Remove environment variables specific to the agentexec command. This is
116+
// especially important for environments that are attempting to develop Coder in Coder.
117+
env := os.Environ()
118+
env = slices.DeleteFunc(env, func(e string) bool {
119+
return strings.HasPrefix(e, EnvProcPrioMgmt) ||
120+
strings.HasPrefix(e, EnvProcOOMScore) ||
121+
strings.HasPrefix(e, EnvProcNiceScore)
122+
})
123+
124+
return syscall.Exec(path, args, env)
92125
}
93126

94127
func defaultNiceScore() (int, error) {
@@ -154,3 +187,16 @@ func execArgs(args []string) []string {
154187
func printfStdErr(format string, a ...any) {
155188
_, _ = fmt.Fprintf(os.Stderr, "coder-agent: %s\n", fmt.Sprintf(format, a...))
156189
}
190+
191+
func dropEffectiveCaps() error {
192+
proc := cap.GetProc()
193+
err := proc.ClearFlag(cap.Effective)
194+
if err != nil {
195+
return xerrors.Errorf("clear effective caps: %w", err)
196+
}
197+
err = proc.SetProc()
198+
if err != nil {
199+
return xerrors.Errorf("set proc: %w", err)
200+
}
201+
return nil
202+
}

0 commit comments

Comments
 (0)