From 31e3697b95537166638c689c230efb7a18b6d38a Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Fri, 3 Jul 2026 18:08:26 +0900 Subject: [PATCH 01/13] feat: add sidebandProgress config option --- config.schema.json | 5 +++++ proxy.config.json | 1 + src/config/generated/config.ts | 10 ++++++++++ src/config/index.ts | 11 +++++++++++ 4 files changed, 27 insertions(+) diff --git a/config.schema.json b/config.schema.json index 5b5d0dfd6..a2b3aac85 100644 --- a/config.schema.json +++ b/config.schema.json @@ -297,6 +297,11 @@ "description": "Flag to enable CSRF protections for UI", "type": "boolean" }, + "sidebandProgress": { + "description": "Stream per-step validation progress messages to the git client's terminal during a push, using git sideband channel 2 (displayed as \"remote: ...\" lines). Requires the client to negotiate the side-band-64k capability, which all modern git clients do. When disabled, or when the client did not negotiate sideband, pushes receive a single buffered response as before. Operators behind intermediaries that buffer HTTP responses may wish to disable this.", + "type": "boolean", + "default": true + }, "plugins": { "type": "array", "description": "List of plugins to integrate on GitProxy's push or pull actions. Each value is either a file path or a module name.", diff --git a/proxy.config.json b/proxy.config.json index 18da6c6b8..5848348c6 100644 --- a/proxy.config.json +++ b/proxy.config.json @@ -147,6 +147,7 @@ "urlShortener": "", "contactEmail": "", "csrfProtection": true, + "sidebandProgress": true, "plugins": [], "apiAuthentication": [ { diff --git a/src/config/generated/config.ts b/src/config/generated/config.ts index 60d2204a8..5f110b51d 100644 --- a/src/config/generated/config.ts +++ b/src/config/generated/config.ts @@ -95,6 +95,15 @@ export interface GitProxyConfig { */ serverPort?: number; sessionMaxAgeHours?: number; + /** + * Stream per-step validation progress messages to the git client's terminal during a push, + * using git sideband channel 2 (displayed as "remote: ..." lines). Requires the client to + * negotiate the side-band-64k capability, which all modern git clients do. When disabled, + * or when the client did not negotiate sideband, pushes receive a single buffered response + * as before. Operators behind intermediaries that buffer HTTP responses may wish to disable + * this. + */ + sidebandProgress?: boolean; /** * List of database sources. The first source in the configuration with enabled=true will be * used. @@ -901,6 +910,7 @@ const typeMap: any = { { json: 'rateLimit', js: 'rateLimit', typ: u(undefined, r('RateLimit')) }, { json: 'serverPort', js: 'serverPort', typ: u(undefined, 3.14) }, { json: 'sessionMaxAgeHours', js: 'sessionMaxAgeHours', typ: u(undefined, 3.14) }, + { json: 'sidebandProgress', js: 'sidebandProgress', typ: u(undefined, true) }, { json: 'sink', js: 'sink', typ: u(undefined, a(r('Database'))) }, { json: 'ssh', js: 'ssh', typ: u(undefined, r('SSH')) }, { json: 'sslCertPemPath', js: 'sslCertPemPath', typ: u(undefined, '') }, diff --git a/src/config/index.ts b/src/config/index.ts index 792062863..26c7ed120 100644 --- a/src/config/index.ts +++ b/src/config/index.ts @@ -55,6 +55,7 @@ const REQUIRED_TOP_LEVEL_CONFIG_KEYS = [ 'rateLimit', 'serverPort', 'sessionMaxAgeHours', + 'sidebandProgress', 'sink', 'tempPassword', 'tls', @@ -484,6 +485,16 @@ export const getMaxPackSizeBytes = (): number => { return fallback; }; +/** + * Whether per-step validation progress should be streamed to the git client's + * terminal during a push using git sideband channel 2. + * @return {boolean} true when sideband progress streaming is enabled + */ +export const getSidebandProgressEnabled = (): boolean => { + const config = loadFullConfiguration(); + return config.sidebandProgress ?? true; +}; + export const getSSHConfig = () => { const defaultHostKey = { privateKeyPath: '.ssh/proxy_host_key', From 98853a7d1e2735f4cabf20bcea499f0fe9f463f3 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Mon, 6 Jul 2026 11:48:05 +0900 Subject: [PATCH 02/13] refactor: move /src/proxy/processors/constants to /src/proxy --- src/proxy/{processors => }/constants.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename src/proxy/{processors => }/constants.ts (96%) diff --git a/src/proxy/processors/constants.ts b/src/proxy/constants.ts similarity index 96% rename from src/proxy/processors/constants.ts rename to src/proxy/constants.ts index f5b7f0c90..8250bb745 100644 --- a/src/proxy/processors/constants.ts +++ b/src/proxy/constants.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { CommitData } from './types'; +import { CommitData } from './processors/types'; export const REFS_PREFIX = 'refs/'; export const BRANCH_PREFIX = 'refs/heads/'; From 60e2284d5342d2b765a1f8967910b9f115473d0f Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Mon, 6 Jul 2026 11:49:42 +0900 Subject: [PATCH 03/13] chore: update src/proxy constant imports --- src/proxy/processors/pktLineParser.ts | 2 +- src/proxy/processors/pre-processor/parsePush.ts | 2 +- src/proxy/processors/push-action/checkEmptyBranch.ts | 2 +- src/proxy/processors/push-action/checkHiddenCommits.ts | 2 +- src/proxy/processors/push-action/getDiff.ts | 2 +- src/proxy/ssh/server.ts | 2 +- test/testParsePush.test.ts | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/proxy/processors/pktLineParser.ts b/src/proxy/processors/pktLineParser.ts index 3fc2d795b..8e7cd246f 100644 --- a/src/proxy/processors/pktLineParser.ts +++ b/src/proxy/processors/pktLineParser.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { PACKET_SIZE } from './constants'; +import { PACKET_SIZE } from '../constants'; /** * Parses the packet lines from a buffer into an array of strings. diff --git a/src/proxy/processors/pre-processor/parsePush.ts b/src/proxy/processors/pre-processor/parsePush.ts index f437514fe..1cf5d3bac 100644 --- a/src/proxy/processors/pre-processor/parsePush.ts +++ b/src/proxy/processors/pre-processor/parsePush.ts @@ -30,7 +30,7 @@ import { PACKET_SIZE, GIT_OBJECT_TYPE_COMMIT, GIT_OBJECT_TYPE_TAG, -} from '../constants'; +} from '../../constants'; import { parsePacketLines } from '../pktLineParser'; import { getErrorMessage } from '../../../utils/errors'; diff --git a/src/proxy/processors/push-action/checkEmptyBranch.ts b/src/proxy/processors/push-action/checkEmptyBranch.ts index 4a665f1bb..e7ccf71bc 100644 --- a/src/proxy/processors/push-action/checkEmptyBranch.ts +++ b/src/proxy/processors/push-action/checkEmptyBranch.ts @@ -18,7 +18,7 @@ import { Request } from 'express'; import simpleGit from 'simple-git'; import { Action, Step } from '../../actions'; -import { EMPTY_COMMIT_HASH } from '../constants'; +import { EMPTY_COMMIT_HASH } from '../../constants'; import { handleErrorAndLog } from '../../../utils/errors'; const isEmptyBranch = async (action: Action): Promise => { diff --git a/src/proxy/processors/push-action/checkHiddenCommits.ts b/src/proxy/processors/push-action/checkHiddenCommits.ts index a32cbac8d..790b1adde 100644 --- a/src/proxy/processors/push-action/checkHiddenCommits.ts +++ b/src/proxy/processors/push-action/checkHiddenCommits.ts @@ -17,7 +17,7 @@ import path from 'path'; import { Action, Step } from '../../actions'; import { spawnSync } from 'child_process'; -import { EMPTY_COMMIT_HASH } from '../constants'; +import { EMPTY_COMMIT_HASH } from '../../constants'; import { Request } from 'express'; import { getErrorMessage } from '../../../utils/errors'; diff --git a/src/proxy/processors/push-action/getDiff.ts b/src/proxy/processors/push-action/getDiff.ts index 315800588..445b119e4 100644 --- a/src/proxy/processors/push-action/getDiff.ts +++ b/src/proxy/processors/push-action/getDiff.ts @@ -18,7 +18,7 @@ import { Request } from 'express'; import simpleGit from 'simple-git'; import { Action, Step } from '../../actions'; -import { EMPTY_COMMIT_HASH } from '../constants'; +import { EMPTY_COMMIT_HASH } from '../../constants'; import { getErrorMessage } from '../../../utils/errors'; const exec = async (_req: Request, action: Action): Promise => { diff --git a/src/proxy/ssh/server.ts b/src/proxy/ssh/server.ts index dc0b3691d..9dd9aa7b1 100644 --- a/src/proxy/ssh/server.ts +++ b/src/proxy/ssh/server.ts @@ -32,7 +32,7 @@ import { processGitUrl } from '../routes/helper'; import { ensureHostKey } from './hostKeyManager'; import { getProtocol, getSessionOutgoingChannelId } from './sshInternals'; import { parsePacketLines } from '../processors/pktLineParser'; -import { BRANCH_PREFIX } from '../processors/constants'; +import { BRANCH_PREFIX } from '../constants'; export class SSHServer { private server: ssh2.Server; diff --git a/test/testParsePush.test.ts b/test/testParsePush.test.ts index 8ca551011..814352167 100644 --- a/test/testParsePush.test.ts +++ b/test/testParsePush.test.ts @@ -29,7 +29,7 @@ import { } from '../src/proxy/processors/pre-processor/parsePush'; import { parsePacketLines } from '../src/proxy/processors/pktLineParser'; -import { EMPTY_COMMIT_HASH, FLUSH_PACKET, PACK_SIGNATURE } from '../src/proxy/processors/constants'; +import { EMPTY_COMMIT_HASH, FLUSH_PACKET, PACK_SIGNATURE } from '../src/proxy/constants'; import { CommitContent } from '../src/proxy/processors/types'; import { Action } from '../src/proxy/actions/Action'; import { Request } from 'express'; From 05dc21bb946c56d82a797d38395fef24d7587920 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Mon, 6 Jul 2026 11:56:11 +0900 Subject: [PATCH 04/13] feat: add SidebandProgressWriter --- src/proxy/constants.ts | 9 +++ src/proxy/sideband.ts | 157 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 166 insertions(+) create mode 100644 src/proxy/sideband.ts diff --git a/src/proxy/constants.ts b/src/proxy/constants.ts index 8250bb745..306749a81 100644 --- a/src/proxy/constants.ts +++ b/src/proxy/constants.ts @@ -46,3 +46,12 @@ export const SAMPLE_REPO = { canAuthorise: ['bob'], }, }; + +/** + * Maximum payload bytes for a single side-band-64k packet + * 65520 bytes total minus 4 bytes of pkt-line length and 1 band byte. + */ +export const MAX_SIDEBAND_PAYLOAD_BYTES = 65515; + +/** Capability that must be negotiated by the client for streamed progress */ +export const SIDE_BAND_64K_CAPABILITY = 'side-band-64k'; diff --git a/src/proxy/sideband.ts b/src/proxy/sideband.ts new file mode 100644 index 000000000..b2928d942 --- /dev/null +++ b/src/proxy/sideband.ts @@ -0,0 +1,157 @@ +/** + * Copyright 2026 GitProxy Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Response } from 'express'; + +import { Action, RequestType } from './actions'; +import { getSidebandProgressEnabled } from '../config'; +import { MAX_SIDEBAND_PAYLOAD_BYTES, SIDE_BAND_64K_CAPABILITY } from './constants'; + +/** Sideband channel numbers - see https://git-scm.com/docs/protocol-common */ +export enum SidebandBand { + /** Pack data or report-status payload */ + Data = 1, + /** Progress messages, displayed by the client as "remote: ..." */ + Progress = 2, + /** Fatal error message */ + Error = 3, +} + +/** + * Encode a payload as one or more sideband pkt-line packets on the given band. + * Payloads larger than the side-band-64k limit are split across packets. + * @param {SidebandBand} band The sideband channel to write to. + * @param {string | Buffer} payload The payload to encode (strings are UTF-8 encoded). + * @return {Buffer} The encoded packet(s), ready to write to the response stream. + */ +export const encodeSidebandChunk = (band: SidebandBand, payload: string | Buffer): Buffer => { + const data = Buffer.isBuffer(payload) ? payload : Buffer.from(payload, 'utf8'); + const packets: Buffer[] = []; + let offset = 0; + + do { + const slice = data.subarray(offset, offset + MAX_SIDEBAND_PAYLOAD_BYTES); + const length = (4 + 1 + slice.length).toString(16).padStart(4, '0'); + packets.push(Buffer.from(length, 'ascii'), Buffer.from([band]), slice); + offset += MAX_SIDEBAND_PAYLOAD_BYTES; + } while (offset < data.length); + + return Buffer.concat(packets); +}; + +/** + * Writer used by the push chain to stream per-step progress messages to the + * git client's terminal while validation runs. + */ +export interface ProgressWriter { + /** Whether messages are currently being streamed to the client. */ + readonly active: boolean; + /** + * Stream a progress message (band 2) to the client. A trailing newline is + * appended when missing. + * @param {string} text The message to display in the client terminal. + */ + message(text: string): void; +} + +/** Progress writer that discards all messages when streaming not available */ +export const NOOP_PROGRESS_WRITER: ProgressWriter = Object.freeze({ + active: false, + message: () => {}, +}); + +/** + * Streams sideband band-2 progress messages over an HTTP response. + * + * The response headers are flushed lazily on the first message, switching the + * response to chunked transfer encoding. Once started, the response must be + * finished as a sideband stream (either by piping the upstream receive-pack + * result through or by writing a final message followed by a flush packet). + */ +export class SidebandProgressWriter implements ProgressWriter { + active = true; + private started = false; + + /** + * @param {Response} res The HTTP response to stream progress messages to. + */ + constructor(private readonly res: Response) {} + + /** + * Send response headers and switch to chunked streaming, once. + */ + private start(): void { + if (this.started) { + return; + } + this.res.status(200); + this.res.set('content-type', 'application/x-git-receive-pack-result'); + this.res.set('expires', 'Fri, 01 Jan 1980 00:00:00 GMT'); + this.res.set('pragma', 'no-cache'); + this.res.set('cache-control', 'no-cache, max-age=0, must-revalidate'); + this.res.set('vary', 'Accept-Encoding'); + this.res.set('x-frame-options', 'DENY'); + this.res.flushHeaders(); + this.started = true; + } + + /** + * Stream a progress message (band 2) to the client. + * @param {string} text The message to display in the client terminal. + */ + message(text: string): void { + if (!this.active) { + return; + } + try { + this.start(); + const payload = text.endsWith('\n') ? text : `${text}\n`; + this.res.write(encodeSidebandChunk(SidebandBand.Progress, payload)); + } catch (err: unknown) { + console.error(`Failed to write sideband progress message: ${err}`); + this.active = false; + } + } +} + +/** + * Create a progress writer for the current request, or a no-op writer when + * streaming is not possible + * @param {Response} res The HTTP response associated with the push. + * @param {Action} action The parsed push action (capabilities must be populated). + * @return {ProgressWriter} An active or no-op progress writer. + */ +export const createProgressWriter = (res: Response, action: Action): ProgressWriter => { + if (!getSidebandProgressEnabled()) { + return NOOP_PROGRESS_WRITER; + } + if (action.type !== RequestType.PUSH || action.protocol !== 'https') { + return NOOP_PROGRESS_WRITER; + } + if (!action.capabilities?.includes(SIDE_BAND_64K_CAPABILITY)) { + return NOOP_PROGRESS_WRITER; + } + if ( + !res || + typeof res.flushHeaders !== 'function' || + typeof res.write !== 'function' || + res.headersSent || + res.writableEnded + ) { + return NOOP_PROGRESS_WRITER; + } + return new SidebandProgressWriter(res); +}; From 963aa1f60f72864a5ab66455067402573975e581 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Mon, 6 Jul 2026 12:07:31 +0900 Subject: [PATCH 05/13] feat: add progress writing into chain, add progress labels for each step --- src/proxy/actions/Action.ts | 1 + src/proxy/chain.ts | 41 ++++++++++++++++++- .../processors/pre-processor/parsePush.ts | 7 ++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/proxy/actions/Action.ts b/src/proxy/actions/Action.ts index d40507a38..275e4908a 100644 --- a/src/proxy/actions/Action.ts +++ b/src/proxy/actions/Action.ts @@ -75,6 +75,7 @@ class Action { tagData?: TagData[]; newIdxFiles?: string[]; protocol?: 'https' | 'ssh'; + capabilities?: string[]; pullAuthStrategy?: | 'basic' | 'ssh-user-key' diff --git a/src/proxy/chain.ts b/src/proxy/chain.ts index ba58b3f4c..5a7e241e8 100644 --- a/src/proxy/chain.ts +++ b/src/proxy/chain.ts @@ -22,6 +22,7 @@ import * as proc from './processors'; import { Processor } from './processors/types'; import { attemptAutoApproval, attemptAutoRejection } from './actions/autoActions'; import { handleErrorAndLog } from '../utils/errors'; +import { createProgressWriter } from './sideband'; const branchPushChain: Processor['exec'][] = [ proc.push.checkEmptyBranch, @@ -57,7 +58,40 @@ const defaultActionChain: Processor['exec'][] = [proc.push.checkRepoInAuthorised let pluginsInserted = false; -export const executeChain = async (req: Request, _res: Response): Promise => { +const stepProgressLabels: Record = { + 'checkEmptyBranch.exec': 'Checking for empty branch', + 'checkRepoInAuthorisedList.exec': 'Checking repository is authorised', + 'checkMessages.exec': 'Checking commit messages', + 'checkAuthorEmails.exec': 'Checking author emails', + 'checkUserPushPermission.exec': 'Checking push permissions', + 'pullRemote.exec': 'Fetching remote repository', + 'writePack.exec': 'writing pack data', + 'checkHiddenCommits.exec': 'Checking for hidden commits', + 'checkIfWaitingAuth.exec': 'Checking approval status', + 'executeExternalPreReceiveHook.exec': 'Running pre-receive hook', + 'getDiff.exec': 'Computing diff', + 'gitleaks.exec': 'Scanning for secrets', + 'scanDiff.exec': 'Scanning diff contents', + 'blockForAuth.exec': 'Requesting approval', +}; + +/** + * Obtain the message to display before a chain step. + * @param {Processor['exec']} fn The chain step about to be executed. + * @return {string} The message to display. + */ +const getProgressMessage = (fn: Processor['exec']): string => { + const displayName = (fn as { displayName?: string }).displayName; + if (displayName && stepProgressLabels[displayName]) { + return stepProgressLabels[displayName]; + } + if (displayName) { + return `running ${displayName.replace(/\.exec$/, '')}`; + } + return 'running plugin'; +}; + +export const executeChain = async (req: Request, res: Response): Promise => { let action: Action = {} as Action; let checkoutCleanUpRequired = false; @@ -71,8 +105,13 @@ export const executeChain = async (req: Request, _res: Response): Promise { throw new Error('Your push has been blocked. No ref updates found.'); } + // The client's capability list (side-band-64k, report-status, agent=..., etc.) + // is appended after a NUL byte on the first ref update line. + const capabilityString = refUpdates[0].split('\0')[1]; + action.capabilities = capabilityString + ? capabilityString.trim().split(/\s+/).filter(Boolean) + : []; + const parsedRefs = refUpdates.map((line) => { const [commitParts] = line.split('\0'); const parts = commitParts.split(' '); From 0d3a7d12e0061702f6e6a72af7535878fa750d2e Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Mon, 6 Jul 2026 12:44:25 +0900 Subject: [PATCH 06/13] feat: add handler for POST /git-receive-pack --- src/proxy/routes/index.ts | 208 +++++++++++++++++++++++++++++++++++++- 1 file changed, 203 insertions(+), 5 deletions(-) diff --git a/src/proxy/routes/index.ts b/src/proxy/routes/index.ts index e6babc385..f064b4c93 100644 --- a/src/proxy/routes/index.ts +++ b/src/proxy/routes/index.ts @@ -22,12 +22,15 @@ import { executeChain } from '../chain'; import { processUrlPath, validGitRequest } from './helper'; import { getAllProxiedHosts } from '../../db'; import { ProxyOptions } from 'express-http-proxy'; -import { getMaxPackSizeBytes } from '../../config'; +import { getMaxPackSizeBytes, getSidebandProgressEnabled } from '../../config'; import { MEGABYTE } from '../../constants'; import { handleErrorAndLog } from '../../utils/errors'; import { getUpstreamProxyConfig } from '../../config'; import { HttpsProxyAgent } from 'https-proxy-agent'; -import { OutgoingHttpHeaders, RequestOptions } from 'http'; +import http, { OutgoingHttpHeaders, RequestOptions } from 'http'; +import https from 'https'; +import { encodeSidebandChunk, SidebandBand } from '../sideband'; +import { FLUSH_PACKET } from '../constants'; enum ActionType { ALLOWED = 'Allowed', @@ -120,9 +123,8 @@ const sendErrorResponse = (req: Request, res: Response, message: string): void = }; const handleMessage = (message: string): string => { - const body = `\t${message}`; - const len = (6 + Buffer.byteLength(body)).toString(16).padStart(4, '0'); - return `${len}\x02${body}\n0000`; + const packet = encodeSidebandChunk(SidebandBand.Progress, `\t${message}\n`); + return packet.toString('utf8') + FLUSH_PACKET; }; const handleRefsErrorMessage = (message: string): string => { @@ -315,6 +317,193 @@ const extractRawBody = async (req: Request, res: Response, next: NextFunction) = } }; +const isReceivePackPost = (req: Request): boolean => + isPackPost(req) && req.url.endsWith('/git-receive-pack'); + +/** + * Finish a response that has already started streaming sideband progress + * @param {Response} res The in-progress streaming response + * @param {string} message The final message to display in the client terminal + */ +const endStreamedResponseWithMessage = (res: Response, message: string): void => { + if (res.writableEnded) { + return; + } + res.write(encodeSidebandChunk(SidebandBand.Progress, `\t${message}\n`)); + res.write(FLUSH_PACKET); + res.end(); +}; + +/** + * Resolve the upstream URL for a request + * @param {Request} req The client request. + * @param {string[]} originsToProxy Origins configured for proxying. + * @return {URL} The full upstream URL to forward the request to. + */ +const resolveUpstreamUrl = (req: Request, originsToProxy: string[]): URL => { + for (const origin of originsToProxy) { + if (req.originalUrl.startsWith(`/${origin}/`)) { + return new URL(`https:/${req.originalUrl}`); + } + } + // fallback (legacy URLs without an origin prefix) + return new URL(`https://github.com${req.originalUrl}`); +}; + +/** Headers that must not be forwarded verbatim to the upstream host. + * accept-encoding is stripped so the upstream response is not compressed, + * allowing it to be piped into an already-started sideband stream. */ +const UPSTREAM_HEADER_BLOCKLIST = [ + 'host', + 'connection', + 'content-length', + 'transfer-encoding', + 'accept-encoding', + 'upgrade', + 'keep-alive', + 'proxy-authorization', + 'te', + 'trailer', +]; + +/** + * Forward a buffered git-receive-pack request to the upstream host and relay + * the response to the client. + * + * When the response has already started streaming sideband progress, the + * upstream response body (itself a sideband pkt-line stream) is piped through + * verbatim, continuing the stream the proxy started. Otherwise the upstream + * status and headers are copied and the body piped, matching the behavior of + * the transparent proxy. + * @param {Request} req The client request (body must be a Buffer). + * @param {Response} res The client response. + * @param {URL} target The upstream URL to forward to. + * @return {Promise} Resolves when the upstream response has been relayed. + */ +const forwardReceivePackUpstream = async ( + req: Request, + res: Response, + target: URL, +): Promise => { + const body: Buffer = req.body; + const client = target.protocol === 'http:' ? http : https; + + const headers: OutgoingHttpHeaders = {}; + for (const [key, value] of Object.entries(req.headers)) { + if (value === undefined || UPSTREAM_HEADER_BLOCKLIST.includes(key.toLowerCase())) { + continue; + } + headers[key] = value; + } + headers['content-length'] = body.length; + + const agent = + target.protocol === 'https:' + ? buildUpstreamProxyAgent({ host: target.hostname, headers }) + : undefined; + + await new Promise((resolve, reject) => { + const upstreamReq = client.request( + target, + { method: 'POST', headers, ...(agent ? { agent } : {}) }, + (upstreamRes) => { + upstreamRes.on('error', reject); + upstreamRes.on('end', resolve); + + if (res.headersSent) { + if (upstreamRes.statusCode !== 200) { + upstreamRes.resume(); + endStreamedResponseWithMessage( + res, + `Push failed: upstream responded with status ${upstreamRes.statusCode}`, + ); + return; + } + upstreamRes.pipe(res); + } else { + // No streaming took place so we just relay everything as is + res.status(upstreamRes.statusCode ?? 502); + for (const [key, value] of Object.entries(upstreamRes.headers)) { + if (key === 'transfer-encoding' || key === 'connection' || value === undefined) { + continue; + } + res.set(key, value); + } + upstreamRes.pipe(res); + } + }, + ); + upstreamReq.on('error', reject); + upstreamReq.end(body); + }); +}; + +/** + * Create the request handler for git-receive-pack POSTs (pushes). + * + * Unlike other git requests, pushes are handled directly rather than through + * express-http-proxy so that validation progress can be streamed to the + * client via git sideband channel 2 while the chain runs. When the + * sidebandProgress config flag is disabled, the handler defers to the + * transparent proxy (identical behavior to previous releases). + * @param {string[]} originsToProxy Origins configured for proxying. + * @return {RequestHandler} The express request handler. + */ +const createReceivePackHandler = (originsToProxy: string[]): RequestHandler => { + return async (req, res, next) => { + if (!isReceivePackPost(req) || !getSidebandProgressEnabled()) { + return next(); + } + + try { + const urlComponents = processUrlPath(req.url); + if ( + !urlComponents || + urlComponents.gitPath === undefined || + !validGitRequest(urlComponents.gitPath, req.headers) + ) { + const message = 'Invalid request received'; + logAction(req.url, req.headers.host, req.headers['user-agent'], ActionType.ERROR, message); + res.status(200).send(handleMessage(message)); + return; + } + + // Use the raw body extracted by the extractRawBody middleware + if (req.bodyRaw) { + req.body = req.bodyRaw; + delete req.bodyRaw; + } + + const action = await executeChain(req, res); + + if (action.error || action.blocked) { + const message = action.errorMessage ?? action.blockedMessage ?? 'Unknown error'; + const type = action.error ? ActionType.ERROR : ActionType.BLOCKED; + + logAction(req.url, req.headers.host, req.headers['user-agent'], type, message); + if (res.headersSent) { + endStreamedResponseWithMessage(res, message); + } else { + sendErrorResponse(req, res, message); + } + return; + } + + logAction(req.url, req.headers.host, req.headers['user-agent'], ActionType.ALLOWED); + await forwardReceivePackUpstream(req, res, resolveUpstreamUrl(req, originsToProxy)); + } catch (error: unknown) { + const message = handleErrorAndLog(error, 'Error processing git-receive-pack request'); + + logAction(req.url, req.headers.host, req.headers['user-agent'], ActionType.ERROR, message); + if (res.headersSent) { + endStreamedResponseWithMessage(res, message); + } else { + sendErrorResponse(req, res, message); + } + } + }; +}; + const getRouter = async () => { const router = Router(); router.use(extractRawBody); @@ -325,6 +514,10 @@ const getRouter = async () => { console.log(`Initializing proxy router for origins: '${JSON.stringify(originsToProxy)}'`); + // Pushes are handled by a dedicated route instead of express-http-proxy + // to stream sidebandProgress if enabled + router.use(createReceivePackHandler(originsToProxy)); + // we need to wrap multiple proxy middlewares in a custom middleware as middlewares // with path are processed in descending path order (/ then /github.com etc.) and // we want the fallback proxy to go last. @@ -392,9 +585,14 @@ export { handleMessage, handleRefsErrorMessage, isPackPost, + isReceivePackPost, extractRawBody, validGitRequest, buildUpstreamProxyAgent, hostMatchesNoProxy, getOrCreateProxyAgent, + createReceivePackHandler, + resolveUpstreamUrl, + forwardReceivePackUpstream, + endStreamedResponseWithMessage, }; From e9f473769d40194c78e40384b615b46ecf087e7c Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Mon, 6 Jul 2026 15:12:02 +0900 Subject: [PATCH 07/13] test: sideband writer and new proxy route --- test/testProxyRoute.test.ts | 434 ++++++++++++++++++++++++++++++++++++ test/testSideband.test.ts | 219 ++++++++++++++++++ 2 files changed, 653 insertions(+) create mode 100644 test/testSideband.test.ts diff --git a/test/testProxyRoute.test.ts b/test/testProxyRoute.test.ts index 32f720015..2c516cfba 100644 --- a/test/testProxyRoute.test.ts +++ b/test/testProxyRoute.test.ts @@ -14,6 +14,8 @@ * limitations under the License. */ +import http from 'http'; +import { AddressInfo } from 'net'; import request from 'supertest'; import express, { Express, Request, Response } from 'express'; import { describe, it, beforeEach, afterEach, expect, vi, beforeAll, afterAll } from 'vitest'; @@ -21,6 +23,7 @@ import { describe, it, beforeEach, afterEach, expect, vi, beforeAll, afterAll } import { Action, Step } from '../src/proxy/actions'; import * as chain from '../src/proxy/chain'; import * as helper from '../src/proxy/routes/helper'; +import * as config from '../src/config'; import { Proxy } from '../src/proxy'; import { handleMessage, @@ -28,11 +31,24 @@ import { getRouter, handleRefsErrorMessage, proxyFilter, + isReceivePackPost, + createReceivePackHandler, + resolveUpstreamUrl, + forwardReceivePackUpstream, + endStreamedResponseWithMessage, } from '../src/proxy/routes'; +import { encodeSidebandChunk, SidebandBand } from '../src/proxy/sideband'; import * as db from '../src/db'; import { Service } from '../src/service'; +/** Collects a binary supertest response body into a Buffer. */ +const binaryParser = (res: request.Response, cb: (err: Error | null, body: Buffer) => void) => { + const chunks: Buffer[] = []; + res.on('data', (chunk: Buffer) => chunks.push(chunk)); + res.on('end', () => cb(null, Buffer.concat(chunks))); +}; + const TEST_DEFAULT_REPO = { url: 'https://github.com/finos/git-proxy.git', name: 'git-proxy', @@ -621,6 +637,424 @@ describe('proxyFilter', () => { }); }); +describe('isReceivePackPost', () => { + it('should return true for POST requests to git-receive-pack', () => { + const req = { + method: 'POST', + url: '/github.com/finos/git-proxy.git/git-receive-pack', + } as Request; + expect(isReceivePackPost(req)).toBe(true); + }); + + it('should return false for git-upload-pack POSTs and GET requests', () => { + expect( + isReceivePackPost({ + method: 'POST', + url: '/github.com/finos/git-proxy.git/git-upload-pack', + } as Request), + ).toBe(false); + expect( + isReceivePackPost({ + method: 'GET', + url: '/github.com/finos/git-proxy.git/git-receive-pack', + } as Request), + ).toBe(false); + }); +}); + +describe('resolveUpstreamUrl', () => { + it('should resolve the upstream URL from the origin in the request path', () => { + const req = { + originalUrl: '/github.com/finos/git-proxy.git/git-receive-pack', + } as Request; + const url = resolveUpstreamUrl(req, ['github.com', 'gitlab.com']); + + expect(url.href).toBe('https://github.com/finos/git-proxy.git/git-receive-pack'); + }); + + it('should resolve non-github origins', () => { + const req = { + originalUrl: '/gitlab.com/gitlab-community/meta.git/git-receive-pack', + } as Request; + const url = resolveUpstreamUrl(req, ['github.com', 'gitlab.com']); + + expect(url.href).toBe('https://gitlab.com/gitlab-community/meta.git/git-receive-pack'); + }); + + it('should fall back to github.com for legacy URLs without an origin prefix', () => { + const req = { + originalUrl: '/finos/git-proxy.git/git-receive-pack', + } as Request; + const url = resolveUpstreamUrl(req, ['gitlab.com']); + + expect(url.href).toBe('https://github.com/finos/git-proxy.git/git-receive-pack'); + }); +}); + +describe('endStreamedResponseWithMessage', () => { + it('should write the final message on band 2, a flush packet and end the response', () => { + const writes: Buffer[] = []; + const res = { + writableEnded: false, + write: vi.fn((chunk: Buffer | string) => { + writes.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); + return true; + }), + end: vi.fn(), + } as unknown as Response; + + endStreamedResponseWithMessage(res, 'Push blocked'); + + const body = Buffer.concat(writes).toString('utf8'); + // eslint-disable-next-line no-control-regex + expect(body).toMatch(/^[0-9a-f]{4}\x02\tPush blocked\n0000$/); + expect(res.end).toHaveBeenCalledOnce(); + }); + + it('should do nothing when the response has already ended', () => { + const res = { + writableEnded: true, + write: vi.fn(), + end: vi.fn(), + } as unknown as Response; + + endStreamedResponseWithMessage(res, 'too late'); + + expect(res.write).not.toHaveBeenCalled(); + expect(res.end).not.toHaveBeenCalled(); + }); +}); + +describe('createReceivePackHandler', () => { + const handler = createReceivePackHandler(['github.com']); + let mockReq: Partial; + let mockRes: Partial & { headersSent: boolean }; + let nextMock: ReturnType; + let writes: Buffer[]; + + beforeEach(() => { + writes = []; + nextMock = vi.fn(); + mockRes = { + headersSent: false, + writableEnded: false, + status: vi.fn().mockReturnThis(), + send: vi.fn().mockReturnThis(), + set: vi.fn().mockReturnThis(), + write: vi.fn((chunk: Buffer | string) => { + writes.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); + return true; + }), + end: vi.fn(), + } as unknown as Partial & { headersSent: boolean }; + + mockReq = { + method: 'POST', + url: '/github.com/finos/git-proxy.git/git-receive-pack', + originalUrl: '/github.com/finos/git-proxy.git/git-receive-pack', + headers: { + host: 'localhost:8080', + 'user-agent': 'git/2.42.0', + accept: 'application/x-git-receive-pack-result', + }, + body: Buffer.from('test'), + }; + + vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should defer to the next handler for non receive-pack requests', async () => { + mockReq.method = 'GET'; + + await handler(mockReq as Request, mockRes as Response, nextMock); + + expect(nextMock).toHaveBeenCalledOnce(); + }); + + it('should defer to the next handler when sidebandProgress is disabled', async () => { + vi.spyOn(config, 'getSidebandProgressEnabled').mockReturnValue(false); + + await handler(mockReq as Request, mockRes as Response, nextMock); + + expect(nextMock).toHaveBeenCalledOnce(); + }); + + it('should reject invalid requests without invoking the chain', async () => { + vi.spyOn(helper, 'processUrlPath').mockReturnValue(null); + const executeChainSpy = vi.spyOn(chain, 'executeChain'); + + await handler(mockReq as Request, mockRes as Response, nextMock); + + expect(executeChainSpy).not.toHaveBeenCalled(); + expect(mockRes.status).toHaveBeenCalledWith(200); + expect(vi.mocked(mockRes.send!).mock.calls[0][0]).toContain('Invalid request received'); + }); + + it('should send a buffered error response when the chain blocked without streaming', async () => { + vi.spyOn(helper, 'processUrlPath').mockReturnValue({ + gitPath: '/finos/git-proxy.git/git-receive-pack', + repoPath: 'github.com', + }); + vi.spyOn(helper, 'validGitRequest').mockReturnValue(true); + vi.spyOn(chain, 'executeChain').mockResolvedValue({ + error: false, + blocked: true, + blockedMessage: 'Push blocked by policy', + } as Action); + + await handler(mockReq as Request, mockRes as Response, nextMock); + + expect(nextMock).not.toHaveBeenCalled(); + expect(mockRes.set).toHaveBeenCalledWith( + 'content-type', + 'application/x-git-receive-pack-result', + ); + const sentMessage = vi.mocked(mockRes.send!).mock.calls[0][0]; + // eslint-disable-next-line no-control-regex + expect(sentMessage).toMatch(/^[0-9a-f]{4}\x02/); + expect(sentMessage).toContain('Push blocked by policy'); + }); + + it('should finish the sideband stream when the chain blocked after streaming started', async () => { + vi.spyOn(helper, 'processUrlPath').mockReturnValue({ + gitPath: '/finos/git-proxy.git/git-receive-pack', + repoPath: 'github.com', + }); + vi.spyOn(helper, 'validGitRequest').mockReturnValue(true); + vi.spyOn(chain, 'executeChain').mockImplementation(async () => { + // simulate the progress writer having flushed headers mid-chain + mockRes.headersSent = true; + return { + error: false, + blocked: true, + blockedMessage: 'Push blocked by policy', + } as Action; + }); + + await handler(mockReq as Request, mockRes as Response, nextMock); + + expect(mockRes.send).not.toHaveBeenCalled(); + const body = Buffer.concat(writes).toString('utf8'); + // eslint-disable-next-line no-control-regex + expect(body).toMatch(/^[0-9a-f]{4}\x02\tPush blocked by policy\n0000$/); + expect(mockRes.end).toHaveBeenCalledOnce(); + }); + + it('should move the raw body onto req.body before invoking the chain', async () => { + vi.spyOn(helper, 'processUrlPath').mockReturnValue({ + gitPath: '/finos/git-proxy.git/git-receive-pack', + repoPath: 'github.com', + }); + vi.spyOn(helper, 'validGitRequest').mockReturnValue(true); + vi.spyOn(chain, 'executeChain').mockResolvedValue({ + error: true, + blocked: false, + errorMessage: 'stop here', + } as Action); + (mockReq as any).bodyRaw = Buffer.from('raw pack data'); + + await handler(mockReq as Request, mockRes as Response, nextMock); + + expect(mockReq.body).toEqual(Buffer.from('raw pack data')); + expect((mockReq as any).bodyRaw).toBeUndefined(); + }); +}); + +describe('forwardReceivePackUpstream', () => { + let upstreamServer: http.Server; + let upstreamPort: number; + let lastUpstreamReq: { + method?: string; + url?: string; + headers?: http.IncomingHttpHeaders; + body?: Buffer; + }; + + const upstreamBody = Buffer.concat([ + encodeSidebandChunk(SidebandBand.Data, 'unpack ok\n'), + Buffer.from('0000'), + ]); + + beforeAll(async () => { + upstreamServer = http.createServer((req, res) => { + const chunks: Buffer[] = []; + req.on('data', (chunk: Buffer) => chunks.push(chunk)); + req.on('end', () => { + lastUpstreamReq = { + method: req.method, + url: req.url, + headers: req.headers, + body: Buffer.concat(chunks), + }; + if (req.url === '/upstream-401') { + res.writeHead(401, { 'content-type': 'text/plain' }); + res.end('auth required'); + return; + } + res.writeHead(200, { + 'content-type': 'application/x-git-receive-pack-result', + 'x-upstream-header': 'yes', + }); + res.end(upstreamBody); + }); + }); + await new Promise((resolve) => upstreamServer.listen(0, resolve)); + upstreamPort = (upstreamServer.address() as AddressInfo).port; + }); + + afterAll(async () => { + await new Promise((resolve, reject) => + upstreamServer.close((err) => (err ? reject(err) : resolve())), + ); + }); + + const createApp = (streaming: boolean, upstreamPath = '/upstream') => { + const app = express(); + app.post('/push', async (req, res, next) => { + try { + req.body = Buffer.from('PACKDATA'); + if (streaming) { + res.status(200); + res.set('content-type', 'application/x-git-receive-pack-result'); + res.flushHeaders(); + res.write(encodeSidebandChunk(SidebandBand.Progress, 'validating...\n')); + } + await forwardReceivePackUpstream( + req, + res, + new URL(`http://127.0.0.1:${upstreamPort}${upstreamPath}`), + ); + } catch (err) { + next(err); + } + }); + return app; + }; + + it('should append the upstream response verbatim to an in-progress sideband stream', async () => { + const res = await request(createApp(true)) + .post('/push') + .set('authorization', 'Basic dGVzdDp0ZXN0') + .set('accept-encoding', 'gzip') + .buffer(true) + .parse(binaryParser) + .send(); + + expect(res.status).toBe(200); + const body = (res.body as Buffer).toString('utf8'); + expect(body).toContain('validating...'); + expect(body).toContain('unpack ok'); + expect(body.endsWith('0000')).toBe(true); + + // proxy-generated packets come first, upstream bytes are appended verbatim + expect(body.indexOf('validating...')).toBeLessThan(body.indexOf('unpack ok')); + + // upstream received the buffered body with credentials, but no accept-encoding + expect(lastUpstreamReq.method).toBe('POST'); + expect(lastUpstreamReq.body?.toString('utf8')).toBe('PACKDATA'); + expect(lastUpstreamReq.headers?.authorization).toBe('Basic dGVzdDp0ZXN0'); + expect(lastUpstreamReq.headers?.['accept-encoding']).toBeUndefined(); + expect(lastUpstreamReq.headers?.['content-length']).toBe('8'); + }); + + it('should relay upstream status, headers and body when no streaming took place', async () => { + const res = await request(createApp(false)) + .post('/push') + .buffer(true) + .parse(binaryParser) + .send(); + + expect(res.status).toBe(200); + expect(res.headers['content-type']).toBe('application/x-git-receive-pack-result'); + expect(res.headers['x-upstream-header']).toBe('yes'); + expect((res.body as Buffer).equals(upstreamBody)).toBe(true); + }); + + it('should end the stream with an error message when upstream responds with a non-200 status', async () => { + const res = await request(createApp(true, '/upstream-401')) + .post('/push') + .buffer(true) + .parse(binaryParser) + .send(); + + expect(res.status).toBe(200); // headers were already flushed before the upstream call + const body = (res.body as Buffer).toString('utf8'); + expect(body).toContain('validating...'); + expect(body).toContain('upstream responded with status 401'); + expect(body.endsWith('0000')).toBe(true); + }); +}); + +describe('receive-pack route wiring', () => { + let app: Express; + + beforeEach(async () => { + app = express(); + app.use('/', await getRouter()); + vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + const sendPush = () => + request(app) + .post('/github.com/finos/git-proxy.git/git-receive-pack') + .set('user-agent', 'git/2.42.0') + .set('accept', 'application/x-git-receive-pack-result') + .set('content-type', 'application/x-git-receive-pack-request') + .buffer(true) + .parse(binaryParser) + .send(Buffer.from('0000')); + + it('should stream chain progress followed by the final blocked message', async () => { + vi.spyOn(chain, 'executeChain').mockImplementation(async (req, res) => { + // simulate the progress writer streaming a step message mid-chain + res.status(200); + res.set('content-type', 'application/x-git-receive-pack-result'); + res.flushHeaders(); + res.write( + encodeSidebandChunk(SidebandBand.Progress, 'checking repository is authorised...\n'), + ); + return { + error: false, + blocked: true, + blockedMessage: 'Push blocked by policy', + } as Action; + }); + + const res = await sendPush(); + + expect(res.status).toBe(200); + expect(res.headers['content-type']).toContain('application/x-git-receive-pack-result'); + const body = (res.body as Buffer).toString('utf8'); + expect(body).toContain('checking repository is authorised...'); + expect(body).toContain('Push blocked by policy'); + expect(body.endsWith('0000')).toBe(true); + }); + + it('should send a single buffered response when the chain did not stream', async () => { + vi.spyOn(chain, 'executeChain').mockResolvedValue({ + error: false, + blocked: true, + blockedMessage: 'Push blocked by policy', + } as Action); + + const res = await sendPush(); + + expect(res.status).toBe(200); + expect(res.headers['content-type']).toContain('application/x-git-receive-pack-result'); + const body = (res.body as Buffer).toString('utf8'); + // eslint-disable-next-line no-control-regex + expect(body).toMatch(/^[0-9a-f]{4}\x02\tPush blocked by policy\n0000$/); + }); +}); + describe('proxy route helpers', () => { describe('handleMessage', async () => { it('should handle short messages', async () => { diff --git a/test/testSideband.test.ts b/test/testSideband.test.ts new file mode 100644 index 000000000..87fcce8d1 --- /dev/null +++ b/test/testSideband.test.ts @@ -0,0 +1,219 @@ +/** + * Copyright 2026 GitProxy Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Response } from 'express'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; + +import { + encodeSidebandChunk, + SidebandBand, + SidebandProgressWriter, + NOOP_PROGRESS_WRITER, + createProgressWriter, +} from '../src/proxy/sideband'; +import { MAX_SIDEBAND_PAYLOAD_BYTES, SIDE_BAND_64K_CAPABILITY } from '../src/proxy/constants'; +import * as config from '../src/config'; +import { Action, RequestType } from '../src/proxy/actions'; + +const createMockRes = () => { + const writes: Buffer[] = []; + const res = { + headersSent: false, + writableEnded: false, + status: vi.fn().mockReturnThis(), + set: vi.fn().mockReturnThis(), + flushHeaders: vi.fn(), + write: vi.fn((chunk: Buffer | string) => { + writes.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); + return true; + }), + end: vi.fn(), + }; + return { res: res as unknown as Response, mocks: res, writes }; +}; + +const pushAction = (overrides: Partial = {}): Action => + ({ + type: RequestType.PUSH, + protocol: 'https', + capabilities: ['report-status', SIDE_BAND_64K_CAPABILITY, 'agent=git/2.42.0'], + ...overrides, + }) as Action; + +describe('encodeSidebandChunk', () => { + it('should encode a small payload as a single pkt-line packet', () => { + const packet = encodeSidebandChunk(SidebandBand.Progress, 'hello\n'); + + // 4 length bytes + 1 band byte + 6 payload bytes = 11 = 0x000b + expect(packet.length).toBe(11); + expect(packet.subarray(0, 4).toString('ascii')).toBe('000b'); + expect(packet[4]).toBe(2); + expect(packet.subarray(5).toString('utf8')).toBe('hello\n'); + }); + + it('should encode band 1 and band 3 packets', () => { + expect(encodeSidebandChunk(SidebandBand.Data, 'x')[4]).toBe(1); + expect(encodeSidebandChunk(SidebandBand.Error, 'x')[4]).toBe(3); + }); + + it('should preserve multi-byte UTF-8 payloads', () => { + const message = '✅ done'; + const packet = encodeSidebandChunk(SidebandBand.Progress, message); + const expectedLength = 5 + Buffer.byteLength(message); + + expect(parseInt(packet.subarray(0, 4).toString('ascii'), 16)).toBe(expectedLength); + expect(packet.subarray(5).toString('utf8')).toBe(message); + }); + + it('should fit a payload of exactly the maximum size in one packet', () => { + const payload = Buffer.alloc(MAX_SIDEBAND_PAYLOAD_BYTES, 0x61); + const packet = encodeSidebandChunk(SidebandBand.Progress, payload); + + expect(packet.length).toBe(4 + 1 + MAX_SIDEBAND_PAYLOAD_BYTES); + expect(packet.subarray(0, 4).toString('ascii')).toBe('fff0'); // 65520 + }); + + it('should split payloads larger than the maximum across multiple packets', () => { + const payload = Buffer.alloc(MAX_SIDEBAND_PAYLOAD_BYTES + 10, 0x62); + const packet = encodeSidebandChunk(SidebandBand.Progress, payload); + + // first packet: full sized + expect(packet.subarray(0, 4).toString('ascii')).toBe('fff0'); + const secondPacket = packet.subarray(4 + 1 + MAX_SIDEBAND_PAYLOAD_BYTES); + // second packet: 4 + 1 + 10 bytes = 15 = 0x000f + expect(secondPacket.subarray(0, 4).toString('ascii')).toBe('000f'); + expect(secondPacket[4]).toBe(2); + expect(secondPacket.subarray(5).toString('utf8')).toBe('b'.repeat(10)); + }); +}); + +describe('SidebandProgressWriter', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should flush headers once and write band-2 packets', () => { + const { res, mocks, writes } = createMockRes(); + const writer = new SidebandProgressWriter(res); + + writer.message('scanning for secrets...'); + writer.message('checking commit messages...'); + + expect(mocks.status).toHaveBeenCalledExactlyOnceWith(200); + expect(mocks.set).toHaveBeenCalledWith('content-type', 'application/x-git-receive-pack-result'); + expect(mocks.flushHeaders).toHaveBeenCalledOnce(); + expect(writes).toHaveLength(2); + + // eslint-disable-next-line no-control-regex + expect(writes[0].toString('utf8')).toMatch(/^[0-9a-f]{4}\x02scanning for secrets\.\.\.\n$/); + // eslint-disable-next-line no-control-regex + expect(writes[1].toString('utf8')).toMatch(/^[0-9a-f]{4}\x02checking commit messages\.\.\.\n$/); + }); + + it('should not append a newline when the message already ends with one', () => { + const { res, writes } = createMockRes(); + const writer = new SidebandProgressWriter(res); + + writer.message('already terminated\n'); + + expect(writes[0].toString('utf8').endsWith('already terminated\n')).toBe(true); + expect(writes[0].toString('utf8').endsWith('\n\n')).toBe(false); + }); + + it('should deactivate itself when a write fails', () => { + const { res, mocks } = createMockRes(); + vi.spyOn(console, 'error').mockImplementation(() => {}); + mocks.write.mockImplementation(() => { + throw new Error('boom'); + }); + const writer = new SidebandProgressWriter(res); + + writer.message('first'); + expect(writer.active).toBe(false); + + writer.message('second'); + expect(mocks.write).toHaveBeenCalledOnce(); + }); +}); + +describe('createProgressWriter', () => { + beforeEach(() => { + vi.spyOn(config, 'getSidebandProgressEnabled').mockReturnValue(true); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should return an active writer for an HTTPS push with side-band-64k negotiated', () => { + const { res } = createMockRes(); + const writer = createProgressWriter(res, pushAction()); + + expect(writer.active).toBe(true); + expect(writer).toBeInstanceOf(SidebandProgressWriter); + }); + + it('should return a no-op writer when the config flag is disabled', () => { + vi.mocked(config.getSidebandProgressEnabled).mockReturnValue(false); + const { res } = createMockRes(); + + expect(createProgressWriter(res, pushAction())).toBe(NOOP_PROGRESS_WRITER); + }); + + it('should return a no-op writer for non-push actions', () => { + const { res } = createMockRes(); + + expect(createProgressWriter(res, pushAction({ type: RequestType.PULL }))).toBe( + NOOP_PROGRESS_WRITER, + ); + }); + + it('should return a no-op writer for SSH pushes', () => { + const { res } = createMockRes(); + + expect(createProgressWriter(res, pushAction({ protocol: 'ssh' }))).toBe(NOOP_PROGRESS_WRITER); + }); + + it('should return a no-op writer when side-band-64k was not negotiated', () => { + const { res } = createMockRes(); + + expect(createProgressWriter(res, pushAction({ capabilities: ['report-status'] }))).toBe( + NOOP_PROGRESS_WRITER, + ); + expect(createProgressWriter(res, pushAction({ capabilities: undefined }))).toBe( + NOOP_PROGRESS_WRITER, + ); + }); + + it('should return a no-op writer when the response is not usable for streaming', () => { + const { res: startedRes, mocks } = createMockRes(); + mocks.headersSent = true; + expect(createProgressWriter(startedRes, pushAction())).toBe(NOOP_PROGRESS_WRITER); + + // mock responses without streaming support (e.g. the SSH server's mock res) + const mockRes = { send: () => {} } as unknown as Response; + expect(createProgressWriter(mockRes, pushAction())).toBe(NOOP_PROGRESS_WRITER); + + expect(createProgressWriter(undefined as unknown as Response, pushAction())).toBe( + NOOP_PROGRESS_WRITER, + ); + }); + + it('the no-op writer should discard messages without error', () => { + expect(NOOP_PROGRESS_WRITER.active).toBe(false); + expect(() => NOOP_PROGRESS_WRITER.message('ignored')).not.toThrow(); + }); +}); From 7c68fb634b1e304c39bf44c517dddf03e9af7e52 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Mon, 6 Jul 2026 15:12:29 +0900 Subject: [PATCH 08/13] test: capability parsing in parsePush --- test/testParsePush.test.ts | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/testParsePush.test.ts b/test/testParsePush.test.ts index 814352167..d42c6599a 100644 --- a/test/testParsePush.test.ts +++ b/test/testParsePush.test.ts @@ -584,6 +584,40 @@ describe('parsePackFile', () => { }); }); + it('should capture the client capability list from the first ref update line', async () => { + const oldCommit = 'a'.repeat(40); + const newCommit = 'b'.repeat(40); + const ref = 'refs/heads/main'; + const packetLine = `${oldCommit} ${newCommit} ${ref}\0report-status side-band-64k agent=git/2.42.0\n`; + + const commitContent = + 'tree 1234567890abcdef1234567890abcdef12345678\n' + + 'parent abcdef1234567890abcdef1234567890abcdef12\n' + + 'author Test Author 1234567890 +0000\n' + + 'committer Test Committer 1234567890 +0000\n\n' + + 'feat: Add new feature\n'; + + const packBuffer = createSamplePackBuffer(1, commitContent, 1); + req.body = Buffer.concat([createPacketLineBuffer([packetLine]), packBuffer]); + + const result = await exec(req, action); + + expect(result.capabilities).toEqual(['report-status', 'side-band-64k', 'agent=git/2.42.0']); + }); + + it('should set an empty capability list when the ref update line has no capabilities', async () => { + const oldCommit = 'a'.repeat(40); + const newCommit = 'b'.repeat(40); + const ref = 'refs/heads/main'; + // no NUL byte, so no capability list + const packetLines = [`${oldCommit} ${newCommit} ${ref}\n`]; + req.body = createPacketLineBuffer(packetLines); + + const result = await exec(req, action); + + expect(result.capabilities).toEqual([]); + }); + it('should successfully parse a valid push request (captured)', async () => { const oldCommit = '640bd00d63208466021143366adbc926824ba66f'; const newCommit = '93ca160407a9660c5ef81b951892b7a9ab1c41ca'; From 16bfca74c3619c3ec2fa261fca223cd7298a9e8c Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Mon, 6 Jul 2026 15:50:31 +0900 Subject: [PATCH 09/13] test: fix broken constant imports --- packages/git-proxy-cli/test/testCli.test.ts | 2 +- test/checkHiddenCommit.test.ts | 2 +- test/db/db.test.ts | 2 +- test/integration/forcePush.integration.test.ts | 2 +- test/processors/checkAuthorEmails.test.ts | 2 +- test/processors/checkEmptyBranch.test.ts | 2 +- test/processors/checkMessages.test.ts | 2 +- test/processors/getDiff.test.ts | 2 +- test/testPush.test.ts | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/git-proxy-cli/test/testCli.test.ts b/packages/git-proxy-cli/test/testCli.test.ts index 078b0a423..5f9829200 100644 --- a/packages/git-proxy-cli/test/testCli.test.ts +++ b/packages/git-proxy-cli/test/testCli.test.ts @@ -20,7 +20,7 @@ import { describe, it, beforeAll, afterAll } from 'vitest'; import { setConfigFile } from '../../../src/config/file'; import { invalidateCache } from '../../../src/config'; -import { SAMPLE_REPO } from '../../../src/proxy/processors/constants'; +import { SAMPLE_REPO } from '../../../src/proxy/constants'; import { handleErrorAndLog } from '../../../src/utils/errors'; setConfigFile( diff --git a/test/checkHiddenCommit.test.ts b/test/checkHiddenCommit.test.ts index 3a81ef0bb..6d1c8d1a5 100644 --- a/test/checkHiddenCommit.test.ts +++ b/test/checkHiddenCommit.test.ts @@ -17,7 +17,7 @@ import { describe, it, beforeEach, afterEach, expect, vi } from 'vitest'; import { exec as checkHidden } from '../src/proxy/processors/push-action/checkHiddenCommits'; import { Action } from '../src/proxy/actions'; -import { EMPTY_COMMIT_HASH } from '../src/proxy/processors/constants'; +import { EMPTY_COMMIT_HASH } from '../src/proxy/constants'; import { Request } from 'express'; // must hoist these before mocking the modules diff --git a/test/db/db.test.ts b/test/db/db.test.ts index 88a65eae4..b245be501 100644 --- a/test/db/db.test.ts +++ b/test/db/db.test.ts @@ -15,7 +15,7 @@ */ import { describe, it, expect, afterEach, vi, beforeEach } from 'vitest'; -import { SAMPLE_REPO } from '../../src/proxy/processors/constants'; +import { SAMPLE_REPO } from '../../src/proxy/constants'; vi.mock('../../src/db/mongo', () => ({ getRepoByUrl: vi.fn(), diff --git a/test/integration/forcePush.integration.test.ts b/test/integration/forcePush.integration.test.ts index 54d282fd6..843614c60 100644 --- a/test/integration/forcePush.integration.test.ts +++ b/test/integration/forcePush.integration.test.ts @@ -23,7 +23,7 @@ import { Request } from 'express'; import { Action, Step } from '../../src/proxy/actions'; import { exec as getDiff } from '../../src/proxy/processors/push-action/getDiff'; import { exec as scanDiff } from '../../src/proxy/processors/push-action/scanDiff'; -import { SAMPLE_COMMIT } from '../../src/proxy/processors/constants'; +import { SAMPLE_COMMIT } from '../../src/proxy/constants'; describe( 'Force Push Integration Test', diff --git a/test/processors/checkAuthorEmails.test.ts b/test/processors/checkAuthorEmails.test.ts index 7ec6339fb..dcc8c72ac 100644 --- a/test/processors/checkAuthorEmails.test.ts +++ b/test/processors/checkAuthorEmails.test.ts @@ -20,7 +20,7 @@ import { exec } from '../../src/proxy/processors/push-action/checkAuthorEmails'; import { Action } from '../../src/proxy/actions'; import * as configModule from '../../src/config'; import * as validator from 'validator'; -import { SAMPLE_COMMIT } from '../../src/proxy/processors/constants'; +import { SAMPLE_COMMIT } from '../../src/proxy/constants'; // mock dependencies vi.mock('../../src/config', async (importOriginal) => { diff --git a/test/processors/checkEmptyBranch.test.ts b/test/processors/checkEmptyBranch.test.ts index 115d4bc0b..64839e541 100644 --- a/test/processors/checkEmptyBranch.test.ts +++ b/test/processors/checkEmptyBranch.test.ts @@ -17,7 +17,7 @@ import { Request } from 'express'; import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { Action } from '../../src/proxy/actions'; -import { EMPTY_COMMIT_HASH, SAMPLE_COMMIT } from '../../src/proxy/processors/constants'; +import { EMPTY_COMMIT_HASH, SAMPLE_COMMIT } from '../../src/proxy/constants'; vi.mock('simple-git'); vi.mock('fs'); diff --git a/test/processors/checkMessages.test.ts b/test/processors/checkMessages.test.ts index 2d2e3751a..f8dd5f3f6 100644 --- a/test/processors/checkMessages.test.ts +++ b/test/processors/checkMessages.test.ts @@ -19,7 +19,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { exec } from '../../src/proxy/processors/push-action/checkMessages'; import { Action } from '../../src/proxy/actions'; import * as configModule from '../../src/config'; -import { SAMPLE_COMMIT } from '../../src/proxy/processors/constants'; +import { SAMPLE_COMMIT } from '../../src/proxy/constants'; vi.mock('../../src/config', async (importOriginal) => { const actual = await importOriginal(); diff --git a/test/processors/getDiff.test.ts b/test/processors/getDiff.test.ts index 65eb45157..af2f4eb4a 100644 --- a/test/processors/getDiff.test.ts +++ b/test/processors/getDiff.test.ts @@ -23,7 +23,7 @@ import fc from 'fast-check'; import { Action } from '../../src/proxy/actions'; import { exec } from '../../src/proxy/processors/push-action/getDiff'; -import { EMPTY_COMMIT_HASH, SAMPLE_COMMIT } from '../../src/proxy/processors/constants'; +import { EMPTY_COMMIT_HASH, SAMPLE_COMMIT } from '../../src/proxy/constants'; describe('getDiff', () => { let tempDir: string; diff --git a/test/testPush.test.ts b/test/testPush.test.ts index 8bf85788d..f4cef13c7 100644 --- a/test/testPush.test.ts +++ b/test/testPush.test.ts @@ -21,7 +21,7 @@ import { Service } from '../src/service'; import { Proxy } from '../src/proxy'; import { Express } from 'express'; import { Action } from '../src/proxy/actions/Action'; -import { EMPTY_COMMIT_HASH } from '../src/proxy/processors/constants'; +import { EMPTY_COMMIT_HASH } from '../src/proxy/constants'; // dummy repo const TEST_ORG = 'finos'; From 987ec9a81bc561c2376cc5820af6184b5d3c315c Mon Sep 17 00:00:00 2001 From: Juan Escalada <97265671+jescalada@users.noreply.github.com> Date: Mon, 6 Jul 2026 15:55:21 +0900 Subject: [PATCH 10/13] fix: server-side request forgery issue Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Juan Escalada <97265671+jescalada@users.noreply.github.com> --- src/proxy/routes/index.ts | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/proxy/routes/index.ts b/src/proxy/routes/index.ts index f064b4c93..583278d2d 100644 --- a/src/proxy/routes/index.ts +++ b/src/proxy/routes/index.ts @@ -341,13 +341,27 @@ const endStreamedResponseWithMessage = (res: Response, message: string): void => * @return {URL} The full upstream URL to forward the request to. */ const resolveUpstreamUrl = (req: Request, originsToProxy: string[]): URL => { + const originalUrl = req.originalUrl || '/'; + for (const origin of originsToProxy) { - if (req.originalUrl.startsWith(`/${origin}/`)) { - return new URL(`https:/${req.originalUrl}`); + const prefix = `/${origin}`; + if (originalUrl === prefix || originalUrl.startsWith(`${prefix}/`)) { + const upstreamBase = new URL(`https://${origin}`); + const strippedPathAndQuery = originalUrl.slice(prefix.length) || '/'; + const safeRelative = + strippedPathAndQuery.startsWith('/') && !strippedPathAndQuery.startsWith('//') + ? strippedPathAndQuery + : `/${strippedPathAndQuery.replace(/^\/+/, '')}`; + return new URL(safeRelative, upstreamBase); } } + // fallback (legacy URLs without an origin prefix) - return new URL(`https://github.com${req.originalUrl}`); + const safeFallbackRelative = + originalUrl.startsWith('/') && !originalUrl.startsWith('//') + ? originalUrl + : `/${originalUrl.replace(/^\/+/, '')}`; + return new URL(safeFallbackRelative, 'https://github.com'); }; /** Headers that must not be forwarded verbatim to the upstream host. From 9d040bc8891ef893bf13bffbac4fa551fde7291d Mon Sep 17 00:00:00 2001 From: Juan Escalada <97265671+jescalada@users.noreply.github.com> Date: Mon, 6 Jul 2026 15:57:46 +0900 Subject: [PATCH 11/13] fix: type confusion on http request parameter Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Juan Escalada <97265671+jescalada@users.noreply.github.com> --- src/proxy/routes/index.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/proxy/routes/index.ts b/src/proxy/routes/index.ts index 583278d2d..c2f545f64 100644 --- a/src/proxy/routes/index.ts +++ b/src/proxy/routes/index.ts @@ -399,6 +399,10 @@ const forwardReceivePackUpstream = async ( res: Response, target: URL, ): Promise => { + if (!Buffer.isBuffer(req.body)) { + res.status(400).send('Bad request'); + return; + } const body: Buffer = req.body; const client = target.protocol === 'http:' ? http : https; From e8e0db5fb9ab716361346e845a0ad15c42663f96 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Tue, 7 Jul 2026 11:49:05 +0900 Subject: [PATCH 12/13] fix: fatal error on rejected pushes or waiting approval --- src/proxy/constants.ts | 4 +++ src/proxy/routes/index.ts | 72 +++++++++++++++++++++++++++++++------ src/proxy/sideband.ts | 75 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 139 insertions(+), 12 deletions(-) diff --git a/src/proxy/constants.ts b/src/proxy/constants.ts index 306749a81..bf2a8f63b 100644 --- a/src/proxy/constants.ts +++ b/src/proxy/constants.ts @@ -55,3 +55,7 @@ export const MAX_SIDEBAND_PAYLOAD_BYTES = 65515; /** Capability that must be negotiated by the client for streamed progress */ export const SIDE_BAND_64K_CAPABILITY = 'side-band-64k'; + +/** Capabilities with which the client requests a receive-pack status report. + * The `ng` (rejected ref) line syntax is identical in both protocol versions. */ +export const REPORT_STATUS_CAPABILITIES = ['report-status', 'report-status-v2']; diff --git a/src/proxy/routes/index.ts b/src/proxy/routes/index.ts index c2f545f64..99909bc71 100644 --- a/src/proxy/routes/index.ts +++ b/src/proxy/routes/index.ts @@ -29,7 +29,8 @@ import { getUpstreamProxyConfig } from '../../config'; import { HttpsProxyAgent } from 'https-proxy-agent'; import http, { OutgoingHttpHeaders, RequestOptions } from 'http'; import https from 'https'; -import { encodeSidebandChunk, SidebandBand } from '../sideband'; +import { buildRejectionReportStatus, encodeSidebandChunk, SidebandBand } from '../sideband'; +import { Action } from '../actions'; import { FLUSH_PACKET } from '../constants'; enum ActionType { @@ -83,9 +84,10 @@ const proxyFilter: ProxyOptions['filter'] = async (req, res) => { if (action.error || action.blocked) { const message = action.errorMessage ?? action.blockedMessage ?? 'Unknown error'; const type = action.error ? ActionType.ERROR : ActionType.BLOCKED; + const statusReport = buildRejectionReportStatus(action, rejectionReasonFor(action)); logAction(req.url, req.headers.host, req.headers['user-agent'], type, message); - sendErrorResponse(req, res, message); + sendErrorResponse(req, res, message, statusReport); return false; } @@ -102,7 +104,12 @@ const proxyFilter: ProxyOptions['filter'] = async (req, res) => { } }; -const sendErrorResponse = (req: Request, res: Response, message: string): void => { +const sendErrorResponse = ( + req: Request, + res: Response, + message: string, + statusReport?: Buffer, +): void => { // GET requests to /info/refs (used to check refs for many git operations) must use Git protocol error packet format if (req.method === 'GET' && req.url.includes('/info/refs')) { res.set('content-type', 'application/x-git-upload-pack-advertisement'); @@ -119,9 +126,33 @@ const sendErrorResponse = (req: Request, res: Response, message: string): void = res.set('x-frame-options', 'DENY'); res.set('connection', 'close'); + if (statusReport) { + // Allow git client to end the session cleanly instead of + // giving "fatal: the remote end hung up unexpectedly" + res + .status(200) + .send( + Buffer.concat([ + encodeSidebandChunk(SidebandBand.Progress, `\t${message}\n`), + statusReport, + Buffer.from(FLUSH_PACKET, 'ascii'), + ]), + ); + return; + } + res.status(200).send(handleMessage(message)); }; +/** + * The reason git shows in its "! [remote rejected] ()" line + * for a push that GitProxy did not forward upstream. + * @param {Action} action The completed push action. + * @return {string} A short, single-line rejection reason. + */ +const rejectionReasonFor = (action: Action): string => + action.error ? 'rejected by GitProxy' : 'approval required'; + const handleMessage = (message: string): string => { const packet = encodeSidebandChunk(SidebandBand.Progress, `\t${message}\n`); return packet.toString('utf8') + FLUSH_PACKET; @@ -324,12 +355,21 @@ const isReceivePackPost = (req: Request): boolean => * Finish a response that has already started streaming sideband progress * @param {Response} res The in-progress streaming response * @param {string} message The final message to display in the client terminal + * @param {Buffer} [statusReport] Optional synthesized band-1 report-status, + * written before the final flush so the client ends the session cleanly. */ -const endStreamedResponseWithMessage = (res: Response, message: string): void => { +const endStreamedResponseWithMessage = ( + res: Response, + message: string, + statusReport?: Buffer, +): void => { if (res.writableEnded) { return; } res.write(encodeSidebandChunk(SidebandBand.Progress, `\t${message}\n`)); + if (statusReport) { + res.write(statusReport); + } res.write(FLUSH_PACKET); res.end(); }; @@ -392,12 +432,15 @@ const UPSTREAM_HEADER_BLOCKLIST = [ * @param {Request} req The client request (body must be a Buffer). * @param {Response} res The client response. * @param {URL} target The upstream URL to forward to. + * @param {Buffer} [failureStatusReport] Optional synthesized band-1 report-status + * appended when upstream fails after streaming started. * @return {Promise} Resolves when the upstream response has been relayed. */ const forwardReceivePackUpstream = async ( req: Request, res: Response, target: URL, + failureStatusReport?: Buffer, ): Promise => { if (!Buffer.isBuffer(req.body)) { res.status(400).send('Bad request'); @@ -434,6 +477,7 @@ const forwardReceivePackUpstream = async ( endStreamedResponseWithMessage( res, `Push failed: upstream responded with status ${upstreamRes.statusCode}`, + failureStatusReport, ); return; } @@ -473,6 +517,7 @@ const createReceivePackHandler = (originsToProxy: string[]): RequestHandler => { return next(); } + let action: Action | undefined; try { const urlComponents = processUrlPath(req.url); if ( @@ -492,31 +537,38 @@ const createReceivePackHandler = (originsToProxy: string[]): RequestHandler => { delete req.bodyRaw; } - const action = await executeChain(req, res); + action = await executeChain(req, res); if (action.error || action.blocked) { const message = action.errorMessage ?? action.blockedMessage ?? 'Unknown error'; const type = action.error ? ActionType.ERROR : ActionType.BLOCKED; + const statusReport = buildRejectionReportStatus(action, rejectionReasonFor(action)); logAction(req.url, req.headers.host, req.headers['user-agent'], type, message); if (res.headersSent) { - endStreamedResponseWithMessage(res, message); + endStreamedResponseWithMessage(res, message, statusReport); } else { - sendErrorResponse(req, res, message); + sendErrorResponse(req, res, message, statusReport); } return; } logAction(req.url, req.headers.host, req.headers['user-agent'], ActionType.ALLOWED); - await forwardReceivePackUpstream(req, res, resolveUpstreamUrl(req, originsToProxy)); + await forwardReceivePackUpstream( + req, + res, + resolveUpstreamUrl(req, originsToProxy), + buildRejectionReportStatus(action, 'upstream error'), + ); } catch (error: unknown) { const message = handleErrorAndLog(error, 'Error processing git-receive-pack request'); + const statusReport = buildRejectionReportStatus(action, 'internal error'); logAction(req.url, req.headers.host, req.headers['user-agent'], ActionType.ERROR, message); if (res.headersSent) { - endStreamedResponseWithMessage(res, message); + endStreamedResponseWithMessage(res, message, statusReport); } else { - sendErrorResponse(req, res, message); + sendErrorResponse(req, res, message, statusReport); } } }; diff --git a/src/proxy/sideband.ts b/src/proxy/sideband.ts index b2928d942..0fcc2d314 100644 --- a/src/proxy/sideband.ts +++ b/src/proxy/sideband.ts @@ -16,9 +16,14 @@ import { Response } from 'express'; -import { Action, RequestType } from './actions'; +import { Action, RequestType, PushType } from './actions'; import { getSidebandProgressEnabled } from '../config'; -import { MAX_SIDEBAND_PAYLOAD_BYTES, SIDE_BAND_64K_CAPABILITY } from './constants'; +import { + FLUSH_PACKET, + MAX_SIDEBAND_PAYLOAD_BYTES, + REPORT_STATUS_CAPABILITIES, + SIDE_BAND_64K_CAPABILITY, +} from './constants'; /** Sideband channel numbers - see https://git-scm.com/docs/protocol-common */ export enum SidebandBand { @@ -52,6 +57,72 @@ export const encodeSidebandChunk = (band: SidebandBand, payload: string | Buffer return Buffer.concat(packets); }; +/** + * Encode a single git pkt-line (4 hex length bytes followed by the payload). + * @param {string} line The line to encode (should include its trailing newline). + * @return {Buffer} The encoded pkt-line. + */ +export const encodePacketLine = (line: string): Buffer => { + const data = Buffer.from(line, 'utf8'); + const length = (4 + data.length).toString(16).padStart(4, '0'); + return Buffer.concat([Buffer.from(length, 'ascii'), data]); +}; + +/** + * Encode a synthesized receive-pack report-status that rejects every pushed + * ref, wrapped in sideband band-1 packets to provide status report + * + * Completing the report-status exchange is what prevents the git client from + * printing "fatal: the remote end hung up unexpectedly" when the proxy ends + * the session without contacting upstream. Instead the client shows a normal + * "! [remote rejected] ()" line. + * + * See https://git-scm.com/docs/pack-protocol#_report_status for more details + * @param {string[]} refs The refs the client attempted to update. + * @param {string} reason Short single-line reason shown by git in parentheses. + * @return {Buffer} Band-1 packet(s) carrying the report-status. + */ +export const encodeRejectionReportStatus = (refs: string[], reason: string): Buffer => { + const safeReason = reason.replace(/[\0\r\n]+/g, ' ').trim() || 'rejected'; + const report = Buffer.concat([ + encodePacketLine('unpack ok\n'), + ...refs.map((ref) => encodePacketLine(`ng ${ref} ${safeReason}\n`)), + Buffer.from(FLUSH_PACKET, 'ascii'), + ]); + return encodeSidebandChunk(SidebandBand.Data, report); +}; + +/** + * Build the synthesized report-status for a blocked/rejected push, or + * undefined when it cannot (or should not) be produced + * @param {Action | undefined} action The push action (undefined if chain never ran) + * @param {string} reason Short single-line reason shown by git in parentheses. + * @return {Buffer | undefined} Band-1 packet(s) carrying the report, or undefined. + */ +export const buildRejectionReportStatus = ( + action: Action | undefined, + reason: string, +): Buffer | undefined => { + if (!action) { + return undefined; + } + const capabilities = action.capabilities; + if (!capabilities?.includes(SIDE_BAND_64K_CAPABILITY)) { + return undefined; + } + if (!capabilities.some((cap) => REPORT_STATUS_CAPABILITIES.includes(cap))) { + return undefined; + } + + const refs = + action.actionType === PushType.TAG ? (action.tags ?? []) : action.branch ? [action.branch] : []; + if (refs.length === 0) { + return undefined; + } + + return encodeRejectionReportStatus(refs, reason); +}; + /** * Writer used by the push chain to stream per-step progress messages to the * git client's terminal while validation runs. From b4f79179b3168871b3e1789c47710983550ff753 Mon Sep 17 00:00:00 2001 From: Juan Escalada Date: Tue, 7 Jul 2026 18:57:30 +0900 Subject: [PATCH 13/13] test: rejection report status and descriptive git client error messages --- test/testProxyRoute.test.ts | 101 ++++++++++++++++++++++++++++- test/testSideband.test.ts | 123 +++++++++++++++++++++++++++++++++++- 2 files changed, 221 insertions(+), 3 deletions(-) diff --git a/test/testProxyRoute.test.ts b/test/testProxyRoute.test.ts index 2c516cfba..341035e21 100644 --- a/test/testProxyRoute.test.ts +++ b/test/testProxyRoute.test.ts @@ -37,7 +37,11 @@ import { forwardReceivePackUpstream, endStreamedResponseWithMessage, } from '../src/proxy/routes'; -import { encodeSidebandChunk, SidebandBand } from '../src/proxy/sideband'; +import { + encodeSidebandChunk, + encodeRejectionReportStatus, + SidebandBand, +} from '../src/proxy/sideband'; import * as db from '../src/db'; import { Service } from '../src/service'; @@ -723,6 +727,29 @@ describe('endStreamedResponseWithMessage', () => { expect(res.write).not.toHaveBeenCalled(); expect(res.end).not.toHaveBeenCalled(); }); + + it('should write the status report between the message and the final flush', () => { + const writes: Buffer[] = []; + const res = { + writableEnded: false, + write: vi.fn((chunk: Buffer | string) => { + writes.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); + return true; + }), + end: vi.fn(), + } as unknown as Response; + const report = encodeRejectionReportStatus(['refs/heads/main'], 'approval required'); + + endStreamedResponseWithMessage(res, 'Push blocked', report); + + const body = Buffer.concat(writes).toString('utf8'); + expect(body).toContain('Push blocked\n'); + expect(body).toContain('unpack ok\n'); + expect(body).toContain('ng refs/heads/main approval required\n'); + // synthesized report (with its inner flush) comes before the outer flush + expect(body.endsWith('0000' + '0000')).toBe(true); + expect(res.end).toHaveBeenCalledOnce(); + }); }); describe('createReceivePackHandler', () => { @@ -844,6 +871,78 @@ describe('createReceivePackHandler', () => { expect(mockRes.end).toHaveBeenCalledOnce(); }); + it('should append a synthesized report-status when the client requested one (buffered)', async () => { + vi.spyOn(helper, 'processUrlPath').mockReturnValue({ + gitPath: '/finos/git-proxy.git/git-receive-pack', + repoPath: 'github.com', + }); + vi.spyOn(helper, 'validGitRequest').mockReturnValue(true); + vi.spyOn(chain, 'executeChain').mockResolvedValue({ + error: false, + blocked: true, + blockedMessage: 'Push queued for approval', + capabilities: ['report-status', 'side-band-64k', 'agent=git/2.42.0'], + branch: 'refs/heads/main', + } as Action); + + await handler(mockReq as Request, mockRes as Response, nextMock); + + const sent = vi.mocked(mockRes.send!).mock.calls[0][0]; + expect(Buffer.isBuffer(sent)).toBe(true); + const body = (sent as Buffer).toString('utf8'); + expect(body).toContain('Push queued for approval'); + expect(body).toContain('unpack ok\n'); + expect(body).toContain('ng refs/heads/main approval required\n'); + expect(body.endsWith('0000' + '0000')).toBe(true); + }); + + it('should append a synthesized report-status when the chain errored after streaming started', async () => { + vi.spyOn(helper, 'processUrlPath').mockReturnValue({ + gitPath: '/finos/git-proxy.git/git-receive-pack', + repoPath: 'github.com', + }); + vi.spyOn(helper, 'validGitRequest').mockReturnValue(true); + vi.spyOn(chain, 'executeChain').mockImplementation(async () => { + mockRes.headersSent = true; + return { + error: true, + blocked: false, + errorMessage: 'Secret detected', + capabilities: ['report-status-v2', 'side-band-64k'], + branch: 'refs/heads/feature', + } as Action; + }); + + await handler(mockReq as Request, mockRes as Response, nextMock); + + expect(mockRes.send).not.toHaveBeenCalled(); + const body = Buffer.concat(writes).toString('utf8'); + expect(body).toContain('Secret detected'); + expect(body).toContain('unpack ok\n'); + expect(body).toContain('ng refs/heads/feature rejected by GitProxy\n'); + expect(mockRes.end).toHaveBeenCalledOnce(); + }); + + it('should not append a report-status when the client did not request one', async () => { + vi.spyOn(helper, 'processUrlPath').mockReturnValue({ + gitPath: '/finos/git-proxy.git/git-receive-pack', + repoPath: 'github.com', + }); + vi.spyOn(helper, 'validGitRequest').mockReturnValue(true); + vi.spyOn(chain, 'executeChain').mockResolvedValue({ + error: false, + blocked: true, + blockedMessage: 'Push blocked by policy', + capabilities: ['side-band-64k'], + branch: 'refs/heads/main', + } as Action); + + await handler(mockReq as Request, mockRes as Response, nextMock); + + const sent = vi.mocked(mockRes.send!).mock.calls[0][0]; + expect(sent.toString()).not.toContain('unpack ok'); + }); + it('should move the raw body onto req.body before invoking the chain', async () => { vi.spyOn(helper, 'processUrlPath').mockReturnValue({ gitPath: '/finos/git-proxy.git/git-receive-pack', diff --git a/test/testSideband.test.ts b/test/testSideband.test.ts index 87fcce8d1..e0a777cb8 100644 --- a/test/testSideband.test.ts +++ b/test/testSideband.test.ts @@ -19,14 +19,21 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { encodeSidebandChunk, + encodePacketLine, + encodeRejectionReportStatus, + buildRejectionReportStatus, SidebandBand, SidebandProgressWriter, NOOP_PROGRESS_WRITER, createProgressWriter, } from '../src/proxy/sideband'; -import { MAX_SIDEBAND_PAYLOAD_BYTES, SIDE_BAND_64K_CAPABILITY } from '../src/proxy/constants'; +import { + FLUSH_PACKET, + MAX_SIDEBAND_PAYLOAD_BYTES, + SIDE_BAND_64K_CAPABILITY, +} from '../src/proxy/constants'; import * as config from '../src/config'; -import { Action, RequestType } from '../src/proxy/actions'; +import { Action, RequestType, PushType } from '../src/proxy/actions'; const createMockRes = () => { const writes: Buffer[] = []; @@ -100,6 +107,118 @@ describe('encodeSidebandChunk', () => { }); }); +describe('encodePacketLine', () => { + it('should prefix the payload with its 4-byte hex length', () => { + const packet = encodePacketLine('unpack ok\n'); + + // 4 length bytes + 10 payload bytes = 14 = 0x000e + expect(packet.toString('utf8')).toBe('000eunpack ok\n'); + }); +}); + +describe('encodeRejectionReportStatus', () => { + it('should wrap unpack ok, ng lines and an inner flush in a band-1 packet', () => { + const packet = encodeRejectionReportStatus(['refs/heads/main'], 'approval required'); + + expect(packet[4]).toBe(SidebandBand.Data); + const payload = packet.subarray(5).toString('utf8'); + expect(payload).toBe( + '000eunpack ok\n' + '0029ng refs/heads/main approval required\n' + FLUSH_PACKET, + ); + }); + + it('should emit one ng line per ref', () => { + const packet = encodeRejectionReportStatus( + ['refs/heads/main', 'refs/tags/v1.0.0'], + 'rejected by GitProxy', + ); + const payload = packet.subarray(5).toString('utf8'); + + expect(payload).toContain('ng refs/heads/main rejected by GitProxy\n'); + expect(payload).toContain('ng refs/tags/v1.0.0 rejected by GitProxy\n'); + expect(payload.endsWith(FLUSH_PACKET)).toBe(true); + }); + + it('should sanitize multi-line reasons into a single line', () => { + const packet = encodeRejectionReportStatus(['refs/heads/main'], 'first\nsecond\r\nthird'); + const payload = packet.subarray(5).toString('utf8'); + + expect(payload).toContain('ng refs/heads/main first second third\n'); + }); + + it('should fall back to a generic reason when the reason is empty', () => { + const packet = encodeRejectionReportStatus(['refs/heads/main'], '\n'); + const payload = packet.subarray(5).toString('utf8'); + + expect(payload).toContain('ng refs/heads/main rejected\n'); + }); +}); + +describe('buildRejectionReportStatus', () => { + const reportAction = (overrides: Partial = {}): Action => + pushAction({ branch: 'refs/heads/main', ...overrides }); + + it('should build a report for a branch push with report-status and side-band-64k', () => { + const report = buildRejectionReportStatus(reportAction(), 'approval required'); + + expect(report).toBeInstanceOf(Buffer); + expect(report!.subarray(5).toString('utf8')).toContain( + 'ng refs/heads/main approval required\n', + ); + }); + + it('should accept report-status-v2', () => { + const report = buildRejectionReportStatus( + reportAction({ capabilities: ['report-status-v2', SIDE_BAND_64K_CAPABILITY] }), + 'approval required', + ); + + expect(report).toBeInstanceOf(Buffer); + }); + + it('should reject all tag refs for a tag push', () => { + const report = buildRejectionReportStatus( + reportAction({ actionType: PushType.TAG, tags: ['refs/tags/v1.0.0', 'refs/tags/v1.0.1'] }), + 'rejected by GitProxy', + ); + + const payload = report!.subarray(5).toString('utf8'); + expect(payload).toContain('ng refs/tags/v1.0.0 rejected by GitProxy\n'); + expect(payload).toContain('ng refs/tags/v1.0.1 rejected by GitProxy\n'); + }); + + it('should return undefined when the action is missing', () => { + expect(buildRejectionReportStatus(undefined, 'reason')).toBeUndefined(); + }); + + it('should return undefined when side-band-64k was not negotiated', () => { + expect( + buildRejectionReportStatus(reportAction({ capabilities: ['report-status'] }), 'reason'), + ).toBeUndefined(); + }); + + it('should return undefined when the client did not request a status report', () => { + expect( + buildRejectionReportStatus( + reportAction({ capabilities: [SIDE_BAND_64K_CAPABILITY] }), + 'reason', + ), + ).toBeUndefined(); + }); + + it('should return undefined when no refs are known', () => { + expect( + buildRejectionReportStatus(reportAction({ branch: undefined }), 'reason'), + ).toBeUndefined(); + expect( + buildRejectionReportStatus( + reportAction({ actionType: PushType.TAG, branch: undefined, tags: [] }), + 'reason', + ), + ).toBeUndefined(); + }); +}); + describe('SidebandProgressWriter', () => { afterEach(() => { vi.restoreAllMocks();