Skip to content

Commit 855b13b

Browse files
committed
fix(tests): adapt tests for windows CI runner
Two failure modes surfaced only after windows-latest joined the test matrix: 1. apply_test.go::TestResolveTemplatePaths/path_outside_rootDir_is_kept_as-is hardcoded /other/project/templates/controlplane.yaml as the outside-root input. That string is absolute on POSIX but not on Windows (no drive letter), so the resolver treated it as relative, joined it with tmpRoot, and produced 'other/project/...' instead of keeping it as-is. Construct the absolute path via filepath.VolumeName + filepath.Separator so it is absolute on both OSes, and expect filepath.ToSlash(absOutside) as the normalized result. 2. DACL assertions in secureperm_windows_test.go and template_windows_test.go used strings.Contains(sddl, fullSid) to verify the trustee. On GitHub Actions windows-latest the runner's RID-500 admin account is emitted as the SDDL alias 'LA' rather than the literal SID, so the substring check failed. Extract the ACE trustee via regex and resolve it through windows.StringToSid (which accepts both literal SIDs and well-known aliases), then compare against the current user SID with windows.EqualSid. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
1 parent 8d4d0a0 commit 855b13b

3 files changed

Lines changed: 86 additions & 8 deletions

File tree

pkg/commands/apply_test.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,12 @@ func TestResolveTemplatePaths(t *testing.T) {
9696
t.Fatalf("failed to create templates dir: %v", err)
9797
}
9898

99+
// Build a platform-portable absolute path outside tmpRoot.
100+
// filepath.VolumeName is "" on POSIX (yielding e.g. "/other/...") and
101+
// "C:" on Windows (yielding "C:\other\..."). Both are absolute and
102+
// definitely outside tmpRoot (which lives under the user temp dir).
103+
absOutside := filepath.Join(filepath.VolumeName(tmpRoot), string(filepath.Separator), "other", "project", "templates", "controlplane.yaml")
104+
99105
tests := []struct {
100106
name string
101107
templates []string
@@ -127,10 +133,14 @@ func TestResolveTemplatePaths(t *testing.T) {
127133
want: []string{"templates/controlplane.yaml"},
128134
},
129135
{
136+
// Constructed to be absolute on both POSIX and Windows so the
137+
// filepath.IsAbs branch is exercised on both CI runners. The
138+
// resolver normalizes outside-root paths via filepath.ToSlash,
139+
// so the expected output is the forward-slash form.
130140
name: "path outside rootDir is kept as-is",
131-
templates: []string{"/other/project/templates/controlplane.yaml"},
141+
templates: []string{absOutside},
132142
rootDir: tmpRoot,
133-
want: []string{"/other/project/templates/controlplane.yaml"},
143+
want: []string{filepath.ToSlash(absOutside)},
134144
},
135145
}
136146

pkg/commands/template_windows_test.go

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package commands
1919
import (
2020
"os"
2121
"path/filepath"
22+
"regexp"
2223
"strings"
2324
"testing"
2425

@@ -119,15 +120,34 @@ func TestWriteInplaceRendered_ProtectedDACL_Windows(t *testing.T) {
119120
if err != nil {
120121
t.Fatalf("GetTokenUser: %v", err)
121122
}
122-
wantSid := tu.User.Sid.String()
123123

124124
if !strings.Contains(sddl, "D:P") {
125125
t.Errorf("DACL not protected; SDDL=%q", sddl)
126126
}
127127
if got := strings.Count(sddl, "(A;"); got != 1 {
128128
t.Errorf("DACL has %d Allow ACEs, want 1; SDDL=%q", got, sddl)
129129
}
130-
if !strings.Contains(sddl, wantSid) {
131-
t.Errorf("DACL does not reference current user SID %q; SDDL=%q", wantSid, sddl)
130+
131+
// SDDL emits well-known SIDs as aliases (e.g. the RID-500 admin on
132+
// GitHub Actions runners comes back as "LA" rather than the full
133+
// "S-1-5-21-...-500"). Resolve the trustee string back through
134+
// StringToSid and compare with EqualSid to get a robust match.
135+
re := regexp.MustCompile(`\(A;([^)]+)\)`)
136+
m := re.FindStringSubmatch(sddl)
137+
if len(m) < 2 {
138+
t.Fatalf("no Allow ACE found in SDDL %q", sddl)
139+
}
140+
fields := strings.Split(m[1], ";")
141+
if len(fields) < 5 {
142+
t.Fatalf("unexpected ACE shape %q", m[1])
143+
}
144+
trusteeStr := fields[4]
145+
aceSid, err := windows.StringToSid(trusteeStr)
146+
if err != nil {
147+
t.Fatalf("StringToSid(%q): %v", trusteeStr, err)
148+
}
149+
if !windows.EqualSid(aceSid, tu.User.Sid) {
150+
t.Errorf("ACE trustee %q (SID %s) != current user SID %s",
151+
trusteeStr, aceSid.String(), tu.User.Sid.String())
132152
}
133153
}

pkg/secureperm/secureperm_windows_test.go

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package secureperm_test
1919
import (
2020
"os"
2121
"path/filepath"
22+
"regexp"
2223
"strings"
2324
"testing"
2425

@@ -27,6 +28,52 @@ import (
2728
"github.com/cozystack/talm/pkg/secureperm"
2829
)
2930

31+
// extractAllowACETrustee pulls the trustee string out of the first
32+
// Allow ACE in a SDDL string. SDDL ACE shape:
33+
//
34+
// (ace_type;ace_flags;rights;object_guid;inherit_object_guid;account_sid)
35+
//
36+
// The returned string may be a literal SID ("S-1-5-21-...") or a
37+
// well-known alias ("LA", "BA", "SY", ...) — caller feeds it to
38+
// windows.StringToSid which accepts both forms.
39+
func extractAllowACETrustee(sddl string) (string, error) {
40+
re := regexp.MustCompile(`\(A;([^)]+)\)`)
41+
m := re.FindStringSubmatch(sddl)
42+
if len(m) < 2 {
43+
return "", &sddlParseError{sddl: sddl, reason: "no Allow ACE"}
44+
}
45+
fields := strings.Split(m[1], ";")
46+
// After the leading "A;" that lives in the regex literal, the inner
47+
// fields are: ace_flags, rights, object_guid, inherit_object_guid,
48+
// account_sid[, resource_attribute]. Minimum 5 fields.
49+
if len(fields) < 5 {
50+
return "", &sddlParseError{sddl: sddl, reason: "ACE has fewer than 5 fields"}
51+
}
52+
return fields[4], nil
53+
}
54+
55+
type sddlParseError struct{ sddl, reason string }
56+
57+
func (e *sddlParseError) Error() string { return e.reason + ": " + e.sddl }
58+
59+
// assertTrusteeMatches resolves the ACE trustee string (literal SID or
60+
// SDDL alias) to a SID and compares to wantSid via EqualSid. SDDL
61+
// output on GitHub Actions runners returns the RID-500 admin as the
62+
// alias "LA" rather than the literal SID, so a string-contains check
63+
// against wantSid.String() is not robust. Resolving both sides to
64+
// canonical *SID and comparing with EqualSid is.
65+
func assertTrusteeMatches(t *testing.T, trusteeStr string, wantSid *windows.SID) {
66+
t.Helper()
67+
aceSid, err := windows.StringToSid(trusteeStr)
68+
if err != nil {
69+
t.Fatalf("StringToSid(%q): %v", trusteeStr, err)
70+
}
71+
if !windows.EqualSid(aceSid, wantSid) {
72+
t.Errorf("ACE trustee %q (SID %s) != current user SID %s",
73+
trusteeStr, aceSid.String(), wantSid.String())
74+
}
75+
}
76+
3077
// assertProtectedOwnerOnlyDACL reads the security descriptor back and
3178
// asserts it is structurally D:P(A;;FA;;;<current-user-SID>) — a
3279
// protected DACL with exactly one Allow ACE naming the current user.
@@ -58,17 +105,18 @@ func assertProtectedOwnerOnlyDACL(t *testing.T, path string) {
58105
if err != nil {
59106
t.Fatalf("GetTokenUser: %v", err)
60107
}
61-
wantSid := tu.User.Sid.String()
62108

63109
if !strings.Contains(sddl, "D:P") {
64110
t.Errorf("DACL is not protected (missing D:P flag); SDDL=%q", sddl)
65111
}
66112
if got := strings.Count(sddl, "(A;"); got != 1 {
67113
t.Errorf("DACL has %d Allow ACEs, want exactly 1; SDDL=%q", got, sddl)
68114
}
69-
if !strings.Contains(sddl, wantSid) {
70-
t.Errorf("DACL does not reference current user SID %q; SDDL=%q", wantSid, sddl)
115+
trusteeStr, err := extractAllowACETrustee(sddl)
116+
if err != nil {
117+
t.Fatalf("extract trustee: %v", err)
71118
}
119+
assertTrusteeMatches(t, trusteeStr, tu.User.Sid)
72120
}
73121

74122
// TestWriteFile_NewFile_DACL_Windows pins the happy path: a brand-new

0 commit comments

Comments
 (0)