Fix host directory policy boundary checks (#459)
* Fix host directory policy boundary checks * Fix host directory policy test lint
This commit is contained in:
parent
461af8de47
commit
1c241832f5
|
|
@ -58,10 +58,10 @@ func (policy HostDirPolicy) Validate(path string, readOnly bool) bool {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
return strings.HasPrefix(
|
path = strings.TrimSuffix(path, "/")
|
||||||
strings.TrimSuffix(path, "/"),
|
pathPrefix := strings.TrimSuffix(policy.PathPrefix, "/")
|
||||||
strings.TrimSuffix(policy.PathPrefix, "/"),
|
|
||||||
)
|
return path == pathPrefix || strings.HasPrefix(path, pathPrefix+"/")
|
||||||
}
|
}
|
||||||
|
|
||||||
func (policy HostDirPolicy) String() string {
|
func (policy HostDirPolicy) String() string {
|
||||||
|
|
|
||||||
|
|
@ -44,6 +44,107 @@ func TestHostDirPolicyValidate(t *testing.T) {
|
||||||
require.False(t, policy.Validate("/..", true))
|
require.False(t, policy.Validate("/..", true))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestHostDirPolicyValidatePathBoundary(t *testing.T) {
|
||||||
|
const (
|
||||||
|
localPathPrefix = "/src/"
|
||||||
|
githubURLPrefix = "https://github.com"
|
||||||
|
)
|
||||||
|
|
||||||
|
testCases := []struct {
|
||||||
|
name string
|
||||||
|
pathPrefix string
|
||||||
|
path string
|
||||||
|
allowed bool
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "local policy allows its exact path",
|
||||||
|
pathPrefix: localPathPrefix,
|
||||||
|
path: "/src",
|
||||||
|
allowed: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "local policy allows descendants",
|
||||||
|
pathPrefix: localPathPrefix,
|
||||||
|
path: "/src/project",
|
||||||
|
allowed: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "local policy without trailing slash rejects sibling sharing its prefix",
|
||||||
|
pathPrefix: "/src",
|
||||||
|
path: "/src-private",
|
||||||
|
allowed: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "local policy rejects sibling sharing its prefix",
|
||||||
|
pathPrefix: localPathPrefix,
|
||||||
|
path: "/src-private",
|
||||||
|
allowed: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "local root policy allows descendants",
|
||||||
|
pathPrefix: "/",
|
||||||
|
path: "/src/project",
|
||||||
|
allowed: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "local root policy rejects remote URLs",
|
||||||
|
pathPrefix: "/",
|
||||||
|
path: "https://github.com/archive.tar.gz",
|
||||||
|
allowed: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "URL policy allows its exact host",
|
||||||
|
pathPrefix: githubURLPrefix + "/",
|
||||||
|
path: githubURLPrefix,
|
||||||
|
allowed: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "URL policy allows paths on its host",
|
||||||
|
pathPrefix: githubURLPrefix,
|
||||||
|
path: "https://github.com/actions/archive.tar.gz",
|
||||||
|
allowed: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "URL policy rejects lookalike host",
|
||||||
|
pathPrefix: githubURLPrefix,
|
||||||
|
path: "https://github.com.attacker.com/archive.tar.gz",
|
||||||
|
allowed: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "URL policy with trailing slash rejects lookalike host",
|
||||||
|
pathPrefix: githubURLPrefix + "/",
|
||||||
|
path: "https://github.com.attacker.com/archive.tar.gz",
|
||||||
|
allowed: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "URL policy rejects host concealed by userinfo",
|
||||||
|
pathPrefix: githubURLPrefix,
|
||||||
|
path: "https://github.com@attacker.example/archive.tar.gz",
|
||||||
|
allowed: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "URL path policy allows descendants",
|
||||||
|
pathPrefix: "https://github.com/actions",
|
||||||
|
path: "https://github.com/actions/runner/archive.tar.gz",
|
||||||
|
allowed: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "URL path policy rejects sibling sharing its prefix",
|
||||||
|
pathPrefix: "https://github.com/actions",
|
||||||
|
path: "https://github.com/actions-private/archive.tar.gz",
|
||||||
|
allowed: false,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, testCase := range testCases {
|
||||||
|
t.Run(testCase.name, func(t *testing.T) {
|
||||||
|
policy := v1.HostDirPolicy{PathPrefix: testCase.pathPrefix, ReadOnly: false}
|
||||||
|
|
||||||
|
require.Equal(t, testCase.allowed, policy.Validate(testCase.path, false))
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestHostDirPolicyValidateReadOnly(t *testing.T) {
|
func TestHostDirPolicyValidateReadOnly(t *testing.T) {
|
||||||
policy := &v1.HostDirPolicy{PathPrefix: "/Users/ci/src", ReadOnly: true}
|
policy := &v1.HostDirPolicy{PathPrefix: "/Users/ci/src", ReadOnly: true}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue