[mlzoo]

What

load_session / save_session in src/session_store.py built the session file path directly from the caller-supplied session_id with no validation.

Impact

A session_id such as ../../../../tmp/secrets — reachable from the load-session CLI subcommand (src/main.py:168) — let a caller read JSON files outside the .port_sessions store, and save_session could likewise write outside it. (Path traversal is listed as in-scope in SECURITY.md.)

Fix

Add _validate_session_id() which rejects anything outside [A-Za-z0-9_-] (path separators, .., absolute and Windows paths) before the id is used to build a filename. Both save_session and load_session validate first. Legitimate ids (uuid4 hex, session-<digits>-<n>) are unaffected.

Tests

Adds SessionStorePathTraversalTests to tests/test_security_scope.py:

• traversal id rejected on load
• absolute/separator/.. ids rejected
• traversal id rejected on save
• legitimate id round-trips (no regression)

Full suite: python3 -m unittest discover -s tests -> 50 passed.

[XinwangTao]

Nice catch on the path traversal. The whitelist approach ([A-Za-z0-9_-]) is definitely the right call over trying to sanitize dots and slashes — much simpler and less error-prone.

One minor consideration: the current allowlist excludes common session-id characters like hyphens in non-leading position. Looking at the test description it seems hyphens are already covered (_ is included), but just confirming — are there any existing session IDs in the wild that use additional safe characters like dots in a flat namespace (e.g., user.session.1)? If not, this is clean.

Security impact is clear and the test coverage looks solid. Good fix.

[mlzoo]

Thanks! To confirm — no existing session IDs use dots. The two formats in the codebase are uuid4 hex strings (e.g. a1b2c3d4e5f6...) and session-<digits>-<n>, both of which are fully covered by [A-Za-z0-9_-]. So no legitimate IDs are rejected by this allowlist.

[XinwangTao]

Confirmed — the whitelist approach is the right call for path traversal prevention. Since all existing session ID formats (uuid4 hex and session--) are covered by [A-Za-z0-9_-], there should be no regression risk. The test coverage for traversal rejection plus legitimate round-trip is thorough. LGTM on the fix.

[mlzoo]

Thanks for confirming — keeping the whitelist approach as-is.

[XinwangTao]

The fix looks complete — validation covers all traversal vectors (.., absolute paths, separators) and the legit session ID formats are preserved. The test coverage for both rejection and round-trip cases is thorough. Ready to merge from my side.

[Yeachan-Heo]

Reviewed exact head 102bc5e. Validation runs before either filename is formed; tests cover traversal, POSIX and Windows paths, rejected writes, and valid-ID round trips. No findings.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[Yeachan-Heo]

Reviewed exact head 102bc5e. Validation runs before either filename is formed; tests cover traversal, POSIX and Windows paths, rejected writes, and valid-ID round trips. No findings.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]

[XinwangTao]

Important security fix — the strict whitelist ([A-Za-z0-9_-]) cleanly closes both the read and write traversal paths, and the round-trip test guards against regression. Good catch, thanks.