Skip to content

feat: Implement Git repository multi-file upload API - #39

Open
sureshchouksey8 wants to merge 6 commits into
FreeCodeCamp-Chengdu:masterfrom
sureshchouksey8:feat/bounty-33
Open

feat: Implement Git repository multi-file upload API#39
sureshchouksey8 wants to merge 6 commits into
FreeCodeCamp-Chengdu:masterfrom
sureshchouksey8:feat/bounty-33

Conversation

@sureshchouksey8

@sureshchouksey8 sureshchouksey8 commented May 28, 2026

Copy link
Copy Markdown

PR-39 PR-39 PR-39 Powered by Pull Request Badge

Closes #33

Summary by CodeRabbit

  • New Features

    • Add Git repository file upload endpoint with multipart support and size limits; returns repository URL, branch, and file count.
    • Persist platform-specific usernames for OAuth credentials to improve account linking.
  • Bug Fixes / Safety

    • Reject uploads that target Git metadata or escape repository via symlinks; require fresh credentials when username is missing.
  • Tests

    • Add comprehensive tests for Git uploads, service flows, and OAuth credential persistence.

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d3764d47-8ad1-4453-9eed-b088fb1aa83b

📥 Commits

Reviewing files that changed from the base of the PR and between 70bfb43 and 9a59081.

📒 Files selected for processing (3)
  • Dockerfile
  • source/service/GitFile.ts
  • test/GitFile.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • Dockerfile
  • test/GitFile.test.ts
  • source/service/GitFile.ts

📝 Walkthrough

Walkthrough

Adds an authenticated PUT /file/Git/:noProtocolURL endpoint and GitFileService to download a repository, copy sanitized uploaded files into the workspace, and push changes via git-utility using OAuth-backed HTTP auth; persists OAuth usernames and includes tests and small Docker/package updates.

Changes

Git Repository Multi-File Upload

Layer / File(s) Summary
OAuth Credential Models & Domain Mapping
source/model/OAuth.ts, source/model/File.ts
OAuthCredential now persists userName. OAuthPlatformDomainMap maps platforms to domains. GitUploadResult defines { repositoryUrl, branch, fileCount } with validation.
OAuth Username Capture & Persistence
source/controller/OAuth.ts, test/OAuth.test.ts
OauthController.syncProfile accepts userName; GitHub/CNB sign-ins pass their username fields. Tests assert credential saved with platform, userName, and accessToken.
GitFileService Implementation & Testing
source/service/GitFile.ts, test/GitFile.test.ts
GitFileService builds auth env from OAuth credential, detects default branch via git ls-remote --symref, sanitizes and resolves incoming file paths (rejects .git segments and escapes), enforces ancestor symlink safety, downloads/uploads repo via git-utility using runCommand wrapper, and cleans up temp workspace. Tests cover success, .git path rejection, symlink escape protection, and stale-credential rejection.
File Upload Controller Endpoint & Tests
source/controller/File.ts, test/FileController.test.ts
Adds authenticated PUT /file/Git/:noProtocolURL using @koa/multer (10MB/file, max 20). Normalizes request.files, rejects empty uploads, delegates to gitFileService.uploadFilesToRepository, and removes temp uploaded files in finally. Tests verify delegation, cleanup on error, and empty-upload rejection.
Docker Build & Package Exports
Dockerfile, package.json, source/service/index.ts
Docker base stage installs git. package.json adds git-utility dependency. source/service/index.ts re-exports GitFile.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I hopped through temp dirs under moonlight,
Dropped files into repos with gentle might,
OAuth tucked close, git-utility in tow,
Pushed branches of code where new seedlings grow,
A tiny rabbit delivering commits in flight.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main feature: implementing a Git repository multi-file upload API, which aligns with the primary objective of the changeset.
Linked Issues check ✅ Passed All requirements from issue #33 are implemented: Git upload endpoint at PUT /file/Git/:noProtocolURL [#33], temporary file handling with git-utility package [#33], userName persistence in OAuthCredential [#33], platform-to-domain mapping via OAuthPlatformDomainMap [#33], and git CLI in Docker image [#33].
Out of Scope Changes check ✅ Passed All changes directly support the Git multi-file upload feature: gitFileService and uploadGitFiles endpoint are core, OAuth userName persistence and domain mapping are required prerequisites, and Dockerfile git installation is explicitly mandated.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
Dockerfile (1)

3-8: ⚡ Quick win

Clean up apt cache to reduce image size.

After installing packages with apt-get, the apt cache remains in /var/lib/apt/lists/ and bloats the image. Chain the cleanup commands in the same RUN layer to keep the intermediate layer small.

🐳 Proposed fix to add apt cache cleanup
 FROM node:22-slim AS base
-RUN apt-get update && \
-    apt-get install curl git -y --no-install-recommends
+RUN apt-get update && \
+    apt-get install curl git -y --no-install-recommends && \
+    apt-get clean && \
+    rm -rf /var/lib/apt/lists/*
 ENV PNPM_HOME="/pnpm"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Dockerfile` around lines 3 - 8, The apt cache isn't cleaned after installing
packages in the Dockerfile RUN that performs "apt-get update && apt-get install
curl git -y --no-install-recommends"; modify that single RUN layer to include
apt-get clean and remove /var/lib/apt/lists/* (e.g., chain "apt-get clean && rm
-rf /var/lib/apt/lists/*") so the intermediate image stays small; keep existing
ENV PNPM_HOME and PATH lines and the following "RUN npm i pnpm@latest -g"
unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@source/service/GitFile.ts`:
- Around line 150-157: The copyIncomingFile flow can be exploited via symlink
ancestors: before creating dirs or copying, resolve realpath for
repositoryFolder and then for each existing ancestor of targetPath (and the
nearest existing parent for non-existing ancestors) call fs.realpath (and use
fs.lstat to detect symlinks) and reject if any resolved ancestor does not
startWith the repositoryFolder realpath; only after these checks call mkdir and
copyFile to the final segment. Also harden external git calls used by
runCommand/git-utility by adding a bounded timeout option to execFile
invocations (pass a sensible timeout and propagate errors on timeout) so
downloads/uploads cannot hang indefinitely. Ensure references: copyIncomingFile,
resolveRepositoryPath, runCommand (git-utility).
- Around line 64-68: In GitFileService.runCommand, execFileAsync currently only
sets maxBuffer and can hang indefinitely; update the call to include sensible
default timeout and killSignal values (e.g., timeout in ms and killSignal like
'SIGKILL') merged with the incoming options so callers can override them, and
ensure the timeout/killSignal are passed into execFileAsync alongside maxBuffer
and env; modify the options merging around the execFileAsync invocation in
runCommand to include these defaults.

---

Nitpick comments:
In `@Dockerfile`:
- Around line 3-8: The apt cache isn't cleaned after installing packages in the
Dockerfile RUN that performs "apt-get update && apt-get install curl git -y
--no-install-recommends"; modify that single RUN layer to include apt-get clean
and remove /var/lib/apt/lists/* (e.g., chain "apt-get clean && rm -rf
/var/lib/apt/lists/*") so the intermediate image stays small; keep existing ENV
PNPM_HOME and PATH lines and the following "RUN npm i pnpm@latest -g" unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 811c7f57-a062-4a1e-85c8-2bee9edbcdb9

📥 Commits

Reviewing files that changed from the base of the PR and between 843f54b and 70bfb43.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (11)
  • Dockerfile
  • package.json
  • source/controller/File.ts
  • source/controller/OAuth.ts
  • source/model/File.ts
  • source/model/OAuth.ts
  • source/service/GitFile.ts
  • source/service/index.ts
  • test/FileController.test.ts
  • test/GitFile.test.ts
  • test/OAuth.test.ts

Comment thread source/service/GitFile.ts
Comment thread source/service/GitFile.ts
@sureshchouksey8

Copy link
Copy Markdown
Author

Addressed the CodeRabbit review on this PR in commit 9a59081.

Changes made:

  • Added default timeout: 60_000 and killSignal: 'SIGKILL' to GitFileService.runCommand() so git subprocesses cannot hang indefinitely, while still allowing caller overrides.
  • Hardened copyIncomingFile() by checking existing repository ancestors with lstat()/realpath() before mkdir()/copyFile(), rejecting symlink ancestors and realpath escapes outside the repository folder.
  • Added a regression test proving an upload through a symlinked repository ancestor is rejected and does not write outside the repo.
  • Cleaned Docker apt cache in the same install layer.

Validation:

  • npx jest test/GitFile.test.ts --runInBand -> 4 passed
  • npm run build -> passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

开发 Git 代码库多文件上传接口

2 participants