Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,8 @@ Error messages should never include raw values from sensitive sources like envir
**Prevention:**
1. Avoid including raw values in error messages when the source is potentially sensitive (env vars, auth headers).
2. Use generic error messages for validation failures of sensitive data.

## 2024-05-08 - [Path Traversal / URL Injection in encodePathSegment]
**Vulnerability:** In `mcp-server.ts`, dot characters (`.`) injected into the URL paths were not encoded by `encodeURIComponent`.
**Learning:** `encodeURIComponent` ignores `.`. In addition, using a ternary check like `val.includes('/') ? encodeURIComponent(val) : val;` completely bypassed the encoding check if the malicious string was exactly `..` (because it does not contain a slash).
**Prevention:** Always encode all URL path values without conditional string presence checks (e.g. unconditionally calling `encodeURIComponent(val).replace(/\./g, '%2E')` handles both directory traversal payloads and standard pathing safely).
2 changes: 1 addition & 1 deletion src/mcp/mcp-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1205,7 +1205,7 @@ export class MCPServer {
*/
private encodePathSegment(value: unknown): string {
const val = String(value);
return val.includes('/') ? encodeURIComponent(val) : val;
return encodeURIComponent(val).replace(/\./g, '%2E');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve already encoded path identifiers

This change unconditionally re-encodes every path parameter, so inputs that are already URL-encoded (a documented supported format like group%2Fproject) become double-encoded (group%252Fproject). For GitLab-style endpoints that expect the path form to be encoded once, this turns a valid project identifier into a different literal value and can cause 404/lookup failures for many operations using namespaced IDs; the same regression pattern is also present in composite path resolution.

Useful? React with πŸ‘Β / πŸ‘Ž.

}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/tooling/composite-executor-security.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('CompositeExecutor Security', () => {
// Fixed behavior: path contains encoded "%2E%2E"
// Note: encodeURIComponent('../admin/secrets') => ..%2Fadmin%2Fsecrets
// The slashes inside the injected value are encoded, preventing directory traversal
const expectedFixedPath = '/users/..%2Fadmin%2Fsecrets/profile';
const expectedFixedPath = '/users/%2E%2E%2Fadmin%2Fsecrets/profile';

expect(capturedPaths[0]).toBe(expectedFixedPath);
});
Expand Down
4 changes: 2 additions & 2 deletions src/tooling/composite-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,14 +171,14 @@ export class CompositeExecutor {
return template.replace(/\{(\w+)\}/g, (_, key) => {
// Try direct match first
if (args[key] !== undefined) {
return encodeURIComponent(String(args[key]));
return encodeURIComponent(String(args[key])).replace(/\./g, '%2E');
}

// Try aliases from profile
const possibleAliases = this.parameterAliases[key] || [];
for (const alias of possibleAliases) {
if (args[alias] !== undefined) {
return encodeURIComponent(String(args[alias]));
return encodeURIComponent(String(args[alias])).replace(/\./g, '%2E');
}
}

Expand Down
Loading