Skip to content

Commit 55dbb4d

Browse files
ajbeattieCopilot
ajbeattie
and
Copilot
authored
Merge commit from fork
* Escape regex metacharacters in attestation SAN matching Apply regexp.QuoteMeta to user-supplied values interpolated into regex patterns in expandToGitHubURLRegex and validateSignerWorkflow. Without escaping, dots in org/repo names act as regex wildcards, allowing attestation spoofing via lookalike repositories. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review: QuoteMeta entire static string at once Per review feedback, wrap the full formatted string in QuoteMeta rather than individual variables, to better indicate the whole piece is escaped and avoid confusion around trailing/leading chars. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 3f6a16a commit 55dbb4d

2 files changed

Lines changed: 29 additions & 12 deletions

File tree

‎pkg/cmd/attestation/verify/policy.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func expandToGitHubURL(tenant, ownerOrRepo string) string {
2424

2525
func expandToGitHubURLRegex(tenant, ownerOrRepo string) string {
2626
url := expandToGitHubURL(tenant, ownerOrRepo)
27-
return fmt.Sprintf("(?i)^%s/", url)
27+
return fmt.Sprintf("(?i)^%s", regexp.QuoteMeta(url+"/"))
2828
}
2929

3030
func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, error) {
@@ -155,7 +155,7 @@ func validateSignerWorkflow(hostname, signerWorkflow string) (string, error) {
155155
}
156156

157157
if match {
158-
return fmt.Sprintf("^https://%s", signerWorkflow), nil
158+
return "^" + regexp.QuoteMeta(fmt.Sprintf("https://%s", signerWorkflow)), nil
159159
}
160160

161161
// if the provided workflow did not match the expect format
@@ -164,5 +164,5 @@ func validateSignerWorkflow(hostname, signerWorkflow string) (string, error) {
164164
return "", errors.New("unknown signer workflow host")
165165
}
166166

167-
return fmt.Sprintf("^https://%s/%s", hostname, signerWorkflow), nil
167+
return "^" + regexp.QuoteMeta(fmt.Sprintf("https://%s/%s", hostname, signerWorkflow)), nil
168168
}

‎pkg/cmd/attestation/verify/policy_test.go‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package verify
22

33
import (
4+
"regexp"
45
"testing"
56

67
"github.com/cli/cli/v2/pkg/cmd/attestation/verification"
@@ -39,7 +40,7 @@ func TestNewEnforcementCriteria(t *testing.T) {
3940

4041
c, err := newEnforcementCriteria(opts)
4142
require.NoError(t, err)
42-
require.Equal(t, "(?i)^https://github.com/foo/bar/", c.SANRegex)
43+
require.Equal(t, `(?i)^https://github\.com/foo/bar/`, c.SANRegex)
4344
require.Zero(t, c.SAN)
4445
})
4546

@@ -55,7 +56,7 @@ func TestNewEnforcementCriteria(t *testing.T) {
5556

5657
c, err := newEnforcementCriteria(opts)
5758
require.NoError(t, err)
58-
require.Equal(t, "(?i)^https://baz.ghe.com/foo/bar/", c.SANRegex)
59+
require.Equal(t, `(?i)^https://baz\.ghe\.com/foo/bar/`, c.SANRegex)
5960
require.Zero(t, c.SAN)
6061
})
6162

@@ -70,7 +71,7 @@ func TestNewEnforcementCriteria(t *testing.T) {
7071

7172
c, err := newEnforcementCriteria(opts)
7273
require.NoError(t, err)
73-
require.Equal(t, "^https://github.com/foo/bar/.github/workflows/attest.yml", c.SANRegex)
74+
require.Equal(t, `^https://github\.com/foo/bar/\.github/workflows/attest\.yml`, c.SANRegex)
7475
require.Zero(t, c.SAN)
7576
})
7677

@@ -83,7 +84,7 @@ func TestNewEnforcementCriteria(t *testing.T) {
8384

8485
c, err := newEnforcementCriteria(opts)
8586
require.NoError(t, err)
86-
require.Equal(t, "(?i)^https://github.com/foo/bar/", c.SANRegex)
87+
require.Equal(t, `(?i)^https://github\.com/foo/bar/`, c.SANRegex)
8788
})
8889

8990
t.Run("sets SANRegex using opts.Owner", func(t *testing.T) {
@@ -94,7 +95,23 @@ func TestNewEnforcementCriteria(t *testing.T) {
9495

9596
c, err := newEnforcementCriteria(opts)
9697
require.NoError(t, err)
97-
require.Equal(t, "(?i)^https://github.com/foo/", c.SANRegex)
98+
require.Equal(t, `(?i)^https://github\.com/foo/`, c.SANRegex)
99+
})
100+
101+
t.Run("SANRegex escapes regex metacharacters in repo names", func(t *testing.T) {
102+
opts := &Options{
103+
ArtifactPath: artifactPath,
104+
SignerRepo: "my.org/my.repo",
105+
}
106+
107+
c, err := newEnforcementCriteria(opts)
108+
require.NoError(t, err)
109+
require.Equal(t, `(?i)^https://github\.com/my\.org/my\.repo/`, c.SANRegex)
110+
111+
// Verify the generated regex does NOT match a lookalike repo
112+
re := regexp.MustCompile(c.SANRegex)
113+
require.True(t, re.MatchString("https://github.com/my.org/my.repo/.github/workflows/build.yml"))
114+
require.False(t, re.MatchString("https://github.com/myXorg/myXrepo/.github/workflows/build.yml"))
98115
})
99116

100117
t.Run("sets Extensions.RunnerEnvironment to GitHubRunner value if opts.DenySelfHostedRunner is true", func(t *testing.T) {
@@ -280,25 +297,25 @@ func TestValidateSignerWorkflow(t *testing.T) {
280297
{
281298
name: "workflow with default host",
282299
providedSignerWorkflow: "github/artifact-attestations-workflows/.github/workflows/attest.yml",
283-
expectedWorkflowRegex: "^https://github.com/github/artifact-attestations-workflows/.github/workflows/attest.yml",
300+
expectedWorkflowRegex: `^https://github\.com/github/artifact-attestations-workflows/\.github/workflows/attest\.yml`,
284301
host: "github.com",
285302
},
286303
{
287304
name: "workflow with workflow URL included",
288305
providedSignerWorkflow: "github.com/github/artifact-attestations-workflows/.github/workflows/attest.yml",
289-
expectedWorkflowRegex: "^https://github.com/github/artifact-attestations-workflows/.github/workflows/attest.yml",
306+
expectedWorkflowRegex: `^https://github\.com/github/artifact-attestations-workflows/\.github/workflows/attest\.yml`,
290307
host: "github.com",
291308
},
292309
{
293310
name: "workflow with GH_HOST set",
294311
providedSignerWorkflow: "github/artifact-attestations-workflows/.github/workflows/attest.yml",
295-
expectedWorkflowRegex: "^https://myhost.github.com/github/artifact-attestations-workflows/.github/workflows/attest.yml",
312+
expectedWorkflowRegex: `^https://myhost\.github\.com/github/artifact-attestations-workflows/\.github/workflows/attest\.yml`,
296313
host: "myhost.github.com",
297314
},
298315
{
299316
name: "workflow with authenticated host",
300317
providedSignerWorkflow: "github/artifact-attestations-workflows/.github/workflows/attest.yml",
301-
expectedWorkflowRegex: "^https://authedhost.github.com/github/artifact-attestations-workflows/.github/workflows/attest.yml",
318+
expectedWorkflowRegex: `^https://authedhost\.github\.com/github/artifact-attestations-workflows/\.github/workflows/attest\.yml`,
302319
host: "authedhost.github.com",
303320
},
304321
}

0 commit comments

Comments
 (0)