synced/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2025-07-09 11:45:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

chore(scripts/rules.go): broaden scope of testingWithOwnerUser linter (#10548)

Browse Source 
* Updated testingWithOwnerUser ruleguard rule to detect:
  a) Passing client from coderdenttest.New() to clitest.SetupConfig() similar to what already exists for AGPL code
  b) Usage of any method of the owner client from coderdenttest.New() - all usages of the owner client must be justified with a `//nolint:gocritic` comment.
* Fixed resulting linter complaints.
* Added new coderdtest helpers CreateGroup and UpdateTemplateMeta.
* Modified check_enterprise_import.sh to ignore scripts/rules.go.
This commit is contained in:
Cian Johnston
2023-11-08 14:54:48 +00:00
committed by GitHub
parent 057b43a935
commit 26740cf00d
27 changed files with 473 additions and 331 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
scripts/check_enterprise_imports.sh
View File

@ -13,6 +13,7 @@ find . -regex ".*\.go" |
grep -v "./enterprise" |
grep -v ./scripts/auditdocgen/ --include="*.go" |
grep -v ./scripts/clidocgen/ --include="*.go" |
grep -v ./scripts/rules.go |
xargs grep -n "github.com/coder/coder/v2/enterprise"
# reverse the exit code because we want this script to fail if grep finds anything.
status=$?

43
scripts/rules.go
View File

@ -52,17 +52,54 @@ func dbauthzAuthorizationContext(m dsl.Matcher) {
func testingWithOwnerUser(m dsl.Matcher) {
m.Import("testing")
m.Import("github.com/coder/coder/v2/cli/clitest")
m.Import("github.com/coder/coder/v2/enterprise/coderd/coderenttest")
// For both AGPL and enterprise code, we check for SetupConfig being called with a
// client authenticated as the Owner user.
m.Match(`
$_ := coderdtest.CreateFirstUser($t, $client)
$*_
clitest.$SetupConfig($t, $client, $_)
$_ := coderdtest.CreateFirstUser($t, $client)
$*_
clitest.$SetupConfig($t, $client, $_)
`).
Where(m["t"].Type.Implements("testing.TB") &&
m["SetupConfig"].Text.Matches("^SetupConfig$") &&
m.File().Name.Matches(`_test\.go$`)).
At(m["SetupConfig"]).
Report(`The CLI will be operating as the owner user, which has unrestricted permissions. Consider creating a different user.`)
m.Match(`
$client, $_ := coderdenttest.New($t, $*_)
$*_
clitest.$SetupConfig($t, $client, $_)
`).Where(m["t"].Type.Implements("testing.TB") &&
m["SetupConfig"].Text.Matches("^SetupConfig$") &&
m.File().Name.Matches(`_test\.go$`)).
At(m["SetupConfig"]).
Report(`The CLI will be operating as the owner user, which has unrestricted permissions. Consider creating a different user.`)
// For the enterprise code, we check for any method called on the client.
// While we want to be a bit stricter here, some methods are known to require
// the owner user, so we exclude them.
m.Match(`
$client, $_ := coderdenttest.New($t, $*_)
$*_
$_, $_ := $client.$Method($*_)
`).Where(m["t"].Type.Implements("testing.TB") &&
m.File().Name.Matches(`_test\.go$`) &&
!m["Method"].Text.Matches(`^(UpdateAppearance|Licenses|AddLicense|InsertLicense|DeleteLicense|CreateWorkspaceProxy|Replicas|Regions)$`)).
At(m["Method"]).
Report(`This client is operating as the owner user, which has unrestricted permissions. Consider creating a different user.`)
// Sadly, we need to match both one- and two-valued assignments separately.
m.Match(`
$client, $_ := coderdenttest.New($t, $*_)
$*_
$_ := $client.$Method($*_)
`).Where(m["t"].Type.Implements("testing.TB") &&
m.File().Name.Matches(`_test\.go$`) &&
!m["Method"].Text.Matches(`^(UpdateAppearance|Licenses|AddLicense|InsertLicense|DeleteLicense|CreateWorkspaceProxy|Replicas|Regions)$`)).
At(m["Method"]).
Report(`This client is operating as the owner user, which has unrestricted permissions. Consider creating a different user.`)
}
// Use xerrors everywhere! It provides additional stacktrace info!