Skip to content

Commit c4f04dd

Browse files
authored
fix(danger): close coverage gaps from PR #43 review (#44)
- Detect chmod setuid/setgid via '=' symbolic modes (u=rws, a=rwxs) - Add regression tests for '=' symbolic chmod cases - Fix staticcheck QF1001 in isAssignment via De Morgan's law - Check devnull.Close() error in test cleanup
1 parent a3dc420 commit c4f04dd

3 files changed

Lines changed: 20 additions & 7 deletions

File tree

internal/danger/classifier.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1622,7 +1622,7 @@ func isAssignment(tok string) bool {
16221622
return false
16231623
}
16241624
for _, r := range tok[:eq] {
1625-
if !(r == '_' || (r >= 'A' && r <= 'Z') || (r >= 'a' && r <= 'z') || (r >= '0' && r <= '9')) {
1625+
if r != '_' && (r < 'A' || r > 'Z') && (r < 'a' || r > 'z') && (r < '0' || r > '9') {
16261626
return false
16271627
}
16281628
}
@@ -1912,9 +1912,10 @@ func isSystemWrite(first string, tokens []string) bool {
19121912
}
19131913

19141914
// chmodSetsSUIDGID reports whether a chmod invocation sets the setuid or setgid
1915-
// bit, either symbolically (u+s, g+s, +s) or via an octal mode whose leading
1916-
// special-permission digit includes 4 (setuid) or 2 (setgid) — e.g. 4755, 2755,
1917-
// 6755. A plain 3- or 4-digit mode with a 0 special digit (0755) does not.
1915+
// bit, either symbolically (u+s, g+s, +s, u=rws, a=rwxs, …) or via an octal
1916+
// mode whose leading special-permission digit includes 4 (setuid) or 2
1917+
// (setgid) — e.g. 4755, 2755, 6755. A plain 3- or 4-digit mode with a 0
1918+
// special digit (0755) does not.
19181919
//
19191920
// Only the mode argument (the first non-flag operand) is inspected; trailing
19201921
// tokens are filenames and must not trigger on an incidental "+...s" or octal
@@ -1924,13 +1925,19 @@ func chmodSetsSUIDGID(tokens []string) bool {
19241925
if strings.HasPrefix(tok, "-") {
19251926
continue // flag (e.g. -R, --recursive, --reference=FILE)
19261927
}
1927-
// Symbolic: any clause that adds the 's' permission (u+s, g+s, a+s, +s,
1928-
// ug+rs, …). We only care about additions, so look for "+...s".
1928+
// Symbolic: any clause that sets the 's' permission (u+s, g+s, a+s, +s,
1929+
// ug+rs, u=rws, a=rwxs, …). Both '+' (add) and '=' (set exactly) can
1930+
// introduce the setuid/setgid bit.
19291931
if plus := strings.IndexByte(tok, '+'); plus >= 0 {
19301932
if strings.ContainsRune(tok[plus+1:], 's') {
19311933
return true
19321934
}
19331935
}
1936+
if eq := strings.IndexByte(tok, '='); eq >= 0 {
1937+
if strings.ContainsRune(tok[eq+1:], 's') {
1938+
return true
1939+
}
1940+
}
19341941
// Octal: a 4-digit mode whose first digit has bit 4 (setuid) or 2
19351942
// (setgid) set. 3-digit modes have no special-permission digit.
19361943
if len(tok) == 4 && isOctalMode(tok) {

internal/danger/coverage_extension_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ func TestClassify_ChmodSUIDGID_IsSystemWrite(t *testing.T) {
6868
"chmod 2755 /tmp/x",
6969
"chmod 6755 /tmp/x",
7070
"chmod ug+rs /tmp/x",
71+
"chmod u=rws /tmp/x",
72+
"chmod a=rwxs /tmp/x",
7173
}
7274
for _, c := range cmds {
7375
t.Run(c, func(t *testing.T) {

internal/danger/whitebox_coverage_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,9 @@ func silenceStderr(t *testing.T) {
121121
os.Stderr = devnull
122122
t.Cleanup(func() {
123123
os.Stderr = orig
124-
devnull.Close()
124+
if err := devnull.Close(); err != nil {
125+
t.Errorf("close /dev/null: %v", err)
126+
}
125127
})
126128
}
127129

@@ -375,6 +377,7 @@ func TestIsOctalMode(t *testing.T) {
375377
func TestChmodSetsSUIDGID_Direct(t *testing.T) {
376378
yes := [][]string{
377379
{"chmod", "u+s", "f"}, {"chmod", "g+s", "f"}, {"chmod", "+s", "f"},
380+
{"chmod", "u=rws", "f"}, {"chmod", "a=rwxs", "f"},
378381
{"chmod", "4755", "f"}, {"chmod", "2755", "f"}, {"chmod", "6755", "f"},
379382
{"chmod", "-R", "u+s", "f"}, // flag skipped, mode still inspected
380383
}
@@ -387,6 +390,7 @@ func TestChmodSetsSUIDGID_Direct(t *testing.T) {
387390
{"chmod", "+x", "f"}, {"chmod", "755", "f"}, {"chmod", "0755", "f"},
388391
{"chmod", "1755", "f"}, // sticky only
389392
{"chmod", "644", "build+gen.s"}, // filename, not mode
393+
{"chmod", "u=rw", "f"}, // no special permission
390394
{"chmod"}, // no operand
391395
}
392396
for _, toks := range no {

0 commit comments

Comments
 (0)