diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml new file mode 100644 index 00000000..4bd95814 --- /dev/null +++ b/.github/workflows/lint.yml @@ -0,0 +1,43 @@ +name: golangci-lint +on: + push: + branches: + - main + pull_request: + +permissions: + contents: read + checks: write + +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: actions/setup-go@v4 + with: + go-version: '1.22' + cache: false + - name: lint go post-processor + uses: golangci/golangci-lint-action@v3 + with: + version: v1.56 + args: post-processors/go/main.go + - name: lint csharp post-processor + uses: golangci/golangci-lint-action@v3 + with: + version: v1.56 + args: post-processors/csharp/main.go + - name: lint schema tooling + uses: golangci/golangci-lint-action@v3 + with: + version: v1.56 + args: schemas/main.go + + # Optional: golangci-lint command line arguments. + # + # Note: By default, the `.golangci.yml` file should be at the root of the repository. + # The location of the configuration file can be changed by using `--config=` + # args: --timeout=30m --config=/my/path/.golangci.yml --issues-exit-code=0 + diff --git a/.golangci.toml b/.golangci.toml new file mode 100644 index 00000000..e80be305 --- /dev/null +++ b/.golangci.toml @@ -0,0 +1,213 @@ +[linters] + + # set all to disabled so we can manually enable only the ones we want + disable-all = true + + # This is a list is just the linters that are critical to run and pass on + # our code. These linters find bugs. If they find a problem with your code, + # you should fix it immediately. + # + # Take care when adding to this list, as any code that fails these linters + # anywhere in any repo that uses this linter will need to be fixed before + # that repo can updating the version of go-linter it's using. + enable = [ + "bodyclose", # checks whether HTTP response body is closed successfully + # Forgetting to close an HTTP body can be a memory leak + "durationcheck", # check for two durations multiplied together + # this is probably a rare bug, but should have basically zero false positives. + "exportloopref", # catch bugs resulting from referencing variables on range scope + # variables initialized in for loops change with each loop, which can cause bugs. + "gocritic", # Provides many diagnostics that check for bugs, performance and style issues. + # This is highly configurable, see the gocritic config section below. + "gosec", # Inspects source code for security problems + # high quality linter that finds real bugs + "govet", # reports suspicious constructs like printf calls that don't have the right # of arguments + # high quality, low false positives + "ineffassign", # Detects when assignments to existing variables are not used + # this finds bugs all the time, where you assign to a value but then never use + # the assigned value due to shadowing etc. + "nilerr", # Finds the code that returns nil even if it checks that the error is not nil. + # finds fairly common bug + "rowserrcheck", # checks whether Err of rows is checked successfully + # finds bugs in SQL code + "sqlclosecheck", # Checks that sql.Rows and sql.Stmt are closed. + # easy and finds bugs + ] + + # We manually enable only the linters we want, above, so we don't need a + # manual disable list as well. + disable = [] + +[run] + # options for analysis running + # Increase timeout from default 1m, first pre-cache run can take a bit in CI/CD + timeout = "5m" + + # default concurrency is the available CPU number + # concurrency = 4 + + # exit code when at least one issue was found, default is 1 + issues-exit-code = 1 + + # include test files or not, default is true + tests = true + + # list of build tags, all linters use it. Default is empty list. + build-tags = [] + + # which dirs to skip: issues from them won't be reported; + # can use regexp here: generated.*, regexp is applied on full path; + # default value is empty list, but default dirs are skipped independently + # from this option's value (see skip-dirs-use-default). + # "/" will be replaced by current OS file path separator to properly work + # on Windows. + skip-dirs = [] + + # default is true. Enables skipping of directories: + # vendor$, third_party$, testdata$, examples$, Godeps$, builtin$ + skip-dirs-use-default = true + + # which files to skip: they will be analyzed, but issues from them + # won't be reported. Default value is empty list, but there is + # no need to include all autogenerated files, we confidently recognize + # autogenerated files. If it's not please let us know. + # "/" will be replaced by current OS file path separator to properly work + # on Windows. + + # skipping pkg/github/markdown/raw_request_builder.go's error of + # "G104: Errors unhandled. + # "requestInfo.SetContentFromScalar(ctx, m.BaseRequestBuilder.RequestAdapter, "text/plain", body)" + # because it happens due to generation and is not quickly fixed. + # 'exclude' cannot be used here because the error text applies to many possible errors + # and we don't want to ignore other instances of this error + skip-files = [ "pkg/github/markdown/raw_request_builder.go" ] + + # by default isn't set. If set we pass it to "go list -mod={option}". From "go help modules": + # If invoked with -mod=readonly, the go command is disallowed from the implicit + # automatic updating of go.mod described above. Instead, it fails when any changes + # to go.mod are needed. This setting is most useful to check that go.mod does + # not need updates, such as in a continuous integration and testing system. + # If invoked with -mod=vendor, the go command assumes that the vendor + # directory holds the correct copies of dependencies and ignores + # the dependency descriptions in go.mod. + modules-download-mode = "" + + # Allow multiple parallel golangci-lint instances running. + # If false (default) - golangci-lint acquires file lock on start. + allow-parallel-runners = false + +[output] + # colored-line-number|line-number|json|tab|checkstyle|code-climate|junit-xml|github-actions + # default is "colored-line-number" + format = "colored-line-number" + + # print lines of code with issue, default is true + print-issued-lines = true + + # print linter name in the end of issue text, default is true + print-linter-name = true + + # make issues output unique by line, default is true + uniq-by-line = true + + # add a prefix to the output file references; default is no prefix + path-prefix = "" + + # sorts results by: filepath, line and column + sort-results = true + +# options to enable differentiating between error and warning severities +[severity] + # GitHub Actions annotations support error and warning only: + # https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-commands-for-github-actions#setting-an-error-message + default-severity = "error" + + # If set to true severity-rules regular expressions become case sensitive. + # The default value is false. + case-sensitive = false + + # Default value is empty list. + # When a list of severity rules are provided, severity information will be added to lint + # issues. Severity rules have the same filtering capability as exclude rules except you + # are allowed to specify one matcher per severity rule. + # Only affects out formats that support setting severity information. + # [[severity.rules]] + # linters = [ + # "revive", + # ] + # severity = "warning" + +[issues] + # List of regexps of issue texts to exclude, empty list by default. + # Please document every exception here so we know what we're suppressing and why. + exclude = [] + + # Maximum issues count per one linter. Set to 0 to disable. Default is 50. + max-issues-per-linter = 0 + + # Maximum count of issues with the same text. Set to 0 to disable. Default is 3. + max-same-issues = 0 + + # The default value is false. If set to true exclude and exclude-rules + # regular expressions become case sensitive. + # exclude-case-sensitive = false + + # This flag suppresses lint issues from several linters, overriding any other configuration you have set. + # It defaults to true. + # NEVER remove this configuration. If you want to suppress something, do so explicitly elsewhere. + exclude-use-default = false + + # The list of ids of default excludes to include or disable. By default it's empty. + # We shouldn't ever need this, since we turn off default excludes. + include = [] + + # Show only new issues: if there are unstaged changes or untracked files, + # only those changes are analyzed, else only changes in HEAD~ are analyzed. + # It's a super-useful option for integration of golangci-lint into existing + # large codebase. It's not practical to fix all existing issues at the moment + # of integration: much better don't allow issues in new code. + # Default is false. + new = false + + # Show only new issues created in git patch with set file path. + # new-from-patch = "path/to/patch/file" + + # Show only new issues created after git revision `REV` + # new-from-rev = "REV" + + # Fix found issues (if it's supported by the linter). Default is false. + fix = false + + # reduce noise in some linters that don't necessarily need to be run in tests + [[issues.exclude-rules]] + path = "_test\\.go" + linters = ["errcheck", "gosec", "noctx", "bodyclose"] + +# +# Specific Linter Settings +# + +[linters-settings.gocritic] + # Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint run` to see all tags and checks. + # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags". + enabled-tags = [ + # "diagnostic", // enabled by default + ] + disabled-tags = ["style"] + disabled-checks = ["appendAssign"] + +[linters-settings.govet] + enable = [ "httpresponse" ] + +[linters-settings.gosec] + excludes = [ + "G110", # potential decompression bomb + "G204", # subprocess launched with variable. + "G301", # expect directory permissions to be 0750 or less. + "G302", # Expect file permissions to be 0600 or less. + "G304", # file inclusion via variable. + "G307", # deferring methods with errors. + "G404", # use of math/rand + ] +[linters-settings.gosec.config.G104] + os = ["Setenv"] \ No newline at end of file diff --git a/post-processors/csharp/main.go b/post-processors/csharp/main.go index 13a9fca8..948c3790 100644 --- a/post-processors/csharp/main.go +++ b/post-processors/csharp/main.go @@ -42,15 +42,14 @@ func run() error { fileContents = fixStringToBool(fileContents) } - if file.Name() == "WebhookConfigInsecureSsl.cs" { + if file.Name() == "WebhookConfigInsecureSsl.cs" { fileContents = fixUsing(fileContents) } fileContents = fixThumbsUpThumbsDownProperties(fileContents) fileContents = fixPagesPaths(fileContents) - // TODO(kfcampbell): verify file permission is what we want - err = os.WriteFile(path, []byte(fileContents), 0666) + err = os.WriteFile(path, []byte(fileContents), 0600) if err != nil { return err } diff --git a/post-processors/go/main.go b/post-processors/go/main.go index 2d3b3bb2..48e3d85a 100644 --- a/post-processors/go/main.go +++ b/post-processors/go/main.go @@ -43,8 +43,7 @@ func run() error { fileContents = fixKiotaNonDeterminism(fileContents, file.Name()) - // TODO(kfcampbell): verify file permission is what we want - err = os.WriteFile(path, []byte(fileContents), 0666) + err = os.WriteFile(path, []byte(fileContents), 0600) if err != nil { return err } @@ -124,7 +123,7 @@ func run() error { stdout.Reset() stderr.Reset() - output, err = cmd.Output() + _, err = cmd.Output() if err != nil { fmt.Printf("could not run go mod tidy: %v\nfull error log:\n%s", err, stderr.String()) } diff --git a/scripts/install-tools.sh b/scripts/install-tools.sh index edf082c5..fee51389 100755 --- a/scripts/install-tools.sh +++ b/scripts/install-tools.sh @@ -1,4 +1,7 @@ -#!/bin/sh +#!/usr/bin/env bash + +# note: check the version of golangci-lint used in .github/workflows/lint.yml to ensure accuracy +go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.56 # Install Kiota KIOTA_TOOL_NAME="microsoft.openapi.kiota" diff --git a/scripts/lint.sh b/scripts/lint.sh new file mode 100755 index 00000000..32cc70b6 --- /dev/null +++ b/scripts/lint.sh @@ -0,0 +1,9 @@ +#!/usr/bin/env bash + +# GitHub Actions runs linting checks from .github/workflows/lint.yml. +# This script may be used to detect issues before pushing to a PR check. +set -ex +./scripts/install-tools.sh +golangci-lint run post-processors/go/main.go +golangci-lint run post-processors/csharp/main.go +golangci-lint run schemas/main.go