alice.pds.demo.boltless.dev
This repository has no description
knotserver/git: harden git clone to prevent command injection
Signed-off-by: Anirudh Oppiliappan <anirudh@tangled.org>
author
Anirudh Oppiliappan
date
(Jul 26, 2026, 6:27 PM +0300)
commit
05fbde99
05fbde99a87b8a55c78973b27093c05c4d3727aa
parent
79d96c9d
79d96c9dd3d40eaa4fa2d359bdcf0130ee0b8078
change-id
vzuzyrkv
vzuzyrkvsutkkpqnkltrquruvwkvtlms
+196
-3
+53
-3
knotserver/git/fork.go
+53
-3
knotserver/git/fork.go
···
8
8
"os"
9
9
"os/exec"
10
10
"path/filepath"
11
+
"strings"
11
12
12
13
"github.com/go-git/go-git/v5"
13
14
"github.com/go-git/go-git/v5/config"
···
23
24
// post-clone configure step in sb. The initial clone itself is not sandboxed
24
25
// because the target directory doesn't exist yet when the ruleset is applied.
25
26
func ForkWithSandbox(repoPath, source string, cfg *knotconfig.Config, sb sandbox.Backend) error {
26
-
u, err := url.Parse(source)
27
+
u, err := validateSource(source)
27
28
if err != nil {
28
-
return fmt.Errorf("failed to parse source URL: %w", err)
29
+
return err
29
30
}
30
31
32
+
localClone := false
31
33
if o := optimizeClone(u, cfg); o != nil {
32
34
u = o
35
+
localClone = true
33
36
}
34
37
35
-
cloneCmd := exec.Command("git", "clone", "--bare", u.String(), repoPath)
38
+
cloneCmd := exec.Command("git", cloneArgs(u, repoPath, localClone)...)
39
+
cloneCmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0")
36
40
if err := cloneCmd.Run(); err != nil {
37
41
return fmt.Errorf("failed to bare clone repository: %w", err)
38
42
}
···
56
60
}
57
61
58
62
return nil
63
+
}
64
+
65
+
// validateSource parses a user-supplied clone source and rejects anything that
66
+
// isn't a plain http(s) URL. This blocks argument injection (a leading "-" that
67
+
// git would read as an option) and non-http transports such as ext::, file://,
68
+
// or ssh that could be abused for command execution or local file access.
69
+
func validateSource(source string) (*url.URL, error) {
70
+
if strings.HasPrefix(source, "-") {
71
+
return nil, fmt.Errorf("invalid source: must not start with '-'")
72
+
}
73
+
74
+
u, err := url.Parse(source)
75
+
if err != nil {
76
+
return nil, fmt.Errorf("failed to parse source URL: %w", err)
77
+
}
78
+
79
+
switch u.Scheme {
80
+
case "http", "https":
81
+
default:
82
+
return nil, fmt.Errorf("invalid source: scheme must be http or https, got %q", u.Scheme)
83
+
}
84
+
85
+
if u.Host == "" {
86
+
return nil, fmt.Errorf("invalid source: missing host")
87
+
}
88
+
89
+
return u, nil
90
+
}
91
+
92
+
// cloneArgs builds the argument list for a hardened `git clone`.
93
+
//
94
+
// protocol.allow=never denies every transport by default; only http and
95
+
// https are re-enabled. This blocks ext::, ssh, git, and other transports
96
+
// that can potentially lead to command execution. The file transport is allowed
97
+
// only for the trusted local-clone optimization.
98
+
func cloneArgs(u *url.URL, repoPath string, localClone bool) []string {
99
+
args := []string{
100
+
"-c", "protocol.allow=never",
101
+
"-c", "protocol.http.allow=always",
102
+
"-c", "protocol.https.allow=always",
103
+
}
104
+
if localClone {
105
+
args = append(args, "-c", "protocol.file.allow=always")
106
+
}
107
+
args = append(args, "clone", "--bare", "--", u.String(), repoPath)
108
+
return args
59
109
}
60
110
61
111
func optimizeClone(u *url.URL, cfg *knotconfig.Config) *url.URL {
+143
knotserver/git/fork_test.go
+143
knotserver/git/fork_test.go
···
1
+
package git
2
+
3
+
import (
4
+
"os"
5
+
"path/filepath"
6
+
"slices"
7
+
"testing"
8
+
9
+
"github.com/stretchr/testify/require"
10
+
knotconfig "tangled.org/core/knotserver/config"
11
+
)
12
+
13
+
// TestValidateSource guards against the argument-injection / RCE reported for
14
+
// sh.tangled.repo.create: a source beginning with "--" (e.g.
15
+
// "--upload-pack=<cmd>") was handed straight to `git clone`, which interpreted
16
+
// it as an option and executed the embedded command as the git user.
17
+
func TestValidateSource(t *testing.T) {
18
+
rejected := []struct {
19
+
name string
20
+
source string
21
+
}{
22
+
{"upload-pack injection", "--upload-pack=touch /tmp/pwned"},
23
+
{"upload-pack injection with IFS", "--upload-pack=touch$IFS/tmp/pwned"},
24
+
{"leading dash", "-oProxyCommand=evil"},
25
+
{"leading dash single", "-"},
26
+
{"ext transport", "ext::sh -c 'touch /tmp/pwned'"},
27
+
{"file scheme", "file:///etc/passwd"},
28
+
{"ssh scheme", "ssh://git@example.com/repo"},
29
+
{"git scheme", "git://example.com/repo"},
30
+
{"scp-like syntax", "git@example.com:repo.git"},
31
+
{"empty", ""},
32
+
{"relative path", "some/local/path"},
33
+
{"missing host", "https://"},
34
+
}
35
+
36
+
for _, tc := range rejected {
37
+
t.Run("reject/"+tc.name, func(t *testing.T) {
38
+
_, err := validateSource(tc.source)
39
+
require.Error(t, err, "source %q must be rejected", tc.source)
40
+
})
41
+
}
42
+
43
+
accepted := []struct {
44
+
name string
45
+
source string
46
+
}{
47
+
{"https", "https://example.com/owner/repo"},
48
+
{"http", "http://example.com/owner/repo"},
49
+
{"https with port", "https://example.com:8443/owner/repo.git"},
50
+
{"https with userinfo", "https://user:token@example.com/owner/repo"},
51
+
}
52
+
53
+
for _, tc := range accepted {
54
+
t.Run("accept/"+tc.name, func(t *testing.T) {
55
+
u, err := validateSource(tc.source)
56
+
require.NoError(t, err, "source %q must be accepted", tc.source)
57
+
require.NotNil(t, u)
58
+
})
59
+
}
60
+
}
61
+
62
+
// TestCloneArgs asserts the hardening flags are present on the clone command:
63
+
// the "--" option terminator and a deny-by-default protocol allowlist.
64
+
func TestCloneArgs(t *testing.T) {
65
+
t.Run("remote http source", func(t *testing.T) {
66
+
u, err := validateSource("https://example.com/owner/repo")
67
+
require.NoError(t, err)
68
+
69
+
args := cloneArgs(u, "/scan/did:plc:abc", false)
70
+
71
+
requireOrdered(t, args, "clone", "--bare", "--", "https://example.com/owner/repo", "/scan/did:plc:abc")
72
+
requireConfig(t, args, "protocol.allow=never")
73
+
requireConfig(t, args, "protocol.http.allow=always")
74
+
requireConfig(t, args, "protocol.https.allow=always")
75
+
// the file transport must NOT be enabled for a user-provided source.
76
+
require.False(t, hasConfig(args, "protocol.file.allow=always"),
77
+
"file transport must not be allowlisted for remote sources")
78
+
79
+
// the source is the operand immediately after "--", so a leading-dash
80
+
// payload can never be read as a flag.
81
+
dashIdx := slices.Index(args, "--")
82
+
require.Positive(t, dashIdx)
83
+
require.Equal(t, "https://example.com/owner/repo", args[dashIdx+1])
84
+
})
85
+
86
+
t.Run("local optimized clone allowlists file", func(t *testing.T) {
87
+
u, err := validateSource("https://example.com/owner/repo")
88
+
require.NoError(t, err)
89
+
90
+
args := cloneArgs(u, "/scan/did:plc:abc", true)
91
+
requireConfig(t, args, "protocol.file.allow=always")
92
+
})
93
+
}
94
+
95
+
// TestForkWithSandboxRejectsInjection is the end-to-end regression: the PoC
96
+
// payload must fail before any command runs, and the sentinel file the payload
97
+
// would create must never appear.
98
+
func TestForkWithSandboxRejectsInjection(t *testing.T) {
99
+
tempDir := t.TempDir()
100
+
sentinel := filepath.Join(tempDir, "pwned")
101
+
102
+
cfg := &knotconfig.Config{}
103
+
cfg.Repo.ScanPath = filepath.Join(tempDir, "scan")
104
+
cfg.Server.Hostname = "knot.example.com"
105
+
106
+
repoPath := filepath.Join(cfg.Repo.ScanPath, "did:plc:victim")
107
+
108
+
// mirrors the PoC: --upload-pack=touch <sentinel>, spaces as $IFS.
109
+
source := "--upload-pack=touch$IFS" + sentinel
110
+
111
+
err := ForkWithSandbox(repoPath, source, cfg, nil)
112
+
require.Error(t, err, "malicious source must be rejected")
113
+
114
+
_, statErr := os.Stat(sentinel)
115
+
require.True(t, os.IsNotExist(statErr),
116
+
"command injection executed: sentinel file %s was created", sentinel)
117
+
}
118
+
119
+
func hasConfig(args []string, kv string) bool {
120
+
for i := 0; i+1 < len(args); i++ {
121
+
if args[i] == "-c" && args[i+1] == kv {
122
+
return true
123
+
}
124
+
}
125
+
return false
126
+
}
127
+
128
+
func requireConfig(t *testing.T, args []string, kv string) {
129
+
t.Helper()
130
+
require.True(t, hasConfig(args, kv), "expected -c %s in %v", kv, args)
131
+
}
132
+
133
+
// requireOrdered asserts want appears as a subsequence (in order) of args.
134
+
func requireOrdered(t *testing.T, args []string, want ...string) {
135
+
t.Helper()
136
+
i := 0
137
+
for _, a := range args {
138
+
if i < len(want) && a == want[i] {
139
+
i++
140
+
}
141
+
}
142
+
require.Equal(t, len(want), i, "expected ordered subsequence %v in %v", want, args)
143
+
}