Skip to content
Open
Show file tree
Hide file tree
Changes from 5 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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ build
.env
.env.local
.env.test
.secrets
coverage/
*.log
# ai
Expand Down
3 changes: 0 additions & 3 deletions .secrets

This file was deleted.

2 changes: 1 addition & 1 deletion config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export const OAUTH_STATELESS_PENDING_TTL_SECONDS = _intEnv(
export const OAUTH_STATELESS_STORED_TTL_SECONDS = _intEnv(
"OAUTH_STATELESS_STORED_TTL_SECONDS",
"oauth-stateless-stored-ttl",
600
120
);

// ---------------------------------------------------------------------------
Expand Down
5 changes: 4 additions & 1 deletion docs/configuration/environment-variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,10 @@ Max age for a sealed OAuth `state` during the callback-proxy `/authorize`
### `OAUTH_STATELESS_STORED_TTL_SECONDS`

Max age for a sealed proxy authorization `code` during the `/callback` →
`/token` hop. Default `600` (10 min).
`/token` hop. Default `120` (2 min). Kept short because sealed codes cannot
enforce cross-pod one-time use; each pod also tracks in-flight and consumed
code hashes in a TTL-bound in-memory cache (no early LRU eviction; fail
closed when full).

### `OAUTH_STATELESS_SESSION_TTL_SECONDS`

Expand Down
19 changes: 11 additions & 8 deletions docs/configuration/stateless-mode.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ Cryptography:
| `OAUTH_STATELESS_SECRET_PREVIOUS` | — | Optional. Accepted on reads only, for rotation. |
| `OAUTH_STATELESS_CLIENT_TTL_SECONDS` | `86400` | Max age for a signed `client_id`. |
| `OAUTH_STATELESS_PENDING_TTL_SECONDS` | `600` | Max age for a sealed OAuth `state`. |
| `OAUTH_STATELESS_STORED_TTL_SECONDS` | `600` | Max age for a sealed proxy `code`. |
| `OAUTH_STATELESS_STORED_TTL_SECONDS` | `120` | Max age for a sealed proxy `code`. |
| `OAUTH_STATELESS_SESSION_TTL_SECONDS` | inherits `SESSION_TIMEOUT_SECONDS` | Inactivity timeout for a sealed `Mcp-Session-Id`. |

CLI arguments take the same names with dashes (e.g. `--oauth-stateless-mode=true`).
Expand Down Expand Up @@ -126,17 +126,20 @@ re-authenticate.

- **OAuth `state`** — replay is tolerated. A replayed state without a matching
valid GitLab auth code yields nothing; GitLab's code is single-use.
- **Proxy `code`** — replay is defeated by the existing PKCE check. An
attacker replaying the code without the matching `code_verifier` fails at
`/token`. Combined with the 10 minute TTL.
- **Proxy `code`** — replay is defeated by PKCE, a short TTL (default 120s),
and a per-pod TTL-bound cache of code hashes (pending while validating,
consumed after a successful `/token`). Failed client/PKCE checks release the
reservation so the legitimate client can retry. Entries are never LRU-evicted
before their TTL; when the cache is full the server fails closed.
- **`Mcp-Session-Id`** — replay is equivalent to presenting the stolen bearer
token, which is a known threat model at the HTTP layer. TLS and operator
discipline on log redaction protect this surface.

One-time-use semantics cannot be enforced in stateless mode without a shared
store. This is an explicit design trade-off: the plan chose "no external
dependency" over "strict one-time use" because the PKCE + TTL combination
provides equivalent practical security.
One-time-use semantics cannot be enforced *across pods* in stateless mode
without a shared store. This is an explicit design trade-off: the plan chose
"no external dependency" over "strict cross-pod one-time use" because the
PKCE + short TTL + per-pod replay cache combination provides equivalent
practical security.
Comment thread
mbathla-sudo marked this conversation as resolved.

## Operational notes

Expand Down
51 changes: 21 additions & 30 deletions index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ import { normalizeGitLabApiUrl } from "./utils/url.js";
import {
estimateMergeCommitCount,
filterDiffsByPatterns,
openSafeOutputWriteStream,
readSafeExistingFile,
summarizeWebhookEvents,
} from "./utils/helpers.js";
import {
Expand Down Expand Up @@ -7486,13 +7488,16 @@ async function downloadJobArtifacts(
await handleGitLabError(response);

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

if (!response.body) {
throw new Error("No response body from GitLab");
}
await streamPipeline(response.body, fs.createWriteStream(savePath));
await streamPipeline(response.body, saveStream);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

return savePath;
}
Expand Down Expand Up @@ -9378,12 +9383,10 @@ async function markdownUpload(
fileBuffer = Buffer.from(content, "base64");
fileName = filename || "upload";
} else if (filePath) {
// Local file mode
if (!fs.existsSync(filePath)) {
throw new Error(`File not found: ${filePath}`);
}
fileBuffer = fs.readFileSync(filePath);
fileName = path.basename(filePath);
// Local file mode — reject absolute/traversal/symlink escapes before reading
const { buffer, basename: safeBasename } = readSafeExistingFile(filePath, "file_path");
fileBuffer = buffer;
fileName = safeBasename;
} else {
throw new Error("Either file_path or content must be provided");
}
Expand Down Expand Up @@ -9481,31 +9484,17 @@ async function downloadAttachment(
// For non-image files, always save to disk.
// For image files, only save to disk if local_path is explicitly provided.
if (!mimeType || localPath) {
let savePath: string;
if (localPath) {
const normalizedLocalPath = path.normalize(localPath);
if (
path.isAbsolute(normalizedLocalPath) ||
normalizedLocalPath === ".." ||
normalizedLocalPath.startsWith(".." + path.sep) ||
normalizedLocalPath.includes(path.sep + ".." + path.sep)
) {
throw new Error("Invalid local_path: directory traversal is not allowed.");
}
savePath = path.join(normalizedLocalPath, safeFilename);
} else {
savePath = safeFilename;
}
const dir = path.dirname(savePath);
if (!fs.existsSync(dir)) {
fs.mkdirSync(dir, { recursive: true });
}
const { stream: saveStream, path: savePath } = openSafeOutputWriteStream(
safeFilename,
localPath,
"local_path"
);

// Stream directly to disk instead of buffering in memory
if (!response.body) {
throw new Error("No response body from GitLab");
}
await streamPipeline(response.body, fs.createWriteStream(savePath));
await streamPipeline(response.body, saveStream);
return { buffer: Buffer.alloc(0), filename: safeFilename, mimeType, savedPath: savePath };
}

Expand Down Expand Up @@ -13003,7 +12992,9 @@ async function handleToolCall(params: any) {
throw new Error(`Unknown tool: ${params.name}`);
}
} catch (error) {
logger.debug(params);
// Log tool name only — never dump raw params (may contain approval_password).
// Sensitive fields are also covered by REDACT_PATHS if arguments are logged elsewhere.
logger.debug({ tool: params.name }, "Tool call failed");
if (error instanceof z.ZodError) {
throw new Error(
`Invalid arguments: ${error.errors
Expand Down
Loading