Skip to content

Commit 227fe5e

Browse files
authored
feat: sanity checks for git repository in Redis cache (#41970)
## Description A potential corruption of repo is a possible when git in memory feature is active and git repo is cached in redis, A sanity check has been placed for git operations when repository is downloaded from Redis. It checks - Presence of .git folder - Compares checksum of repo with branch key Fixes #https://linear.app/appsmith/issue/APP-15285/git-connection-issue-persists-for-appsmith-treasury-app ## Automation /ok-to-test tags="@tag.Git" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/29080326412> > Commit: 6cbf5a2 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=29080326412&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Git` > Spec: > <hr>Fri, 10 Jul 2026 08:54:58 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved reliability when loading cached Git data by verifying repository integrity and branch information before continuing. * Automatically clears invalid cached data and stops the download if the repository looks corrupted or incomplete. * Added checks for missing Git metadata, missing branches, and mismatched commit data. * **Tests** * Added coverage for Git repository sanity checks, including valid repositories and several invalid repository states. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 066100e commit 227fe5e

2 files changed

Lines changed: 211 additions & 0 deletions

File tree

app/server/appsmith-git/src/main/resources/git.sh

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,97 @@ upload_branches_to_redis_hash() {
9090
redis-exec "$redis_url" HSET "$key_value_pair_key" $branches
9191
}
9292

93+
# Removes cached repository artifact and branch store from Redis.
94+
invalidate_redis_git_cache() {
95+
local redis_key="$1"
96+
local redis_url="$2"
97+
local key_value_pair_key="$3"
98+
99+
log_warn "Invalidating corrupted Redis git cache for key: $redis_key"
100+
redis-exec "$redis_url" DEL "$redis_key"
101+
redis-exec "$redis_url" DEL "$key_value_pair_key"
102+
}
103+
104+
# Verifies the extracted directory is a usable git repository.
105+
verify_git_repo_sanity() {
106+
local target_folder="$1"
107+
108+
if [[ ! -d "$target_folder/.git" ]]; then
109+
log_warn "Git sanity check failed: missing .git directory in $target_folder"
110+
return 1
111+
fi
112+
113+
if [[ ! -f "$target_folder/.git/HEAD" ]]; then
114+
log_warn "Git sanity check failed: missing HEAD in $target_folder/.git"
115+
return 1
116+
fi
117+
118+
if [[ "$(git -C "$target_folder" rev-parse --is-inside-work-tree 2>/dev/null || echo false)" != "true" ]]; then
119+
log_warn "Git sanity check failed: $target_folder is not a git work tree"
120+
return 1
121+
fi
122+
123+
if ! git -C "$target_folder" rev-parse --git-dir >/dev/null 2>&1; then
124+
log_warn "Git sanity check failed: cannot resolve git dir for $target_folder"
125+
return 1
126+
fi
127+
128+
log_info "Git sanity check passed for $target_folder"
129+
return 0
130+
}
131+
132+
# Verifies cached branch tips in Redis match the extracted repository.
133+
verify_cached_branch_store() {
134+
local target_folder="$1"
135+
local redis_url="$2"
136+
local key_value_pair_key="$3"
137+
138+
log_info "Verifying cached branch store for key: $key_value_pair_key"
139+
140+
local raw
141+
raw=$(redis-exec "$redis_url" --raw HGETALL "$key_value_pair_key" | sed 's/\"//g')
142+
143+
if [[ -z "$raw" ]]; then
144+
log_warn "Branch store empty or missing for key '$key_value_pair_key'; skipping branch sanity check"
145+
return 0
146+
fi
147+
148+
local arr=()
149+
while IFS= read -r line; do
150+
arr+=( "$line" )
151+
done <<< "$raw"
152+
153+
for ((i=0; i<${#arr[@]}; i+=2)); do
154+
local branch="${arr[i]}"
155+
local expected_commit="${arr[i+1]}"
156+
157+
if [[ -z "$branch" || -z "$expected_commit" ]]; then
158+
continue
159+
fi
160+
161+
if ! git -C "$target_folder" show-ref --verify --quiet "refs/heads/$branch" 2>/dev/null; then
162+
log_warn "Branch store sanity failed: branch '$branch' not found in repository"
163+
return 1
164+
fi
165+
166+
if ! git -C "$target_folder" cat-file -e "${expected_commit}^{commit}" 2>/dev/null; then
167+
log_warn "Branch store sanity failed: commit '$expected_commit' for branch '$branch' not found"
168+
return 1
169+
fi
170+
171+
local actual_commit
172+
actual_commit=$(git -C "$target_folder" rev-parse "refs/heads/$branch")
173+
174+
if [[ "$actual_commit" != "$expected_commit" && "$actual_commit" != "$expected_commit"* ]]; then
175+
log_warn "Branch store sanity failed: branch '$branch' at '$actual_commit', expected '$expected_commit'"
176+
return 1
177+
fi
178+
done
179+
180+
log_info "Branch store sanity check passed for $target_folder"
181+
return 0
182+
}
183+
93184
# Downloads git repo from Redis or clones if not cached
94185
git_download() {
95186
local author_email="$1"
@@ -113,8 +204,19 @@ git_download() {
113204
return 1
114205
fi
115206

207+
if ! verify_git_repo_sanity "$target_folder"; then
208+
invalidate_redis_git_cache "$redis_key" "$redis_url" "$key_value_pair_key"
209+
return 1
210+
fi
211+
116212
rm -f "$target_folder/.git/index.lock"
117213
git -C "$target_folder" reset --hard
214+
215+
if ! verify_cached_branch_store "$target_folder" "$redis_url" "$key_value_pair_key"; then
216+
invalidate_redis_git_cache "$redis_key" "$redis_url" "$key_value_pair_key"
217+
return 1
218+
fi
219+
118220
git -C "$target_folder" config user.name "$author_name"
119221
git -C "$target_folder" config user.email "$author_email"
120222
}
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
package com.appsmith.git.service;
2+
3+
import org.junit.jupiter.api.BeforeAll;
4+
import org.junit.jupiter.api.DisplayName;
5+
import org.junit.jupiter.api.Test;
6+
import org.junit.jupiter.api.io.TempDir;
7+
8+
import java.io.IOException;
9+
import java.nio.file.Files;
10+
import java.nio.file.Path;
11+
import java.nio.file.Paths;
12+
import java.util.concurrent.TimeUnit;
13+
14+
import static org.assertj.core.api.Assertions.assertThat;
15+
16+
/**
17+
* Unit tests for {@code verify_git_repo_sanity} in {@code git.sh}.
18+
*/
19+
public class GitRepoSanityCheckTest {
20+
21+
private static Path gitShPath;
22+
23+
@BeforeAll
24+
static void locateGitScript() throws Exception {
25+
var resource = GitRepoSanityCheckTest.class.getClassLoader().getResource("git.sh");
26+
assertThat(resource).as("git.sh on classpath").isNotNull();
27+
gitShPath = Paths.get(resource.toURI());
28+
}
29+
30+
@Test
31+
@DisplayName("passes for a valid git repository")
32+
void passesForValidRepository(@TempDir Path tempDir) throws Exception {
33+
Path repo = tempDir.resolve("valid-repo");
34+
initRepositoryWithCommit(repo);
35+
36+
assertThat(runVerifyGitRepoSanity(repo)).isZero();
37+
}
38+
39+
@Test
40+
@DisplayName("fails when directory has no .git")
41+
void failsForEmptyDirectory(@TempDir Path tempDir) throws Exception {
42+
Path repo = tempDir.resolve("empty-dir");
43+
Files.createDirectories(repo);
44+
45+
assertThat(runVerifyGitRepoSanity(repo)).isEqualTo(1);
46+
}
47+
48+
@Test
49+
@DisplayName("fails when .git exists but HEAD is missing")
50+
void failsWhenHeadIsMissing(@TempDir Path tempDir) throws Exception {
51+
Path repo = tempDir.resolve("partial-git");
52+
Files.createDirectories(repo.resolve(".git"));
53+
54+
assertThat(runVerifyGitRepoSanity(repo)).isEqualTo(1);
55+
}
56+
57+
@Test
58+
@DisplayName("fails when .git is a file instead of a directory")
59+
void failsWhenGitDirIsAFile(@TempDir Path tempDir) throws Exception {
60+
Path repo = tempDir.resolve("broken-git");
61+
Files.createDirectories(repo);
62+
Files.writeString(repo.resolve(".git"), "not-a-directory");
63+
64+
assertThat(runVerifyGitRepoSanity(repo)).isEqualTo(1);
65+
}
66+
67+
private static void initRepositoryWithCommit(Path repo) throws IOException, InterruptedException {
68+
Files.createDirectories(repo);
69+
runGit(repo, "init", "-b", "main");
70+
runGit(
71+
repo,
72+
"-c",
73+
"user.email=test@appsmith.com",
74+
"-c",
75+
"user.name=Test User",
76+
"commit",
77+
"--allow-empty",
78+
"-m",
79+
"init");
80+
}
81+
82+
private static void runGit(Path repo, String... args) throws IOException, InterruptedException {
83+
String[] command = new String[args.length + 3];
84+
command[0] = "git";
85+
command[1] = "-C";
86+
command[2] = repo.toAbsolutePath().toString();
87+
System.arraycopy(args, 0, command, 3, args.length);
88+
89+
Process process = new ProcessBuilder(command).redirectErrorStream(true).start();
90+
int exitCode = process.waitFor(30, TimeUnit.SECONDS) ? process.exitValue() : -1;
91+
assertThat(exitCode).as("git %s", String.join(" ", args)).isZero();
92+
}
93+
94+
private static int runVerifyGitRepoSanity(Path repo) throws IOException, InterruptedException {
95+
String command = String.format(
96+
"source '%s' && verify_git_repo_sanity '%s'", gitShPath.toAbsolutePath(), repo.toAbsolutePath());
97+
98+
Process process = new ProcessBuilder("bash", "-c", command)
99+
.redirectErrorStream(true)
100+
.start();
101+
102+
if (!process.waitFor(30, TimeUnit.SECONDS)) {
103+
process.destroyForcibly();
104+
return -1;
105+
}
106+
107+
return process.exitValue();
108+
}
109+
}

0 commit comments

Comments
 (0)