fix(git-sync): drop the .git suffix from git http-backend PATH_INFO (smart-HTTP 404)
The /git smart-HTTP host 404'd EVERY fetch and push: PATH_INFO was built as `/<spaceId>.git/<subpath>`, so `git http-backend` resolved the repo at `<GIT_PROJECT_ROOT>/<spaceId>.git` — which does not exist. The vault is a NON-bare working repo (the engine needs a working tree) at `<dataDir>/<spaceId>`, so the CGI repo path must be `<spaceId>` (git http-backend serves the `.git` inside). The URL's conventional `.git` suffix is already stripped to `spaceId` by parseGitPath; re-appending it for PATH_INFO was the bug. Found by standing up a full e2e stand (real Postgres/Redis + server + a real git clone/push over the /git remote): clone and push both 404'd until this fix, after which a clone → edit → push round-trips the change all the way into the Docmost page. Also extracts the CGI-env construction into a pure, exported `buildGitBackendCgiEnv` and adds unit tests (the env build was previously untested — the gap this bug hid in): a regression guard pinning PATH_INFO to `/<spaceId>/<subpath>` (no `.git`), plus method/query/content-type/remote-user forwarding and the conditional GIT_PROTOCOL. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -4,8 +4,50 @@
|
||||
import {
|
||||
parseCgiResponse,
|
||||
splitCgiBuffer,
|
||||
buildGitBackendCgiEnv,
|
||||
} from './git-http-backend.service';
|
||||
|
||||
describe('buildGitBackendCgiEnv', () => {
|
||||
const base = {
|
||||
spaceId: 'space-1',
|
||||
subpath: 'info/refs',
|
||||
method: 'GET',
|
||||
queryString: 'service=git-upload-pack',
|
||||
contentType: '',
|
||||
remoteUser: 'alice@example.com',
|
||||
};
|
||||
|
||||
it('points PATH_INFO at the NON-bare repo dir (no .git suffix)', () => {
|
||||
// Regression guard: the vault lives at <root>/<spaceId> (a working repo), so
|
||||
// PATH_INFO must be /<spaceId>/<subpath>. A `.git` suffix made git
|
||||
// http-backend resolve <root>/<spaceId>.git and 404 every fetch/push.
|
||||
const env = buildGitBackendCgiEnv(base, '/vaults');
|
||||
expect(env.PATH_INFO).toBe('/space-1/info/refs');
|
||||
expect(env.PATH_INFO).not.toContain('.git');
|
||||
expect(env.GIT_PROJECT_ROOT).toBe('/vaults');
|
||||
});
|
||||
|
||||
it('forwards method/query/content-type/remote-user and exports all repos', () => {
|
||||
const env = buildGitBackendCgiEnv(
|
||||
{ ...base, method: 'POST', subpath: 'git-receive-pack', contentType: 'application/x-git-receive-pack-request', queryString: '' },
|
||||
'/vaults',
|
||||
);
|
||||
expect(env.REQUEST_METHOD).toBe('POST');
|
||||
expect(env.PATH_INFO).toBe('/space-1/git-receive-pack');
|
||||
expect(env.CONTENT_TYPE).toBe('application/x-git-receive-pack-request');
|
||||
expect(env.REMOTE_USER).toBe('alice@example.com');
|
||||
expect(env.GIT_HTTP_EXPORT_ALL).toBe('1');
|
||||
});
|
||||
|
||||
it('sets GIT_PROTOCOL only when the client sent the header', () => {
|
||||
expect(buildGitBackendCgiEnv(base, '/vaults').GIT_PROTOCOL).toBeUndefined();
|
||||
expect(
|
||||
buildGitBackendCgiEnv({ ...base, gitProtocol: 'version=2' }, '/vaults')
|
||||
.GIT_PROTOCOL,
|
||||
).toBe('version=2');
|
||||
});
|
||||
});
|
||||
|
||||
describe('parseCgiResponse', () => {
|
||||
it('defaults to status 200 with no Status header', () => {
|
||||
const r = parseCgiResponse('Content-Type: application/x-git-upload-pack-result');
|
||||
|
||||
@@ -101,6 +101,39 @@ export interface GitHttpBackendRequest {
|
||||
* Node response. Errors before any output produce a 500. Credentials are never
|
||||
* logged.
|
||||
*/
|
||||
/**
|
||||
* Build the `git http-backend` CGI environment overlay for one request (the
|
||||
* variables layered on top of `vaultGitEnv`'s cwd-isolated base). Pure so the
|
||||
* PATH_INFO / REMOTE_USER / conditional GIT_PROTOCOL wiring is unit-testable
|
||||
* without spawning git.
|
||||
*
|
||||
* PATH_INFO is the repo-relative CGI path. The vault is a NON-BARE working repo
|
||||
* on disk at `<dataDir>/<spaceId>` (the engine needs a working tree), so the
|
||||
* repo directory git http-backend must resolve is `<spaceId>` — NOT
|
||||
* `<spaceId>.git`. The URL carries the conventional `.git` suffix (stripped by
|
||||
* parseGitPath into `spaceId`); re-appending it here pointed the CGI at a
|
||||
* non-existent `<dataDir>/<spaceId>.git` and every fetch/push 404'd.
|
||||
*/
|
||||
export function buildGitBackendCgiEnv(
|
||||
parsed: GitHttpBackendRequest,
|
||||
projectRoot: string,
|
||||
): Record<string, string> {
|
||||
const cgiEnv: Record<string, string> = {
|
||||
GIT_PROJECT_ROOT: projectRoot,
|
||||
GIT_HTTP_EXPORT_ALL: '1', // authz is done by us; no git-daemon-export-ok file
|
||||
PATH_INFO: `/${parsed.spaceId}/${parsed.subpath}`,
|
||||
REQUEST_METHOD: parsed.method,
|
||||
QUERY_STRING: parsed.queryString,
|
||||
CONTENT_TYPE: parsed.contentType,
|
||||
REMOTE_USER: parsed.remoteUser,
|
||||
};
|
||||
// GIT_PROTOCOL is only set when the client sent the Git-Protocol header.
|
||||
if (parsed.gitProtocol) {
|
||||
cgiEnv.GIT_PROTOCOL = parsed.gitProtocol;
|
||||
}
|
||||
return cgiEnv;
|
||||
}
|
||||
|
||||
@Injectable()
|
||||
export class GitHttpBackendService {
|
||||
private readonly logger = new Logger(GitHttpBackendService.name);
|
||||
@@ -120,26 +153,11 @@ export class GitHttpBackendService {
|
||||
rawRes: ServerResponse,
|
||||
): Promise<void> {
|
||||
const projectRoot = this.environmentService.getGitSyncDataDir();
|
||||
// PATH_INFO is the repo-relative CGI path: /<spaceId>.git/<subpath>.
|
||||
const pathInfo = `/${parsed.spaceId}.git/${parsed.subpath}`;
|
||||
|
||||
// Build the CGI env from the engine's cwd-isolated base (strips GIT_DIR /
|
||||
// GIT_WORK_TREE), then layer the http-backend CGI variables. GIT_PROTOCOL is
|
||||
// only set when the client sent the Git-Protocol header. PATH is preserved
|
||||
// (vaultGitEnv already copies process.env, so PATH carries through).
|
||||
const cgiEnv: Record<string, string> = {
|
||||
GIT_PROJECT_ROOT: projectRoot,
|
||||
GIT_HTTP_EXPORT_ALL: '1', // authz is done by us; no git-daemon-export-ok file
|
||||
PATH_INFO: pathInfo,
|
||||
REQUEST_METHOD: parsed.method,
|
||||
QUERY_STRING: parsed.queryString,
|
||||
CONTENT_TYPE: parsed.contentType,
|
||||
REMOTE_USER: parsed.remoteUser,
|
||||
};
|
||||
if (parsed.gitProtocol) {
|
||||
cgiEnv.GIT_PROTOCOL = parsed.gitProtocol;
|
||||
}
|
||||
const env = vaultGitEnv(cgiEnv);
|
||||
// GIT_WORK_TREE), then layer the http-backend CGI variables. PATH is
|
||||
// preserved (vaultGitEnv already copies process.env, so PATH carries
|
||||
// through).
|
||||
const env = vaultGitEnv(buildGitBackendCgiEnv(parsed, projectRoot));
|
||||
|
||||
return new Promise<void>((resolve) => {
|
||||
let settled = false;
|
||||
|
||||
Reference in New Issue
Block a user