Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7466,6 +7466,19 @@ async function downloadJobArtifacts(
jobId: string,
localPath?: string
): Promise<string> {
if (localPath) {
const normalizedLocalPath = path.normalize(localPath);
if (
path.isAbsolute(normalizedLocalPath) ||
path.parse(normalizedLocalPath).root !== "" ||
normalizedLocalPath === ".." ||
normalizedLocalPath.startsWith(".." + path.sep) ||
normalizedLocalPath.includes(path.sep + ".." + path.sep)
) {
throw new Error("Invalid local_path: directory traversal is not allowed.");
}
}

projectId = decodeURIComponent(projectId);
const effectiveProjectId = getEffectiveProjectId(projectId);

Expand All @@ -7486,7 +7499,7 @@ async function downloadJobArtifacts(
await handleGitLabError(response);

const filename = `artifacts_job_${encodeGitLabPathSegment(jobId)}.zip`;
const savePath = localPath ? path.join(localPath, filename) : filename;
const savePath = localPath ? path.join(path.normalize(localPath), filename) : filename;
fs.mkdirSync(path.dirname(savePath), { recursive: true });

if (!response.body) {
Expand Down
47 changes: 44 additions & 3 deletions test/test-job-artifacts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { spawn } from 'child_process';
import { MockGitLabServer, findMockServerPort } from './utils/mock-gitlab-server.js';
import fs from 'node:fs';
import path from 'node:path';
import os from 'node:os';

const MOCK_TOKEN = 'glpat-mock-token-12345';
const TEST_PROJECT_ID = '123';
Expand Down Expand Up @@ -143,8 +142,12 @@ describe('job artifacts tools', () => {
await mockGitLab.start();
mockGitLabUrl = mockGitLab.getUrl();

// Create a temp directory for download tests
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitlab-mcp-test-'));
// Create a temp directory for download tests. Must be relative to the
// process cwd — download_job_artifacts rejects absolute local_path
// values as directory traversal (see downloadJobArtifacts/downloadAttachment).
tmpDir = `gitlab-mcp-test-artifacts-${process.pid}`;
fs.rmSync(tmpDir, { recursive: true, force: true });
fs.mkdirSync(tmpDir, { recursive: true });
});

after(async () => {
Expand Down Expand Up @@ -210,6 +213,44 @@ describe('job artifacts tools', () => {
assert.ok(fs.existsSync(nestedLocalPath), `Directory should be created at ${nestedLocalPath}`);
});

test('download_job_artifacts rejects local_path directory traversal', async () => {
try {
await callTool(
'download_job_artifacts',
{ project_id: TEST_PROJECT_ID, job_id: TEST_JOB_ID, local_path: '../../../tmp' },
{
GITLAB_API_URL: `${mockGitLabUrl}/api/v4`,
GITLAB_PERSONAL_ACCESS_TOKEN: MOCK_TOKEN,
}
);
assert.fail('Expected download_job_artifacts to reject a traversal local_path');
} catch (error: any) {
assert.ok(
typeof error?.message === 'string' && error.message.toLowerCase().includes('traversal'),
`Expected a traversal error, got: ${error?.message}`
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
);
}
});

test('download_job_artifacts rejects absolute local_path', async () => {
try {
await callTool(
'download_job_artifacts',
{ project_id: TEST_PROJECT_ID, job_id: TEST_JOB_ID, local_path: '/tmp' },
{
GITLAB_API_URL: `${mockGitLabUrl}/api/v4`,
GITLAB_PERSONAL_ACCESS_TOKEN: MOCK_TOKEN,
}
);
assert.fail('Expected download_job_artifacts to reject an absolute local_path');
} catch (error: any) {
assert.ok(
typeof error?.message === 'string' && error.message.toLowerCase().includes('traversal'),
`Expected a traversal error, got: ${error?.message}`
);
}
});

test('get_job_artifact_file returns file content', async () => {
const result = await callTool(
'get_job_artifact_file',
Expand Down