[PR #98] [CLOSED] Title: Add embedded credentials validation to URL input validator #117

New issue

Closed

opened 2026-02-27 08:09:16 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 08:09:16 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/KeygraphHQ/shannon/pull/98
Author: @new1earn5
Created: 2/8/2026
Status: ❌ Closed

Base: main ← Head: agent-team/run-1

📝 Commits (1)

ece60f8 Add validation and tests

📊 Changes

6 files changed (+1811 additions, -1308 deletions)

View changed files

📝 package-lock.json (+1596 -58)
📝 package.json (+4 -1)
➕ src/cli/__tests__/input-validator.test.ts (+207 -0)
📝 src/cli/input-validator.ts (+4 -0)
➖ xben-benchmark-results/XBEN-057-24/deliverables/injection_analysis_deliverable.md (+0 -343)
➖ xben-benchmark-results/XBEN-057-24/deliverables/recon_deliverable.md (+0 -906)

📄 Description

Hardens validateWebUrl in src/cli/input-validator.ts to reject target URLs that contain embedded credentials (e.g., http://admin:secret@example.com). For a penetration testing tool, credentials should be supplied through the YAML configuration system's authentication section, never inline in the URL -- where they leak into audit logs, Temporal workflow history, Docker process lists, and shell history.
Also bootstraps the project's first test suite using vitest, with 29 unit tests covering the full input-validator module.
Changes:
src/cli/input-validator.ts -- Added a guard clause (3 lines) that checks parsed.username || parsed.password after the existing protocol and hostname checks, returning { valid: false, error: 'Web URL must not contain embedded credentials' } when userinfo is present.
src/cli/tests/input-validator.test.ts -- New file. 29 tests covering validateWebUrl (10 happy path, 3 protocol rejection, 3 format rejection, 7 credential rejection, 2 false-positive guards, 1 guard-ordering verification) and validateRepoPath (2 valid, 3 invalid).
package.json -- Added vitest as a dev dependency; added test and test:watch scripts.
Testing:
npm test runs all 29 tests via vitest. All pass (29/29, <1s).
Key edge cases exercised:
Percent-encoded credentials (%61dmin decoding to admin) -- confirms the URL API auto-decodes, so the check cannot be bypassed.
@ in path (/users/@admin) and query (?email=user@domain.com) -- confirms no false positives.
Guard clause ordering (ftp://user:pass@host) -- confirms protocol error takes precedence over credentials error.
No existing tests to regress against (this is the first test file in the repository).
Risks / Notes:
Low risk. The production change is purely additive validation -- it rejects a strictly larger set of inputs. All previously valid URLs remain valid.
Pre-existing gap (not introduced here): validateWebUrl and validateRepoPath are defined but not yet wired into the Temporal client entry point (src/temporal/client.ts). Integrating them into the actual CLI invocation path is a recommended follow-up.
Assumption: vitest runs in the project's CI environment. If CI is not yet configured, adding a GitHub Actions workflow for npm test is a natural next step.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/KeygraphHQ/shannon/pull/98 **Author:** [@new1earn5](https://github.com/new1earn5) **Created:** 2/8/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `agent-team/run-1` --- ### 📝 Commits (1) - [`ece60f8`](https://github.com/KeygraphHQ/shannon/commit/ece60f86671c12e8fabece9af93d10f880c67b5a) Add validation and tests ### 📊 Changes **6 files changed** (+1811 additions, -1308 deletions) <details> <summary>View changed files</summary> 📝 `package-lock.json` (+1596 -58) 📝 `package.json` (+4 -1) ➕ `src/cli/__tests__/input-validator.test.ts` (+207 -0) 📝 `src/cli/input-validator.ts` (+4 -0) ➖ `xben-benchmark-results/XBEN-057-24/deliverables/injection_analysis_deliverable.md` (+0 -343) ➖ `xben-benchmark-results/XBEN-057-24/deliverables/recon_deliverable.md` (+0 -906) </details> ### 📄 Description Hardens validateWebUrl in src/cli/input-validator.ts to reject target URLs that contain embedded credentials (e.g., http://admin:secret@example.com). For a penetration testing tool, credentials should be supplied through the YAML configuration system's authentication section, never inline in the URL -- where they leak into audit logs, Temporal workflow history, Docker process lists, and shell history. Also bootstraps the project's first test suite using vitest, with 29 unit tests covering the full input-validator module. Changes: src/cli/input-validator.ts -- Added a guard clause (3 lines) that checks parsed.username || parsed.password after the existing protocol and hostname checks, returning { valid: false, error: 'Web URL must not contain embedded credentials' } when userinfo is present. src/cli/__tests__/input-validator.test.ts -- New file. 29 tests covering validateWebUrl (10 happy path, 3 protocol rejection, 3 format rejection, 7 credential rejection, 2 false-positive guards, 1 guard-ordering verification) and validateRepoPath (2 valid, 3 invalid). package.json -- Added vitest as a dev dependency; added test and test:watch scripts. Testing: npm test runs all 29 tests via vitest. All pass (29/29, <1s). Key edge cases exercised: Percent-encoded credentials (%61dmin decoding to admin) -- confirms the URL API auto-decodes, so the check cannot be bypassed. @ in path (/users/@admin) and query (?email=user@domain.com) -- confirms no false positives. Guard clause ordering (ftp://user:pass@host) -- confirms protocol error takes precedence over credentials error. No existing tests to regress against (this is the first test file in the repository). Risks / Notes: Low risk. The production change is purely additive validation -- it rejects a strictly larger set of inputs. All previously valid URLs remain valid. Pre-existing gap (not introduced here): validateWebUrl and validateRepoPath are defined but not yet wired into the Temporal client entry point (src/temporal/client.ts). Integrating them into the actual CLI invocation path is a recommended follow-up. Assumption: vitest runs in the project's CI environment. If CI is not yet configured, adding a GitHub Actions workflow for npm test is a natural next step. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>