Skip to content

Commit e71ba85

Browse files
mafredrimafredri
committed
Merge branch 'main' into mafredri/refactor-agent-sshd
2 parents 94d7593 + 2da0702 commit e71ba85

File tree

98 files changed

+3704
-1833
lines changed

Some content is hidden

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

98 files changed

+3704
-1833
lines changed

‎.github/workflows/security.yaml

+3
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,9 @@ jobs:
9797
restore-keys: |
9898
js-${{ runner.os }}-
9999
100+
- name: Install sqlc
101+
run: |
102+
curl -sSL https://github.com/kyleconroy/sqlc/releases/download/v1.17.2/sqlc_1.17.2_linux_amd64.tar.gz | sudo tar -C /usr/bin -xz sqlc
100103
- name: Install yq
101104
run: go run github.com/mikefarah/yq/v4@v4.30.6
102105
- name: Install protoc-gen-go

‎cli/agent.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -336,8 +336,8 @@ func urlPort(u string) (int, error) {
336336
return -1, xerrors.Errorf("invalid url %q: %w", u, err)
337337
}
338338
if parsed.Port() != "" {
339-
port, err := strconv.ParseInt(parsed.Port(), 10, 64)
340-
if err == nil && port > 0 {
339+
port, err := strconv.ParseUint(parsed.Port(), 10, 16)
340+
if err == nil && port > 0 && port < 1<<16 {
341341
return int(port), nil
342342
}
343343
}

‎cli/cliui/output.go

+21
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package cliui
33
import (
44
"context"
55
"encoding/json"
6+
"fmt"
67
"reflect"
78
"strings"
89

@@ -171,3 +172,23 @@ func (jsonFormat) Format(_ context.Context, data any) (string, error) {
171172

172173
return string(outBytes), nil
173174
}
175+
176+
type textFormat struct{}
177+
178+
var _ OutputFormat = textFormat{}
179+
180+
// TextFormat is a formatter that just outputs unstructured text.
181+
// It uses fmt.Sprintf under the hood.
182+
func TextFormat() OutputFormat {
183+
return textFormat{}
184+
}
185+
186+
func (textFormat) ID() string {
187+
return "text"
188+
}
189+
190+
func (textFormat) AttachOptions(_ *clibase.OptionSet) {}
191+
192+
func (textFormat) Format(_ context.Context, data any) (string, error) {
193+
return fmt.Sprintf("%s", data), nil
194+
}

‎cli/cliui/output_test.go

+3
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ func Test_OutputFormatter(t *testing.T) {
5050
require.Panics(t, func() {
5151
cliui.NewOutputFormatter(cliui.JSONFormat())
5252
})
53+
require.NotPanics(t, func() {
54+
cliui.NewOutputFormatter(cliui.JSONFormat(), cliui.TextFormat())
55+
})
5356
})
5457

5558
t.Run("NoMissingFormatID", func(t *testing.T) {

‎cli/root.go

+1-31
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ func (r *RootCmd) Core() []*clibase.Cmd {
8282
r.templates(),
8383
r.users(),
8484
r.tokens(),
85-
r.version(),
85+
r.version(defaultVersionInfo),
8686

8787
// Workspace Commands
8888
r.configSSH(),
@@ -370,36 +370,6 @@ func LoggerFromContext(ctx context.Context) (slog.Logger, bool) {
370370
return l, ok
371371
}
372372

373-
// version prints the coder version
374-
func (*RootCmd) version() *clibase.Cmd {
375-
return &clibase.Cmd{
376-
Use: "version",
377-
Short: "Show coder version",
378-
Handler: func(inv *clibase.Invocation) error {
379-
var str strings.Builder
380-
_, _ = str.WriteString("Coder ")
381-
if buildinfo.IsAGPL() {
382-
_, _ = str.WriteString("(AGPL) ")
383-
}
384-
_, _ = str.WriteString(buildinfo.Version())
385-
buildTime, valid := buildinfo.Time()
386-
if valid {
387-
_, _ = str.WriteString(" " + buildTime.Format(time.UnixDate))
388-
}
389-
_, _ = str.WriteString("\r\n" + buildinfo.ExternalURL() + "\r\n\r\n")
390-
391-
if buildinfo.IsSlim() {
392-
_, _ = str.WriteString(fmt.Sprintf("Slim build of Coder, does not support the %s subcommand.\n", cliui.Styles.Code.Render("server")))
393-
} else {
394-
_, _ = str.WriteString(fmt.Sprintf("Full build of Coder, supports the %s subcommand.\n", cliui.Styles.Code.Render("server")))
395-
}
396-
397-
_, _ = fmt.Fprint(inv.Stdout, str.String())
398-
return nil
399-
},
400-
}
401-
}
402-
403373
func isTest() bool {
404374
return flag.Lookup("test.v") != nil
405375
}

‎cli/server.go

+22-15
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ import (
7878
"github.com/coder/coder/coderd/tracing"
7979
"github.com/coder/coder/coderd/updatecheck"
8080
"github.com/coder/coder/coderd/util/slice"
81+
"github.com/coder/coder/coderd/workspaceapps"
8182
"github.com/coder/coder/codersdk"
8283
"github.com/coder/coder/cryptorand"
8384
"github.com/coder/coder/provisioner/echo"
@@ -731,6 +732,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
731732
UsernameField: cfg.OIDC.UsernameField.String(),
732733
EmailField: cfg.OIDC.EmailField.String(),
733734
AuthURLParams: cfg.OIDC.AuthURLParams.Value,
735+
IgnoreUserInfo: cfg.OIDC.IgnoreUserInfo.Value(),
734736
GroupField: cfg.OIDC.GroupField.String(),
735737
GroupMapping: cfg.OIDC.GroupMapping.Value,
736738
SignInText: cfg.OIDC.SignInText.String(),
@@ -780,37 +782,42 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
780782
}
781783
}
782784

783-
// Read the app signing key from the DB. We store it hex
784-
// encoded since the config table uses strings for the value and
785-
// we don't want to deal with automatic encoding issues.
786-
appSigningKeyStr, err := tx.GetAppSigningKey(ctx)
785+
// Read the app signing key from the DB. We store it hex encoded
786+
// since the config table uses strings for the value and we
787+
// don't want to deal with automatic encoding issues.
788+
appSecurityKeyStr, err := tx.GetAppSecurityKey(ctx)
787789
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
788790
return xerrors.Errorf("get app signing key: %w", err)
789791
}
790-
if appSigningKeyStr == "" {
791-
// Generate 64 byte secure random string.
792-
b := make([]byte, 64)
792+
// If the string in the DB is an invalid hex string or the
793+
// length is not equal to the current key length, generate a new
794+
// one.
795+
//
796+
// If the key is regenerated, old signed tokens and encrypted
797+
// strings will become invalid. New signed app tokens will be
798+
// generated automatically on failure. Any workspace app token
799+
// smuggling operations in progress may fail, although with a
800+
// helpful error.
801+
if decoded, err := hex.DecodeString(appSecurityKeyStr); err != nil || len(decoded) != len(workspaceapps.SecurityKey{}) {
802+
b := make([]byte, len(workspaceapps.SecurityKey{}))
793803
_, err := rand.Read(b)
794804
if err != nil {
795805
return xerrors.Errorf("generate fresh app signing key: %w", err)
796806
}
797807

798-
appSigningKeyStr = hex.EncodeToString(b)
799-
err = tx.InsertAppSigningKey(ctx, appSigningKeyStr)
808+
appSecurityKeyStr = hex.EncodeToString(b)
809+
err = tx.UpsertAppSecurityKey(ctx, appSecurityKeyStr)
800810
if err != nil {
801811
return xerrors.Errorf("insert freshly generated app signing key to database: %w", err)
802812
}
803813
}
804814

805-
appSigningKey, err := hex.DecodeString(appSigningKeyStr)
815+
appSecurityKey, err := workspaceapps.KeyFromString(appSecurityKeyStr)
806816
if err != nil {
807-
return xerrors.Errorf("decode app signing key from database as hex: %w", err)
808-
}
809-
if len(appSigningKey) != 64 {
810-
return xerrors.Errorf("app signing key must be 64 bytes, key in database is %d bytes", len(appSigningKey))
817+
return xerrors.Errorf("decode app signing key from database: %w", err)
811818
}
812819

813-
options.AppSigningKey = appSigningKey
820+
options.AppSecurityKey = appSecurityKey
814821
return nil
815822
}, nil)
816823
if err != nil {

‎cli/server_test.go

+13-7
Original file line numberDiff line numberDiff line change
@@ -668,8 +668,7 @@ func TestServer(t *testing.T) {
668668
if c.tlsListener {
669669
accessURLParsed, err := url.Parse(c.requestURL)
670670
require.NoError(t, err)
671-
client := codersdk.New(accessURLParsed)
672-
client.HTTPClient = &http.Client{
671+
client := &http.Client{
673672
CheckRedirect: func(req *http.Request, via []*http.Request) error {
674673
return http.ErrUseLastResponse
675674
},
@@ -682,11 +681,15 @@ func TestServer(t *testing.T) {
682681
},
683682
},
684683
}
685-
defer client.HTTPClient.CloseIdleConnections()
686-
_, err = client.HasFirstUser(ctx)
687-
if err != nil {
688-
require.ErrorContains(t, err, "Invalid application URL")
689-
}
684+
defer client.CloseIdleConnections()
685+
686+
req, err := http.NewRequestWithContext(ctx, http.MethodGet, accessURLParsed.String(), nil)
687+
require.NoError(t, err)
688+
resp, err := client.Do(req)
689+
// We don't care much about the response, just that TLS
690+
// worked.
691+
require.NoError(t, err)
692+
defer resp.Body.Close()
690693
}
691694
})
692695
}
@@ -1086,6 +1089,7 @@ func TestServer(t *testing.T) {
10861089
require.Equal(t, "preferred_username", deploymentConfig.Values.OIDC.UsernameField.Value())
10871090
require.Equal(t, "email", deploymentConfig.Values.OIDC.EmailField.Value())
10881091
require.Equal(t, map[string]string{"access_type": "offline"}, deploymentConfig.Values.OIDC.AuthURLParams.Value)
1092+
require.False(t, deploymentConfig.Values.OIDC.IgnoreUserInfo.Value())
10891093
require.Empty(t, deploymentConfig.Values.OIDC.GroupField.Value())
10901094
require.Empty(t, deploymentConfig.Values.OIDC.GroupMapping.Value)
10911095
require.Equal(t, "OpenID Connect", deploymentConfig.Values.OIDC.SignInText.Value())
@@ -1125,6 +1129,7 @@ func TestServer(t *testing.T) {
11251129
"--oidc-username-field", "not_preferred_username",
11261130
"--oidc-email-field", "not_email",
11271131
"--oidc-auth-url-params", `{"prompt":"consent"}`,
1132+
"--oidc-ignore-userinfo",
11281133
"--oidc-group-field", "serious_business_unit",
11291134
"--oidc-group-mapping", `{"serious_business_unit": "serious_business_unit"}`,
11301135
"--oidc-sign-in-text", "Sign In With Coder",
@@ -1169,6 +1174,7 @@ func TestServer(t *testing.T) {
11691174
require.True(t, deploymentConfig.Values.OIDC.IgnoreEmailVerified.Value())
11701175
require.Equal(t, "not_preferred_username", deploymentConfig.Values.OIDC.UsernameField.Value())
11711176
require.Equal(t, "not_email", deploymentConfig.Values.OIDC.EmailField.Value())
1177+
require.True(t, deploymentConfig.Values.OIDC.IgnoreUserInfo.Value())
11721178
require.Equal(t, map[string]string{"prompt": "consent"}, deploymentConfig.Values.OIDC.AuthURLParams.Value)
11731179
require.Equal(t, "serious_business_unit", deploymentConfig.Values.OIDC.GroupField.Value())
11741180
require.Equal(t, map[string]string{"serious_business_unit": "serious_business_unit"}, deploymentConfig.Values.OIDC.GroupMapping.Value)

‎cli/testdata/coder_server_--help.golden

+5-4
Original file line numberDiff line numberDiff line change
@@ -280,13 +280,17 @@ can safely ignore these settings.
280280
Change the OIDC default 'groups' claim field. By default, will be
281281
'groups' if present in the oidc scopes argument.
282282

283-
--oidc-group-mapping struct[map[string]string], $OIDC_GROUP_MAPPING (default: {})
283+
--oidc-group-mapping struct[map[string]string], $CODER_OIDC_GROUP_MAPPING (default: {})
284284
A map of OIDC group IDs and the group in Coder it should map to. This
285285
is useful for when OIDC providers only return group IDs.
286286

287287
--oidc-ignore-email-verified bool, $CODER_OIDC_IGNORE_EMAIL_VERIFIED
288288
Ignore the email_verified claim from the upstream provider.
289289

290+
--oidc-ignore-userinfo bool, $CODER_OIDC_IGNORE_USERINFO (default: false)
291+
Ignore the userinfo endpoint and only use the ID token for user
292+
information.
293+
290294
--oidc-issuer-url string, $CODER_OIDC_ISSUER_URL
291295
Issuer URL to use for Login with OIDC.
292296

@@ -353,9 +357,6 @@ telemetrywhen required by your organization's security policy.
353357
Enterprise Options
354358
These options are only available in the Enterprise Edition.
355359

356-
--audit-logging bool, $CODER_AUDIT_LOGGING (default: true)
357-
Specifies whether audit logging is enabled.
358-
359360
--browser-only bool, $CODER_BROWSER_ONLY
360361
Whether Coder only allows connections to workspaces via the browser.
361362

‎cli/testdata/coder_version_--help.golden

+5-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
1-
Usage: coder version
1+
Usage: coder version [flags]
22

33
Show coder version
44

5+
Options
6+
-o, --output string (default: text)
7+
Output format. Available formats: text, json.
8+
59
---
610
Run `coder --help` for a list of global options.

‎cli/version.go

+84
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
package cli
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
"time"
7+
8+
"github.com/coder/coder/buildinfo"
9+
"github.com/coder/coder/cli/clibase"
10+
"github.com/coder/coder/cli/cliui"
11+
)
12+
13+
// versionInfo wraps the stuff we get from buildinfo so that it's
14+
// easier to emit in different formats.
15+
type versionInfo struct {
16+
Version string `json:"version"`
17+
BuildTime time.Time `json:"build_time"`
18+
ExternalURL string `json:"external_url"`
19+
Slim bool `json:"slim"`
20+
AGPL bool `json:"agpl"`
21+
}
22+
23+
// String() implements Stringer
24+
func (vi versionInfo) String() string {
25+
var str strings.Builder
26+
_, _ = str.WriteString("Coder ")
27+
if vi.AGPL {
28+
_, _ = str.WriteString("(AGPL) ")
29+
}
30+
_, _ = str.WriteString(vi.Version)
31+
32+
if !vi.BuildTime.IsZero() {
33+
_, _ = str.WriteString(" " + vi.BuildTime.Format(time.UnixDate))
34+
}
35+
_, _ = str.WriteString("\r\n" + vi.ExternalURL + "\r\n\r\n")
36+
37+
if vi.Slim {
38+
_, _ = str.WriteString(fmt.Sprintf("Slim build of Coder, does not support the %s subcommand.", cliui.Styles.Code.Render("server")))
39+
} else {
40+
_, _ = str.WriteString(fmt.Sprintf("Full build of Coder, supports the %s subcommand.", cliui.Styles.Code.Render("server")))
41+
}
42+
return str.String()
43+
}
44+
45+
func defaultVersionInfo() *versionInfo {
46+
buildTime, _ := buildinfo.Time()
47+
return &versionInfo{
48+
Version: buildinfo.Version(),
49+
BuildTime: buildTime,
50+
ExternalURL: buildinfo.ExternalURL(),
51+
Slim: buildinfo.IsSlim(),
52+
AGPL: buildinfo.IsAGPL(),
53+
}
54+
}
55+
56+
// version prints the coder version
57+
func (*RootCmd) version(versionInfo func() *versionInfo) *clibase.Cmd {
58+
var (
59+
formatter = cliui.NewOutputFormatter(
60+
cliui.TextFormat(),
61+
cliui.JSONFormat(),
62+
)
63+
vi = versionInfo()
64+
)
65+
66+
cmd := &clibase.Cmd{
67+
Use: "version",
68+
Short: "Show coder version",
69+
Options: clibase.OptionSet{},
70+
Handler: func(inv *clibase.Invocation) error {
71+
out, err := formatter.Format(inv.Context(), vi)
72+
if err != nil {
73+
return err
74+
}
75+
76+
_, err = fmt.Fprintln(inv.Stdout, out)
77+
return err
78+
},
79+
}
80+
81+
formatter.AttachOptions(&cmd.Options)
82+
83+
return cmd
84+
}

0 commit comments

Comments
 (0)