fix(security): validate domain parameter in comet_tabs (#26)

[RapierCraft]

Summary

Adds validateDomain() to the shared validator module and calls it in both src/index.ts and src/http-bridge.ts before the domain parameter reaches findTabByDomain() in the switch/close actions of comet_tabs.

This addresses the three issues identified in the PR #14 security review:

1. No length limit on the domain string
2. Attacker-controlled input echoed in "No tab found for domain: ${domain}" responses
3. Same gap in the HTTP bridge (handleTabs())

Changes

• src/upload-validator.ts: Added validateDomain() export — max 253 chars (DNS hostname limit), hostname-safe charset [A-Za-z0-9.\-], static error messages that never echo back the input
• src/index.ts: Import validateDomain; validate before findTabByDomain() in switch and close branches; change "not found" errors to static strings
• src/http-bridge.ts: Same import and validation in both domain branches of handleTabs()

Testing

• comet_tabs switch with valid domain → proceeds normally [type:manual]
• comet_tabs switch with >253 char domain → static error, no echo [type:manual]
• comet_tabs switch with <script> or control chars → static error, no echo [type:manual]
• comet_tabs close with invalid domain → same static error [type:manual]
• HTTP bridge switch/close with invalid domain → static error, no echo [type:manual]

---

Closes #26
Implementation branch: fix/validate-domain-comet-tabs-26
Base: main

[RapierCraft]

Security Review — PR #31

Reviewed commit: `64cceb7` | Agent: General Security | Domain: Input Validation / XSS Prevention

Scope

3 files changed: `src/upload-validator.ts`, `src/index.ts`, `src/http-bridge.ts`

Findings

SEC-1 (Informational): Prettier formatting was not applied to the original commits. The orchestrator applied `npx prettier --write` and committed a formatting-only follow-up commit (`64cceb7`). This is already resolved before review.

All other checks: CLEAN

Validation Evidence

1. validateDomain() logic verified:

   • Length check: `domain.length === 0 || domain.length > 253` → static error (no echo)
   • Charset check: `/^[A-Za-z0-9.\-]+$/` → static error (no echo)
   • Tested against: `<script>`, null bytes, newline injection, `../etc/passwd`, `domain:8080`, `user@domain.com` — all correctly rejected
   • Valid inputs (`github.com`, `sub.example.org`, `my-site.co.uk`, `localhost`) — all correctly pass

2. No domain echo in error responses: All 4 previously vulnerable ``No tab found for domain: ${domain}`` messages changed to static `"No tab found for the specified domain"`

3. tab.domain in success responses is safe: Success path echoes `tab.domain` which comes from `extractDomain(target.url)` — an internal CDP-sourced value, not the user-supplied parameter

4. Both code paths covered: switch and close cases in both `src/index.ts` and `src/http-bridge.ts` validated

5. Build: `tsc` clean, no errors

6. Pre-existing test failures: 2 screenshot tests fail on both main and this branch — confirmed pre-existing, unrelated to this PR. 53/55 tests pass.

7. Secrets: No hardcoded secrets or credentials introduced

8. Env vars: No new environment variables introduced

Verdict

APPROVED — No security findings. The implementation correctly addresses all three issues from the PR #14 security review.

---

[RapierCraft]

APPROVED: commit 64cceb7 after context-aware security review (1 agent: General Security). 0 findings. Safe to merge.

[RapierCraft]

PR Review Summary: #31 — fix(security): validate domain parameter in comet_tabs

Review Integrity

Reviewed commit: `64cceb7` | Current HEAD: `64cceb7` | Status: CURRENT

Verdict: APPROVED ✓

Context-Aware Review

Domains: SECURITY, AUTH_SENSITIVE | Agents: 1 (General Security)

Integration Checks (Phase 2.5)

Code registration: PASS — validateDomain exported from upload-validator.ts, imported in both index.ts and http-bridge.ts, build clean
SOPS deploy chain: N/A — no new env vars introduced
Purpose Regression Gate (7A): N/A — not a milestone PR

Risk Matrix

Category	Risk	Blocking?	Confidence
Input validation completeness	LOW	No	CONFIRMED CLEAN
XSS echo vector	LOW	No	CONFIRMED FIXED
Static error messages	LOW	No	CONFIRMED FIXED

Findings

None — review clean.

Automated Checks

Check	Result
TypeScript build (tsc)	✓ PASS
Prettier formatting	✓ PASS (applied in commit 64cceb7)
Unit tests	✓ 53/55 pass (2 pre-existing failures unrelated to PR)
Secrets scan	✓ PASS
Env var audit	✓ PASS (no new vars)

Recommendation

Merged to main. No follow-up issues required.

---

Context-aware review complete. 1 agent + integration checks. 0 findings. PR merged.