diff --git a/.github/workflows/linter.yml b/.github/workflows/linter.yml index fee3f6e..c1f3607 100644 --- a/.github/workflows/linter.yml +++ b/.github/workflows/linter.yml @@ -14,4 +14,4 @@ jobs: uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 # v8.0.0 continue-on-error: false with: - version: v2.9.0 + version: v2.13.2 diff --git a/.golangci.yml b/.golangci.yml index 6e3e8a3..36d555c 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -11,6 +11,8 @@ linters: # Highly annoying. We'd need to whitelist all packages we import, which is a lot of work and adds very little value. - depguard # Annoying and unhelpful. Assumes uninitialized (zero-valued) fields are always a bug, which is plain wrong. + - exhaustruct_v5 + # Deprecated alias of exhaustruct_v5; keep it off too so it doesn't emit a deprecation warning. - exhaustruct # Marks [TODO, FIXME, BUG] comments as errors. We use these, so this is not helpful - unless we decide this is a good policy. - godox @@ -40,6 +42,8 @@ linters: - nilnil # Deprecated alias of wsl_v5; keep it off so only wsl_v5 runs (avoids a deprecation warning). - wsl + # Deprecated alias of gomodguard_v2; keep it off so only gomodguard_v2 runs (avoids a deprecation warning). + - gomodguard settings: ireturn: allow: diff --git a/clerk/clerk.go b/clerk/clerk.go index 9a55cb8..69578ce 100644 --- a/clerk/clerk.go +++ b/clerk/clerk.go @@ -202,6 +202,7 @@ func FetchJwt(ctx context.Context) (string, error) { //nolint:funlen,cyclop Domain: GetClerkDomain(), Secure: true, HttpOnly: true, + SameSite: http.SameSiteStrictMode, }) } diff --git a/cmd/deploy.go b/cmd/deploy.go index 0af5b5f..ed0aecb 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -359,8 +359,8 @@ func formatGlobalPromptMessage(integrations []integrationRemovedObjectsInfo) str for _, info := range integrations { objectList := strings.Join(info.removedObjects, ", ") installationWord := pluralizer.Pluralize("installation", info.installationCount, false) - lines.WriteString(fmt.Sprintf(" • %s: %s (%d %s)\n", - info.integrationName, objectList, info.installationCount, installationWord)) + fmt.Fprintf(&lines, " • %s: %s (%d %s)\n", + info.integrationName, objectList, info.installationCount, installationWord) } message += lines.String() @@ -378,11 +378,11 @@ func formatGlobalPromptMessage(integrations []integrationRemovedObjectsInfo) str func formatAffectedInstallations(groups []groupInfo, totalCount int) string { var result strings.Builder for _, g := range groups { - result.WriteString(fmt.Sprintf("\n - %s (%s)", g.name, g.ref)) + fmt.Fprintf(&result, "\n - %s (%s)", g.name, g.ref) } if totalCount > len(groups) { - result.WriteString(fmt.Sprintf("\n - and %d more", totalCount-len(groups))) + fmt.Fprintf(&result, "\n - and %d more", totalCount-len(groups)) } return result.String() diff --git a/cmd/login.go b/cmd/login.go index f71d34b..7bcdff6 100644 --- a/cmd/login.go +++ b/cmd/login.go @@ -55,8 +55,12 @@ func (h *handler) ServeHTTP(writer http.ResponseWriter, request *http.Request) { writer.WriteHeader(http.StatusOK) + // rsp is the login-success page rendered by clerk.getHTML, whose only + // interpolation is mustache's {{email}} -- the escaping form, so the claim + // value cannot inject markup. gosec's taint analysis cannot see through the + // template engine. // nosemgrep: go.lang.security.audit.xss.no-direct-write-to-responsewriter.no-direct-write-to-responsewriter - _, _ = writer.Write([]byte(rsp)) + _, _ = writer.Write([]byte(rsp)) //nolint:gosec // G705: template-escaped, see above go func() { // Tell the user we're done and then forcefully exit the program. @@ -87,7 +91,10 @@ func processLogin(ctx context.Context, payload []byte, write bool) (string, stri path := clerk.GetJwtPath() if write { - err := os.WriteFile(path, pretty.Pretty(payload), JwtFilePermissions) + // path is the XDG config path for this stage (clerk.GetJwtPath). The only + // caller-influenced part is AMP_STAGE_OVERRIDE, an env var the user sets for + // themselves on their own machine, so there is no cross-trust-boundary taint. + err := os.WriteFile(path, pretty.Pretty(payload), JwtFilePermissions) //nolint:gosec // G703: user's own config path if err != nil { return "", "", err } diff --git a/cmd/trigger.go b/cmd/trigger.go index 443db71..7cffef1 100644 --- a/cmd/trigger.go +++ b/cmd/trigger.go @@ -140,7 +140,7 @@ func openInEditor(ctx context.Context, data []byte) ([]byte, error) { // vi/notepad fallback) and runs locally as the invoking user, so there is // no untrusted input and no injection surface here. // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command - cmd := exec.CommandContext(ctx, editor, tmpFile.Name()) + cmd := exec.CommandContext(ctx, editor, tmpFile.Name()) //nolint:gosec // G702: user's own $EDITOR, see above cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr diff --git a/files/manifest_test.go b/files/manifest_test.go index de5ef5e..59a11ed 100644 --- a/files/manifest_test.go +++ b/files/manifest_test.go @@ -6,6 +6,11 @@ import ( "github.com/amp-labs/cli/openapi" ) +const ( + objAccounts = "accounts" + objContacts = "contacts" +) + func TestGetRemovedReadObjects(t *testing.T) { t.Parallel() @@ -21,15 +26,15 @@ func TestGetRemovedReadObjects(t *testing.T) { Read: &openapi.IntegrationRead{ Objects: &[]openapi.IntegrationObject{ {ObjectName: "Accounts"}, - {ObjectName: "contacts"}, + {ObjectName: objContacts}, }, }, }, newInteg: &openapi.Integration{ Read: &openapi.IntegrationRead{ Objects: &[]openapi.IntegrationObject{ - {ObjectName: "accounts"}, - {ObjectName: "contacts"}, + {ObjectName: objAccounts}, + {ObjectName: objContacts}, }, }, }, @@ -41,33 +46,33 @@ func TestGetRemovedReadObjects(t *testing.T) { Read: &openapi.IntegrationRead{ Objects: &[]openapi.IntegrationObject{ {ObjectName: "AccounTs"}, - {ObjectName: "contacts"}, + {ObjectName: objContacts}, }, }, }, newInteg: &openapi.Integration{ Read: &openapi.IntegrationRead{ Objects: &[]openapi.IntegrationObject{ - {ObjectName: "accounts"}, + {ObjectName: objAccounts}, }, }, }, - want: []string{"contacts"}, + want: []string{objContacts}, }, { name: "all objects removed", oldRevision: &openapi.Integration{ Read: &openapi.IntegrationRead{ Objects: &[]openapi.IntegrationObject{ - {ObjectName: "accounts"}, - {ObjectName: "contacts"}, + {ObjectName: objAccounts}, + {ObjectName: objContacts}, }, }, }, newInteg: &openapi.Integration{ Read: nil, }, - want: []string{"accounts", "contacts"}, + want: []string{objAccounts, objContacts}, }, { name: "no old read config", @@ -75,7 +80,7 @@ func TestGetRemovedReadObjects(t *testing.T) { newInteg: &openapi.Integration{ Read: &openapi.IntegrationRead{ Objects: &[]openapi.IntegrationObject{ - {ObjectName: "accounts"}, + {ObjectName: objAccounts}, }, }, }, @@ -87,7 +92,7 @@ func TestGetRemovedReadObjects(t *testing.T) { newInteg: &openapi.Integration{ Read: &openapi.IntegrationRead{ Objects: &[]openapi.IntegrationObject{ - {ObjectName: "accounts"}, + {ObjectName: objAccounts}, }, }, }, diff --git a/go.mod b/go.mod index e85936f..421984c 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/amp-labs/cli -go 1.26.4 +go 1.27.1 require ( github.com/adrg/xdg v0.5.3 @@ -18,13 +18,6 @@ require ( sigs.k8s.io/yaml v1.6.0 ) -require ( - github.com/inconshreveable/mousetrap v1.1.0 // indirect - github.com/spf13/pflag v1.0.10 // indirect - golang.org/x/sys v0.47.0 // indirect - golang.org/x/text v0.40.0 // indirect -) - require ( github.com/apapsch/go-jsonmerge/v2 v2.0.0 // indirect github.com/chzyer/readline v1.5.1 // indirect @@ -34,15 +27,19 @@ require ( github.com/go-viper/mapstructure/v2 v2.5.0 // indirect github.com/google/go-cmp v0.7.0 // indirect github.com/google/uuid v1.6.0 // indirect + github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/pelletier/go-toml/v2 v2.4.3 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/rogpeppe/go-internal v1.14.1 // indirect github.com/sagikazarmark/locafero v0.12.0 // indirect github.com/spf13/afero v1.15.0 // indirect github.com/spf13/cast v1.10.0 // indirect + github.com/spf13/pflag v1.0.10 // indirect github.com/subosito/gotenv v1.6.0 // indirect go.yaml.in/yaml/v2 v2.4.4 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect golang.org/x/crypto v0.54.0 // indirect + golang.org/x/sys v0.47.0 // indirect + golang.org/x/text v0.40.0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect ) diff --git a/request/request.go b/request/request.go index 86935ac..9793988 100644 --- a/request/request.go +++ b/request/request.go @@ -16,7 +16,12 @@ import ( "github.com/amp-labs/cli/utils" ) -const clientName = "amp-cli" +const ( + clientName = "amp-cli" + + headerContentType = "Content-Type" + mimeApplicationJSON = "application/json" +) type Client struct { Client *http.Client @@ -209,7 +214,7 @@ func (c *Client) makeRequestAndParseJSONResult(req *http.Request, result any) (* } if res.StatusCode < 200 || res.StatusCode > 299 { //nolint:nestif - ct := res.Header.Get("Content-Type") + ct := res.Header.Get(headerContentType) if len(ct) > 0 { mt, _, err := mime.ParseMediaType(ct) if err == nil { @@ -291,7 +296,7 @@ func makeJSONPatchRequest(ctx context.Context, url string, headers []Header, bod addDebugHeader(req) - headers = append(headers, Header{Key: "Content-Type", Value: "application/json"}) + headers = append(headers, Header{Key: headerContentType, Value: mimeApplicationJSON}) req.ContentLength = int64(len(jBody)) return addAcceptJSONHeaders(req, headers) @@ -310,7 +315,7 @@ func makeJSONPostRequest(ctx context.Context, url string, headers []Header, body addDebugHeader(req) - headers = append(headers, Header{Key: "Content-Type", Value: "application/json"}) + headers = append(headers, Header{Key: headerContentType, Value: mimeApplicationJSON}) req.ContentLength = int64(len(jBody)) return addAcceptJSONHeaders(req, headers) @@ -329,7 +334,7 @@ func makeJSONPutRequest(ctx context.Context, url string, headers []Header, body addDebugHeader(req) - headers = append(headers, Header{Key: "Content-Type", Value: "application/json"}) + headers = append(headers, Header{Key: headerContentType, Value: mimeApplicationJSON}) req.ContentLength = int64(len(jBody)) return addAcceptJSONHeaders(req, headers) @@ -359,7 +364,7 @@ func addHeaders(req *http.Request, headers []Header) *http.Request { func addAcceptJSONHeaders(req *http.Request, headers []Header) (*http.Request, error) { // Request JSON - req.Header.Add("Accept", "application/json") + req.Header.Add("Accept", mimeApplicationJSON) // Apply any custom headers for _, hdr := range headers { diff --git a/utils/utils.go b/utils/utils.go index 918e67e..065f6a1 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -46,8 +46,7 @@ func ReadStruct(r io.Reader, out any) (Format, error) { // A JSON syntax error means the data may still be YAML, so fall through. Any other // error means the data is JSON-shaped but invalid (e.g. a type mismatch); report it. - var se *json.SyntaxError - if !errors.As(err, &se) { + if _, ok := errors.AsType[*json.SyntaxError](err); !ok { return Unknown, err }