Skip to content

Commit cb4b5ee

Browse files
Kira-PilotKira-Pilot
committed
Merge remote-tracking branch 'origin/main' into audit-on-build/kira-pilot
2 parents 69cdf39 + 72288c3 commit cb4b5ee

File tree

110 files changed

+3156
-764
lines changed

Some content is hidden

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

110 files changed

+3156
-764
lines changed

‎.golangci.yaml

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -235,10 +235,15 @@ linters:
235235
- noctx
236236
- paralleltest
237237
- revive
238-
- rowserrcheck
239-
- sqlclosecheck
238+
239+
# These don't work until the following issue is solved.
240+
# https://github.com/golangci/golangci-lint/issues/2649
241+
# - rowserrcheck
242+
# - sqlclosecheck
243+
# - structcheck
244+
# - wastedassign
245+
240246
- staticcheck
241-
- structcheck
242247
- tenv
243248
# In Go, it's possible for a package to test it's internal functionality
244249
# without testing any exported functions. This is enabled to promote
@@ -253,4 +258,3 @@ linters:
253258
- unconvert
254259
- unused
255260
- varcheck
256-
- wastedassign

‎agent/agent.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,18 @@ func (a *agent) init(ctx context.Context) {
448448
"sftp": func(session ssh.Session) {
449449
session.DisablePTYEmulation()
450450

451-
server, err := sftp.NewServer(session)
451+
var opts []sftp.ServerOption
452+
// Change current working directory to the users home
453+
// directory so that SFTP connections land there.
454+
// https://github.com/coder/coder/issues/3620
455+
u, err := user.Current()
456+
if err != nil {
457+
a.logger.Warn(ctx, "get sftp working directory failed, unable to get current user", slog.Error(err))
458+
} else {
459+
opts = append(opts, sftp.WithServerWorkingDirectory(u.HomeDir))
460+
}
461+
462+
server, err := sftp.NewServer(session, opts...)
452463
if err != nil {
453464
a.logger.Debug(session.Context(), "initialize sftp server", slog.Error(err))
454465
return

‎agent/agent_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"net/netip"
1111
"os"
1212
"os/exec"
13+
"os/user"
1314
"path/filepath"
1415
"runtime"
1516
"strconv"
@@ -212,12 +213,21 @@ func TestAgent(t *testing.T) {
212213

213214
t.Run("SFTP", func(t *testing.T) {
214215
t.Parallel()
216+
u, err := user.Current()
217+
require.NoError(t, err, "get current user")
218+
home := u.HomeDir
219+
if runtime.GOOS == "windows" {
220+
home = "/" + strings.ReplaceAll(home, "\\", "/")
221+
}
215222
conn, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0)
216223
sshClient, err := conn.SSHClient()
217224
require.NoError(t, err)
218225
defer sshClient.Close()
219226
client, err := sftp.NewClient(sshClient)
220227
require.NoError(t, err)
228+
wd, err := client.Getwd()
229+
require.NoError(t, err, "get working directory")
230+
require.Equal(t, home, wd, "working directory should be home user home")
221231
tempFile := filepath.Join(t.TempDir(), "sftp")
222232
file, err := client.Create(tempFile)
223233
require.NoError(t, err)

‎agent/apphealth.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,10 @@ func NewWorkspaceAppHealthReporter(logger slog.Logger, workspaceAgentApps Worksp
6060
continue
6161
}
6262
app := nextApp
63-
t := time.NewTicker(time.Duration(app.Healthcheck.Interval) * time.Second)
6463
go func() {
64+
t := time.NewTicker(time.Duration(app.Healthcheck.Interval) * time.Second)
65+
defer t.Stop()
66+
6567
for {
6668
select {
6769
case <-ctx.Done():
@@ -118,6 +120,7 @@ func NewWorkspaceAppHealthReporter(logger slog.Logger, workspaceAgentApps Worksp
118120
lastHealth := copyHealth(health)
119121
mu.Unlock()
120122
reportTicker := time.NewTicker(time.Second)
123+
defer reportTicker.Stop()
121124
// every second we check if the health values of the apps have changed
122125
// and if there is a change we will report the new values.
123126
for {

‎cli/server.go

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,15 +208,25 @@ func Server(dflags *codersdk.DeploymentFlags, newAPI func(context.Context, *code
208208
)
209209
defer closeTunnel()
210210

211-
// If the access URL is empty, we attempt to run a reverse-proxy tunnel
212-
// to make the initial setup really simple.
211+
// If the access URL is empty, we attempt to run a reverse-proxy
212+
// tunnel to make the initial setup really simple.
213213
if dflags.AccessURL.Value == "" {
214214
cmd.Printf("Opening tunnel so workspaces can connect to your deployment. For production scenarios, specify an external access URL\n")
215215
tunnel, tunnelErr, err = devtunnel.New(ctxTunnel, logger.Named("devtunnel"))
216216
if err != nil {
217217
return xerrors.Errorf("create tunnel: %w", err)
218218
}
219219
dflags.AccessURL.Value = tunnel.URL
220+
221+
if dflags.WildcardAccessURL.Value == "" {
222+
u, err := parseURL(ctx, tunnel.URL)
223+
if err != nil {
224+
return xerrors.Errorf("parse tunnel url: %w", err)
225+
}
226+
227+
// Suffixed wildcard access URL.
228+
dflags.WildcardAccessURL.Value = fmt.Sprintf("*--%s", u.Hostname())
229+
}
220230
}
221231

222232
accessURLParsed, err := parseURL(ctx, dflags.AccessURL.Value)
@@ -752,7 +762,7 @@ func Server(dflags *codersdk.DeploymentFlags, newAPI func(context.Context, *code
752762

753763
// parseURL parses a string into a URL. It works around some technically correct
754764
// but undesired behavior of url.Parse by prepending a scheme if one does not
755-
// exist so that the URL does not get parsed improprely.
765+
// exist so that the URL does not get parsed improperly.
756766
func parseURL(ctx context.Context, u string) (*url.URL, error) {
757767
var (
758768
hasScheme = strings.HasPrefix(u, "http:") || strings.HasPrefix(u, "https:")
@@ -1108,7 +1118,9 @@ func serveHandler(ctx context.Context, logger slog.Logger, handler http.Handler,
11081118
}
11091119
}()
11101120

1111-
return func() { _ = srv.Close() }
1121+
return func() {
1122+
_ = srv.Close()
1123+
}
11121124
}
11131125

11141126
// embeddedPostgresURL returns the URL for the embedded PostgreSQL deployment.

‎coderd/audit/diff.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ type Auditable interface {
1616
database.User |
1717
database.Workspace |
1818
database.WorkspaceBuild |
19-
database.GitSSHKey
19+
database.GitSSHKey |
20+
database.Group
2021
}
2122

2223
// Map is a map of changed fields in an audited resource. It maps field names to

‎coderd/audit/request.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ func ResourceTarget[T Auditable](tgt T) string {
4949
return string(typed.BuildNumber)
5050
case database.GitSSHKey:
5151
return typed.PublicKey
52+
case database.Group:
53+
return typed.Name
5254
default:
5355
panic(fmt.Sprintf("unknown resource %T", tgt))
5456
}
@@ -70,6 +72,8 @@ func ResourceID[T Auditable](tgt T) uuid.UUID {
7072
return typed.ID
7173
case database.GitSSHKey:
7274
return typed.UserID
75+
case database.Group:
76+
return typed.ID
7377
default:
7478
panic(fmt.Sprintf("unknown resource %T", tgt))
7579
}
@@ -91,6 +95,8 @@ func ResourceType[T Auditable](tgt T) database.ResourceType {
9195
return database.ResourceTypeWorkspaceBuild
9296
case database.GitSSHKey:
9397
return database.ResourceTypeGitSshKey
98+
case database.Group:
99+
return database.ResourceTypeGroup
94100
default:
95101
panic(fmt.Sprintf("unknown resource %T", tgt))
96102
}

‎coderd/coderd.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ func New(options *Options) *API {
204204
// app URL. If it is, it will serve that application.
205205
api.handleSubdomainApplications(
206206
// Middleware to impose on the served application.
207-
httpmw.RateLimitPerMinute(options.APIRateLimit),
207+
httpmw.RateLimit(options.APIRateLimit, time.Minute),
208208
httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
209209
DB: options.Database,
210210
OAuth2Configs: oauthConfigs,
@@ -229,7 +229,7 @@ func New(options *Options) *API {
229229
apps := func(r chi.Router) {
230230
r.Use(
231231
tracing.Middleware(api.TracerProvider),
232-
httpmw.RateLimitPerMinute(options.APIRateLimit),
232+
httpmw.RateLimit(options.APIRateLimit, time.Minute),
233233
httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
234234
DB: options.Database,
235235
OAuth2Configs: oauthConfigs,
@@ -267,7 +267,7 @@ func New(options *Options) *API {
267267
r.Use(
268268
tracing.Middleware(api.TracerProvider),
269269
// Specific routes can specify smaller limits.
270-
httpmw.RateLimitPerMinute(options.APIRateLimit),
270+
httpmw.RateLimit(options.APIRateLimit, time.Minute),
271271
)
272272
r.Get("/", func(w http.ResponseWriter, r *http.Request) {
273273
httpapi.Write(r.Context(), w, http.StatusOK, codersdk.Response{
@@ -304,7 +304,7 @@ func New(options *Options) *API {
304304
apiKeyMiddleware,
305305
// This number is arbitrary, but reading/writing
306306
// file content is expensive so it should be small.
307-
httpmw.RateLimitPerMinute(12),
307+
httpmw.RateLimit(12, time.Minute),
308308
)
309309
r.Get("/{fileID}", api.fileByID)
310310
r.Post("/", api.postFile)
@@ -391,7 +391,15 @@ func New(options *Options) *API {
391391
r.Route("/users", func(r chi.Router) {
392392
r.Get("/first", api.firstUser)
393393
r.Post("/first", api.postFirstUser)
394-
r.Post("/login", api.postLogin)
394+
r.Group(func(r chi.Router) {
395+
// We use a tight limit for password login to protect
396+
// against audit-log write DoS, pbkdf2 DoS, and simple
397+
// brute-force attacks.
398+
//
399+
// Making this too small can break tests.
400+
r.Use(httpmw.RateLimit(60, time.Minute))
401+
r.Post("/login", api.postLogin)
402+
})
395403
r.Get("/authmethods", api.userAuthMethods)
396404
r.Route("/oauth2", func(r chi.Router) {
397405
r.Route("/github", func(r chi.Router) {
@@ -495,6 +503,7 @@ func New(options *Options) *API {
495503
apiKeyMiddleware,
496504
)
497505
r.Get("/", api.workspaces)
506+
r.Get("/count", api.workspaceCount)
498507
r.Route("/{workspace}", func(r chi.Router) {
499508
r.Use(
500509
httpmw.ExtractWorkspaceParam(options.Database),

‎coderd/coderdtest/authorize.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,8 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) {
243243
"POST:/api/v2/organizations/{organization}/templateversions": {StatusCode: http.StatusBadRequest, NoAuthorize: true},
244244

245245
// Endpoints that use the SQLQuery filter.
246-
"GET:/api/v2/workspaces/": {StatusCode: http.StatusOK, NoAuthorize: true},
246+
"GET:/api/v2/workspaces/": {StatusCode: http.StatusOK, NoAuthorize: true},
247+
"GET:/api/v2/workspaces/count": {StatusCode: http.StatusOK, NoAuthorize: true},
247248
}
248249

249250
// Routes like proxy routes support all HTTP methods. A helper func to expand

0 commit comments

Comments
 (0)