Skip to content

Commit d23edf8

Browse files
coadlercoadler
committed
Merge branch 'main' into colin/auditv2
2 parents b3776e0 + f037aad commit d23edf8

Some content is hidden

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

45 files changed

+1540
-75
lines changed

‎.envrc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
use nix

‎cli/configssh.go

Lines changed: 72 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -170,10 +170,20 @@ func configSSH() *cobra.Command {
170170
// that it's possible to capture the diff.
171171
out = cmd.OutOrStderr()
172172
}
173-
binaryFile, err := currentBinPath(out)
173+
coderBinary, err := currentBinPath(out)
174174
if err != nil {
175175
return err
176176
}
177+
escapedCoderBinary, err := sshConfigExecEscape(coderBinary)
178+
if err != nil {
179+
return xerrors.Errorf("escape coder binary for ssh failed: %w", err)
180+
}
181+
182+
root := createConfig(cmd)
183+
escapedGlobalConfig, err := sshConfigExecEscape(string(root))
184+
if err != nil {
185+
return xerrors.Errorf("escape global config for ssh failed: %w", err)
186+
}
177187

178188
homedir, err := os.UserHomeDir()
179189
if err != nil {
@@ -238,7 +248,6 @@ func configSSH() *cobra.Command {
238248
}
239249

240250
configModified := configRaw
241-
root := createConfig(cmd)
242251

243252
buf := &bytes.Buffer{}
244253
before, after := sshConfigSplitOnCoderSection(configModified)
@@ -280,11 +289,17 @@ func configSSH() *cobra.Command {
280289
"\tLogLevel ERROR",
281290
)
282291
if !skipProxyCommand {
283-
if !wireguard {
284-
configOptions = append(configOptions, fmt.Sprintf("\tProxyCommand %q --global-config %q ssh --stdio %s", binaryFile, root, hostname))
285-
} else {
286-
configOptions = append(configOptions, fmt.Sprintf("\tProxyCommand %q --global-config %q ssh --wireguard --stdio %s", binaryFile, root, hostname))
292+
wgArg := ""
293+
if wireguard {
294+
wgArg = "--wireguard "
287295
}
296+
configOptions = append(
297+
configOptions,
298+
fmt.Sprintf(
299+
"\tProxyCommand %s --global-config %s ssh %s--stdio %s",
300+
escapedCoderBinary, escapedGlobalConfig, wgArg, hostname,
301+
),
302+
)
288303
}
289304

290305
_, _ = buf.WriteString(strings.Join(configOptions, "\n"))
@@ -451,6 +466,11 @@ func writeWithTempFileAndMove(path string, r io.Reader) (err error) {
451466
dir := filepath.Dir(path)
452467
name := filepath.Base(path)
453468

469+
// Ensure that e.g. the ~/.ssh directory exists.
470+
if err = os.MkdirAll(dir, 0o700); err != nil {
471+
return xerrors.Errorf("create directory: %w", err)
472+
}
473+
454474
// Create a tempfile in the same directory for ensuring write
455475
// operation does not fail.
456476
f, err := os.CreateTemp(dir, fmt.Sprintf(".%s.", name))
@@ -482,6 +502,52 @@ func writeWithTempFileAndMove(path string, r io.Reader) (err error) {
482502
return nil
483503
}
484504

505+
// sshConfigExecEscape quotes the string if it contains spaces, as per
506+
// `man 5 ssh_config`. However, OpenSSH uses exec in the users shell to
507+
// run the command, and as such the formatting/escape requirements
508+
// cannot simply be covered by `fmt.Sprintf("%q", path)`.
509+
//
510+
// Always escaping the path with `fmt.Sprintf("%q", path)` usually works
511+
// on most platforms, but double quotes sometimes break on Windows 10
512+
// (see #2853). This function takes a best-effort approach to improving
513+
// compatibility and covering edge cases.
514+
//
515+
// Given the following ProxyCommand:
516+
//
517+
// ProxyCommand "/path/with space/coder" ssh --stdio work
518+
//
519+
// This is ~what OpenSSH would execute:
520+
//
521+
// /bin/bash -c '"/path/with space/to/coder" ssh --stdio workspace'
522+
//
523+
// However, since it's actually an arg in C, the contents inside the
524+
// single quotes are interpreted as is, e.g. if there was a '\t', it
525+
// would be the literal string '\t', not a tab.
526+
//
527+
// See:
528+
// - https://github.com/coder/coder/issues/2853
529+
// - https://github.com/openssh/openssh-portable/blob/V_9_0_P1/sshconnect.c#L158-L167
530+
// - https://github.com/PowerShell/openssh-portable/blob/v8.1.0.0/sshconnect.c#L231-L293
531+
// - https://github.com/PowerShell/openssh-portable/blob/v8.1.0.0/contrib/win32/win32compat/w32fd.c#L1075-L1100
532+
func sshConfigExecEscape(path string) (string, error) {
533+
// This is unlikely to ever happen, but newlines are allowed on
534+
// certain filesystems, but cannot be used inside ssh config.
535+
if strings.ContainsAny(path, "\n") {
536+
return "", xerrors.Errorf("invalid path: %s", path)
537+
}
538+
// In the unlikely even that a path contains quotes, they must be
539+
// escaped so that they are not interpreted as shell quotes.
540+
if strings.Contains(path, "\"") {
541+
path = strings.ReplaceAll(path, "\"", "\\\"")
542+
}
543+
// A space or a tab requires quoting, but tabs must not be escaped
544+
// (\t) since OpenSSH interprets it as a literal \t, not a tab.
545+
if strings.ContainsAny(path, " \t") {
546+
path = fmt.Sprintf("\"%s\"", path) //nolint:gocritic // We don't want %q here.
547+
}
548+
return path, nil
549+
}
550+
485551
// currentBinPath returns the path to the coder binary suitable for use in ssh
486552
// ProxyCommand.
487553
func currentBinPath(w io.Writer) (string, error) {

‎cli/configssh_internal_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package cli
2+
3+
import (
4+
"os"
5+
"os/exec"
6+
"path/filepath"
7+
"runtime"
8+
"strings"
9+
"testing"
10+
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
// This test tries to mimic the behavior of OpenSSH
15+
// when executing e.g. a ProxyCommand.
16+
func Test_sshConfigExecEscape(t *testing.T) {
17+
t.Parallel()
18+
19+
tests := []struct {
20+
name string
21+
path string
22+
wantErr bool
23+
windows bool
24+
}{
25+
{"no spaces", "simple", false, true},
26+
{"spaces", "path with spaces", false, true},
27+
{"quotes", "path with \"quotes\"", false, false},
28+
{"backslashes", "path with \\backslashes", false, false},
29+
{"tabs", "path with \ttabs", false, false},
30+
{"newline fails", "path with \nnewline", true, false},
31+
}
32+
for _, tt := range tests {
33+
tt := tt
34+
t.Run(tt.name, func(t *testing.T) {
35+
t.Parallel()
36+
37+
if runtime.GOOS == "windows" {
38+
t.Skip("Windows doesn't typically execute via /bin/sh or cmd.exe, so this test is not applicable.")
39+
}
40+
41+
dir := filepath.Join(t.TempDir(), tt.path)
42+
err := os.MkdirAll(dir, 0o755)
43+
require.NoError(t, err)
44+
bin := filepath.Join(dir, "coder")
45+
contents := []byte("#!/bin/sh\necho yay\n")
46+
err = os.WriteFile(bin, contents, 0o755) //nolint:gosec
47+
require.NoError(t, err)
48+
49+
escaped, err := sshConfigExecEscape(bin)
50+
if tt.wantErr {
51+
require.Error(t, err)
52+
return
53+
}
54+
require.NoError(t, err)
55+
56+
b, err := exec.Command("/bin/sh", "-c", escaped).CombinedOutput() //nolint:gosec
57+
require.NoError(t, err)
58+
got := strings.TrimSpace(string(b))
59+
require.Equal(t, "yay", got)
60+
})
61+
}
62+
}

‎cli/features.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package cli
22

33
import (
4+
"bytes"
45
"encoding/json"
56
"fmt"
67
"strings"
@@ -53,12 +54,14 @@ func featuresList() *cobra.Command {
5354
return xerrors.Errorf("render table: %w", err)
5455
}
5556
case "json":
56-
outBytes, err := json.Marshal(entitlements)
57+
buf := new(bytes.Buffer)
58+
enc := json.NewEncoder(buf)
59+
enc.SetIndent("", " ")
60+
err = enc.Encode(entitlements)
5761
if err != nil {
5862
return xerrors.Errorf("marshal features to JSON: %w", err)
5963
}
60-
61-
out = string(outBytes)
64+
out = buf.String()
6265
default:
6366
return xerrors.Errorf(`unknown output format %q, only "table" and "json" are supported`, outputFormat)
6467
}

‎coderd/coderd.go

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
package coderd
22

33
import (
4-
"context"
54
"crypto/x509"
6-
"fmt"
75
"io"
86
"net/http"
97
"net/url"
@@ -68,6 +66,7 @@ type Options struct {
6866
TracerProvider *sdktrace.TracerProvider
6967
AutoImportTemplates []AutoImportTemplate
7068
LicenseHandler http.Handler
69+
FeaturesService FeaturesService
7170
}
7271

7372
// New constructs a Coder API handler.
@@ -97,6 +96,9 @@ func New(options *Options) *API {
9796
if options.LicenseHandler == nil {
9897
options.LicenseHandler = licenses()
9998
}
99+
if options.FeaturesService == nil {
100+
options.FeaturesService = featuresService{}
101+
}
100102

101103
siteCacheDir := options.CacheDir
102104
if siteCacheDir != "" {
@@ -125,11 +127,8 @@ func New(options *Options) *API {
125127
apiKeyMiddleware := httpmw.ExtractAPIKey(options.Database, oauthConfigs, false)
126128

127129
r.Use(
128-
func(next http.Handler) http.Handler {
129-
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
130-
next.ServeHTTP(middleware.NewWrapResponseWriter(w, r.ProtoMajor), r)
131-
})
132-
},
130+
httpmw.Recover(api.Logger),
131+
httpmw.Logger(api.Logger),
133132
httpmw.Prometheus(options.PrometheusRegistry),
134133
)
135134

@@ -159,7 +158,6 @@ func New(options *Options) *API {
159158
r.Use(
160159
// Specific routes can specify smaller limits.
161160
httpmw.RateLimitPerMinute(options.APIRateLimit),
162-
debugLogRequest(api.Logger),
163161
tracing.HTTPMW(api.TracerProvider, "coderd.http"),
164162
)
165163
r.Get("/", func(w http.ResponseWriter, r *http.Request) {
@@ -406,7 +404,7 @@ func New(options *Options) *API {
406404
})
407405
r.Route("/entitlements", func(r chi.Router) {
408406
r.Use(apiKeyMiddleware)
409-
r.Get("/", entitlements)
407+
r.Get("/", api.FeaturesService.EntitlementsAPI)
410408
})
411409
r.Route("/licenses", func(r chi.Router) {
412410
r.Use(apiKeyMiddleware)
@@ -438,15 +436,6 @@ func (api *API) Close() error {
438436
return api.workspaceAgentCache.Close()
439437
}
440438

441-
func debugLogRequest(log slog.Logger) func(http.Handler) http.Handler {
442-
return func(next http.Handler) http.Handler {
443-
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
444-
log.Debug(context.Background(), fmt.Sprintf("%s %s", r.Method, r.URL.Path))
445-
next.ServeHTTP(rw, r)
446-
})
447-
}
448-
}
449-
450439
func compressHandler(h http.Handler) http.Handler {
451440
cmp := middleware.NewCompressor(5,
452441
"text/*",

‎coderd/database/databasefake/databasefake.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,19 @@ func (q *fakeQuerier) GetUserCount(_ context.Context) (int64, error) {
246246
return int64(len(q.users)), nil
247247
}
248248

249+
func (q *fakeQuerier) GetActiveUserCount(_ context.Context) (int64, error) {
250+
q.mutex.RLock()
251+
defer q.mutex.RUnlock()
252+
253+
active := int64(0)
254+
for _, u := range q.users {
255+
if u.Status == database.UserStatusActive {
256+
active++
257+
}
258+
}
259+
return active, nil
260+
}
261+
249262
func (q *fakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams) ([]database.User, error) {
250263
q.mutex.RLock()
251264
defer q.mutex.RUnlock()
@@ -2322,6 +2335,21 @@ func (q *fakeQuerier) GetLicenses(_ context.Context) ([]database.License, error)
23222335
return results, nil
23232336
}
23242337

2338+
func (q *fakeQuerier) GetUnexpiredLicenses(_ context.Context) ([]database.License, error) {
2339+
q.mutex.RLock()
2340+
defer q.mutex.RUnlock()
2341+
2342+
now := time.Now()
2343+
var results []database.License
2344+
for _, l := range q.licenses {
2345+
if l.Exp.After(now) {
2346+
results = append(results, l)
2347+
}
2348+
}
2349+
sort.Slice(results, func(i, j int) bool { return results[i].ID < results[j].ID })
2350+
return results, nil
2351+
}
2352+
23252353
func (q *fakeQuerier) DeleteLicense(_ context.Context, id int32) (int32, error) {
23262354
q.mutex.Lock()
23272355
defer q.mutex.Unlock()

‎coderd/database/querier.go

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)