dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 6 Packages Projects Releases Wiki Activity

fix(git): ignore inherited indexed Git config beyond GIT_CONFIG_COUNT (#2475)

Browse Source 
Co-authored-by: Kandev Agent <agent@kandev.dev>
This commit is contained in:
João Salavisa
2026-08-10 00:48:44 +02:00
committed by GitHub
parent 01d6e8db77
commit e47f7c59f7
4 changed files with 100 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/backend/AGENTS.md
+4 -4
View File
@@ -225,10 +225,10 @@ Client (WS) ← Orchestrator ← Lifecycle Manager ←──── stream update
- **Path/security tests:** Avoid using the real filesystem root as a fixture root. Build fake absolute roots under `t.TempDir()` with `filepath.Join`; this keeps tests portable across Windows, POSIX, and privileged cloud executors.
- **Filesystem permission tests:** Assert permission-denied behavior only after probing that the current executor enforces the permission bit change. Root-like Sprite executors may bypass `chmod` restrictions.
- **Filesystem safety checks:** Carry the original `os.Lstat` `FileInfo` (or opened handle) through every decision; do not re-stat a validated path, which reopens a TOCTOU window before side effects. Test mismatches before writes or manager commands.
- **Git indexed-environment tests:** When a test supplies `GIT_CONFIG_COUNT`,
`GIT_CONFIG_KEY_n`, and `GIT_CONFIG_VALUE_n`, clear every inherited indexed
key first and restore it with `t.Cleanup`. Changing only the count can leave
higher parent indexes behind and create a malformed Git config block.
- **Git indexed-environment tests:** When a test supplies `GIT_CONFIG_COUNT`, `GIT_CONFIG_KEY_n`,
and `GIT_CONFIG_VALUE_n`, clear every inherited indexed key first and restore it with
`t.Cleanup`. `internal/gitconfigenv` ignores indexes past the count the way Git does, so an
uncleared parent silently changes what the block contains instead of failing loudly.
- **Full test output:** For local full-suite pass/fail validation, prefer plain `go test -race ./...`. `go test -json ./...` can emit very large JSONL streams; if a wrapper or tracing tool truncates the stream mid-record, downstream JSON parsing may fail even when Go tests passed. Use JSON output mainly for CI artifacts or test-report tooling that explicitly requires it.
- **Private executable helpers:** When library code starts `os.Executable()` in
a private helper mode, do not rely only on a command package's dispatch or
apps/backend/internal/agentctl/server/config/config_test.go
+34
View File
@@ -282,6 +282,40 @@ func TestCollectAgentEnvPreservesParentIndexedGitConfig(t *testing.T) {
}
}
func TestCollectAgentEnvIgnoresParentIndexedGitConfigBeyondCount(t *testing.T) {
// Hosts inherit indexed entries from a parent that later lowered
// GIT_CONFIG_COUNT. Git ignores the leftovers, so instance creation must
// too instead of failing every task start.
clearParentIndexedGitConfig(t)
t.Setenv("GIT_CONFIG_COUNT", "1")
t.Setenv("GIT_CONFIG_KEY_0", "core.hooksPath")
t.Setenv("GIT_CONFIG_VALUE_0", "/opt/locstat/hooks")
t.Setenv("GIT_CONFIG_KEY_1", "notes.augment.mergeStrategy")
t.Setenv("GIT_CONFIG_VALUE_1", "cat_sort_uniq")
env, err := CollectAgentEnvWithError(map[string]string{
"GIT_CONFIG_COUNT": "1",
"GIT_CONFIG_KEY_0": "credential.https://github.com.helper",
"GIT_CONFIG_VALUE_0": "!agentctl git-credential",
})
if err != nil {
t.Fatalf("CollectAgentEnvWithError() error = %v", err)
}
if got := envSliceValue(env, "GIT_CONFIG_COUNT"); got != "2" {
t.Fatalf("GIT_CONFIG_COUNT = %q, want 2", got)
}
if got := envSliceValue(env, "GIT_CONFIG_KEY_0"); got != "core.hooksPath" {
t.Fatalf("GIT_CONFIG_KEY_0 = %q, want core.hooksPath", got)
}
if got := envSliceValue(env, "GIT_CONFIG_KEY_1"); got != "credential.https://github.com.helper" {
t.Fatalf("GIT_CONFIG_KEY_1 = %q, want managed helper", got)
}
if got := envSliceValue(env, "GIT_CONFIG_KEY_2"); got != "" {
t.Fatalf("GIT_CONFIG_KEY_2 = %q, want stray parent entry dropped", got)
}
}
func TestCollectAgentEnvPreservesParentGitConfigHook(t *testing.T) {
clearParentIndexedGitConfig(t)
repository := t.TempDir()
apps/backend/internal/gitconfigenv/environment.go
+6 -36
View File
@@ -123,9 +123,9 @@ func entriesFrom(env map[string]string) ([]Entry, error) {
}
countValue, hasCount := env[countKey]
if !hasCount {
if hasIndexedEntry(env) {
return nil, fmt.Errorf("%s is required when indexed entries are set", countKey)
}
// Git reads indexed entries only when GIT_CONFIG_COUNT is set, so any
// leftover key/value pair configures nothing. Ignore them the way Git
// does rather than rejecting an inherited environment we do not own.
return nil, nil
}
count, err := strconv.Atoi(countValue)
@@ -141,42 +141,12 @@ func entriesFrom(env map[string]string) ([]Entry, error) {
}
entries = append(entries, Entry{Key: key, Value: value})
}
if hasOutOfRangeIndexedEntry(env, count) {
return nil, fmt.Errorf("indexed entry exceeds %s", countKey)
}
// Entries at or past the count are invisible to Git — a parent process that
// lowered GIT_CONFIG_COUNT leaves its higher indexes behind — so they are
// dropped here instead of failing the whole block.
return entries, nil
}
func hasIndexedEntry(env map[string]string) bool {
for key := range env {
if strings.HasPrefix(key, keyPrefix) || strings.HasPrefix(key, valuePrefix) {
return true
}
}
return false
}
func hasOutOfRangeIndexedEntry(env map[string]string, count int) bool {
for key := range env {
index, ok := indexedEntryIndex(key)
if ok && index >= count {
return true
}
}
return false
}
func indexedEntryIndex(key string) (int, bool) {
prefix := keyPrefix
if strings.HasPrefix(key, valuePrefix) {
prefix = valuePrefix
} else if !strings.HasPrefix(key, keyPrefix) {
return 0, false
}
index, err := strconv.Atoi(strings.TrimPrefix(key, prefix))
return index, err == nil && index >= 0
}
func removeBoundaryOverlap(base, overlay []Entry) []Entry {
maxOverlap := min(len(base), len(overlay))
for overlap := maxOverlap; overlap > 0; overlap-- {
apps/backend/internal/gitconfigenv/environment_test.go
+56
View File
@@ -77,6 +77,62 @@ func TestIndexedGitConfigMergeRejectsMalformedBlock(t *testing.T) {
}
}
func TestIndexedGitConfigMergeIgnoresEntriesBeyondCount(t *testing.T) {
// Observed on a host whose shell hook re-initialized the block at count 2
// without clearing the higher indexes a nested shell had appended. Git
// reads only entries 0 and 1, so the leftovers must not fail agent startup.
merged, err := Merge(map[string]string{
"GIT_CONFIG_COUNT": "2",
"GIT_CONFIG_KEY_0": "notes.augment.mergeStrategy",
"GIT_CONFIG_VALUE_0": "union",
"GIT_CONFIG_KEY_1": "core.hooksPath",
"GIT_CONFIG_VALUE_1": "/Users/example/.locstat/git/hooks",
"GIT_CONFIG_KEY_2": "credential.useHttpPath",
"GIT_CONFIG_VALUE_2": "true",
"GIT_CONFIG_KEY_3": "notes.augment.mergeStrategy",
"GIT_CONFIG_VALUE_3": "union",
"GIT_CONFIG_KEY_4": "core.hooksPath",
"GIT_CONFIG_VALUE_4": "/Users/example/.locstat/git/hooks",
}, environmentWithEntries(
Entry{Key: "credential.https://github.com.helper", Value: "!agentctl git-credential"},
))
if err != nil {
t.Fatalf("Merge() error = %v", err)
}
if got := merged["GIT_CONFIG_COUNT"]; got != "3" {
t.Fatalf("GIT_CONFIG_COUNT = %q, want 3", got)
}
if got := merged["GIT_CONFIG_KEY_1"]; got != "core.hooksPath" {
t.Fatalf("GIT_CONFIG_KEY_1 = %q, want core.hooksPath", got)
}
if got := merged["GIT_CONFIG_KEY_2"]; got != "credential.https://github.com.helper" {
t.Fatalf("GIT_CONFIG_KEY_2 = %q, want managed helper", got)
}
if got, exists := merged["GIT_CONFIG_KEY_3"]; exists {
t.Fatalf("GIT_CONFIG_KEY_3 = %q, want stray entry dropped", got)
}
}
func TestIndexedGitConfigMergeIgnoresIndexedEntriesWithoutCount(t *testing.T) {
// Git reads indexed entries only when GIT_CONFIG_COUNT is set.
merged, err := Merge(map[string]string{
"GIT_CONFIG_KEY_0": "core.hooksPath",
"GIT_CONFIG_VALUE_0": "/opt/locstat/hooks",
"HOME": "/home/kandev",
}, nil)
if err != nil {
t.Fatalf("Merge() error = %v", err)
}
if got, exists := merged["GIT_CONFIG_KEY_0"]; exists {
t.Fatalf("GIT_CONFIG_KEY_0 = %q, want orphaned entry dropped", got)
}
if got := merged["HOME"]; got != "/home/kandev" {
t.Fatalf("HOME = %q, want ordinary variables preserved", got)
}
}
func TestFilterRemovesOnlyRequestedIndexedEntries(t *testing.T) {
env := map[string]string{
"GIT_CONFIG_COUNT": "2",