fix(demo): validate PREWARM_REPOS before reflecting it on the demo endpoint (Closes #180)

[SakethSumanBathini]

Closes #180

Problem

GET /demo is registered with no middleware (workspace.routes.ts:34), and getDemoWorkspace echoed operator-supplied configuration straight back to anonymous callers:

const firstRepo = (process.env.PREWARM_REPOS || 'QBobWatson/ila').split(',')[0].trim();
const [owner, repo] = firstRepo.split('/');
const cached = buildExecutor.getBuildCacheEntry(`${owner}/${repo}`);
...
res.json({ sessionId: cached.sessionId, owner, repo });

Three ways that misbehaves:

• owner/repo?token=xyz puts the query string into repo, which is then returned to any unauthenticated caller — the case this issue reports.
• An entry with no slash leaves repo as undefined and builds the cache key owner/undefined, which silently never matches.
• Extra path segments are dropped by the two-element destructure without complaint.

Change

Both segments are validated against ^[A-Za-z0-9_.-]+$ — the character set GitHub permits in owner and repository names — and the entry must split into exactly two parts. Anything else returns the same 503 the endpoint already uses when no build is cached, so an unauthenticated caller learns nothing about the configuration either way.

Verified against the cases that matter:

ACCEPT  "QBobWatson/ila"
ACCEPT  "my-org/my.repo_1"
REFUSE  "owner/repo?token=xyz"
REFUSE  "ownerrepo"
REFUSE  "a/b/c"
REFUSE  "owner/"
REFUSE  "/repo"
REFUSE  "owner/re po"

The rejected value is deliberately not logged. If it contains a credential — the scenario this issue is about — writing it to the log moves the exposure rather than removing it. The log records that the value was invalid, not what it was.

Scope note

This validates what leaves the endpoint. It doesn't stop an operator putting a credential in PREWARM_REPOS in the first place — that value is also used to drive prewarm builds elsewhere, and a malformed entry there deserves its own treatment. Happy to look at that separately if useful.

Verification

• npx tsc --noEmit in backend/ clean.
• Pattern exercised against the eight cases above.
• Diff is 28 insertions, 1 deletion, confined to getDemoWorkspace.

Ordering

Touches workspace.controller.ts. My open #105 branch modifies createTeamSession in the same file, well away from this function — should merge, but worth landing one and re-checking.

Summary by CodeRabbit

• Bug Fixes
  • Improved validation for demo repository configuration.
  • Invalid repository values now display a clear demo-unavailable message instead of causing malformed requests.
  • Demo environments remain marked as building while the required repository configuration is corrected.

[coderabbitai]

📝 Walkthrough

Walkthrough

The demo workspace controller now validates the first configured prewarm repository entry before destructuring its owner and repository segments. Invalid entries return a 503 response with building: true instead of proceeding to cache lookup.

Changes

Demo workspace validation

Layer / File(s)	Summary
Repository input validation backend/src/controllers/workspace.controller.ts	Adds repository segment character validation and requires exactly two segments before constructing the demo workspace cache key; invalid input returns 503 with building: true.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: harsharajkumar-273

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately reflects the demo PREWARM_REPOS validation change.
Linked Issues check	✅ Passed	The change enforces strict owner/repo validation before getDemoWorkspace reflects demo session details, matching issue #180.
Out of Scope Changes check	✅ Passed	The diff appears focused on the requested demo endpoint validation with no unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

backend/src/controllers/workspace.controller.ts (1)

52-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add regression coverage for malformed repository entries.

Test missing/extra slashes, query strings, forbidden characters, and embedded line terminators. Assert the exact 503 response and that the rejected value is absent from logs.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/src/controllers/workspace.controller.ts` around lines 52 - 60, Add
regression tests for the repository validation branch in the controller handling
PREWARM_REPOS, covering missing or extra slashes, query strings, forbidden
characters, and embedded line terminators. Assert each malformed value returns
the exact 503 JSON response with building set to true, and verify the rejected
value is not written to logs.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@backend/src/controllers/workspace.controller.ts`:
- Around line 52-60: Add regression tests for the repository validation branch
in the controller handling PREWARM_REPOS, covering missing or extra slashes,
query strings, forbidden characters, and embedded line terminators. Assert each
malformed value returns the exact 503 JSON response with building set to true,
and verify the rejected value is not written to logs.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 21f2a79f-f373-48b1-b542-4dec8bbafd72

📥 Commits

Reviewing files that changed from the base of the PR and between 4c403cd and 5437582.

📒 Files selected for processing (1)

• backend/src/controllers/workspace.controller.ts

📜 Review details

⏰ Context from checks skipped due to timeout. (1)

• GitHub Check: test

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2026-07-24T17:54:49.155Z

Learnt from: SakethSumanBathini
Repo: harsharajkumar-273/Proofdesk PR: 112
File: backend/src/controllers/import.controller.ts:0-0
Timestamp: 2026-07-24T17:54:49.155Z
Learning: For Express controllers that run long-running work (e.g., PDF imports), handle client disconnects using `res.on('close')` and treat the connection as aborted only when `res.writableFinished === false`. Avoid using `req.on('close')` for this purpose, since it can trigger during normal request-body consumption even when the response has not completed.

Applied to files:

• backend/src/controllers/workspace.controller.ts

🔇 Additional comments (1)

backend/src/controllers/workspace.controller.ts (1)

34-34: 🔒 Security & Privacy

No end-anchor bypass here. The current regex already rejects segments with trailing line terminators, so owner\n/repo does not pass.

			> Likely an incorrect or invalid review comment.