Skip to content

Commit 8506620

Browse files
committed
docs: add security hardening implementation agentlog
- Add comprehensive agentlog documenting all security measures - Minor style fixes: import reordering, test cleanup
1 parent 15f6012 commit 8506620

File tree

4 files changed

+54
-16
lines changed

4 files changed

+54
-16
lines changed
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# Security Hardening Implementation
2+
3+
**Date:** 2025-10-25
4+
**Agent:** TypeScript Expert
5+
**PR:** [Link to be added]
6+
7+
## Summary
8+
9+
Implemented comprehensive security hardening measures for the MCP PTY system to prevent various attack vectors and resource exhaustion scenarios.
10+
11+
## Changes Made
12+
13+
### Environment Variable Sanitization (sec-7)
14+
- Added `sanitizeEnv` function in `packages/pty-manager/src/process.ts`
15+
- Removes dangerous environment variables: `LD_PRELOAD`, `DYLD_INSERT_LIBRARIES`, `PYTHONPATH`, `NODE_PATH`, `GEM_PATH`, `PERL5LIB`, `RUBYLIB`, `CLASSPATH`, `PATH`
16+
- Applied to all PTY spawn operations to prevent library injection attacks
17+
18+
### Sh -c Bypass Prevention (sec-2/3)
19+
- Enhanced `validateCommandAST` in `packages/normalize-commands/src/index.ts`
20+
- Added recursive validation for `sh -c` command arguments
21+
- Parses and validates the inner command string to prevent bypass attacks like `sh -c "rm -rf /"`
22+
23+
### Resource Limits Implementation
24+
- **PTY Count Limit (sec-10):** Added `MAX_PTY_PER_SESSION = 10` in `packages/pty-manager/src/manager.ts`
25+
- Throws error when attempting to create 11th PTY in a session
26+
- **Execution Timeout (sec-11):** Added `execTimeout` option to `PtyOptions`
27+
- Implements activity-based timeout reset to prevent hanging processes
28+
29+
### Testing
30+
- Added PTY count limit test in `packages/pty-manager/src/__tests__/security.test.ts`
31+
- Added sh -c bypass tests in `packages/normalize-commands/src/__tests__/index.test.ts`
32+
- All tests pass with comprehensive coverage
33+
34+
## Impact
35+
36+
- **Security:** Prevents environment variable injection, command bypass, and resource exhaustion attacks
37+
- **Stability:** Limits resource usage per session to prevent DoS scenarios
38+
- **Compatibility:** Maintains backward compatibility while adding security layers
39+
40+
## Next Steps
41+
42+
- Update documentation with security features
43+
- Consider additional hardening measures for post-v1.0
44+
- Monitor for any edge cases in production use

packages/mcp-pty/src/resources/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ import {
77
type SessionManager,
88
} from "@pkgs/session-manager";
99
import {
10-
CONTROL_CODES,
1110
CONTROL_CODE_DESCRIPTIONS,
1211
CONTROL_CODE_EXAMPLES,
12+
CONTROL_CODES,
1313
} from "../types/control-codes.js";
1414
import {
1515
SESSION_ID_SYMBOL,

packages/mcp-pty/src/server/handlers/tools.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
import type { z } from "zod";
22
import { resolveControlCode } from "../../types/control-codes";
33
import { normalizeWorkingDirectory } from "../../utils";
4-
import {
5-
type KillPtyInputSchema,
6-
type ListPtyInputSchema,
7-
type ReadPtyInputSchema,
8-
type StartPtyInputSchema,
4+
import type {
5+
KillPtyInputSchema,
6+
ListPtyInputSchema,
7+
ReadPtyInputSchema,
8+
StartPtyInputSchema,
99
WriteInputSchema,
1010
} from "../schemas";
1111
import type { HandlerContext, ToolResult } from "../types";

packages/pty-manager/src/__tests__/security.test.ts

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,4 @@
1-
import {
2-
afterAll,
3-
afterEach,
4-
beforeAll,
5-
expect,
6-
mock,
7-
spyOn,
8-
test,
9-
} from "bun:test";
1+
import { afterAll, afterEach, beforeAll, expect, spyOn, test } from "bun:test";
102
import { consola } from "consola";
113
import { PtyManager } from "../manager";
124
import { PtyProcess } from "../process";
@@ -175,5 +167,7 @@ test("should enforce PTY count limit per session", async () => {
175167
);
176168

177169
// Cleanup
178-
ptys.forEach((p) => manager.removePty(p.processId));
170+
for (const p of ptys) {
171+
manager.removePty(p.processId);
172+
}
179173
});

0 commit comments

Comments
 (0)