Menu

#14 Fix Windows VCS auth: PowerShell syntax + token expiry tracking

closed
nobody
None
2026-03-30
2026-03-24
Anonymous
No

Originally created by: joiskash

Summary

  • PowerShell here-string fixgitCredentialHelperWrite() in src/os/windows.ts used PowerShell here-strings (@'...'@) joined with semicolons, which is invalid syntax (here-string delimiters must be alone on their own lines). Replaced with array -join pattern using backtick-escaped newlines.
  • Token expiry persistence — GitHub App tokens expire in ~1 hour but expiresAt was only shown in transient output, never stored. Added vcsProvider and vcsTokenExpiresAt fields to the Agent type, persisted to registry after deployment, and added expiry warnings when tokens are expired or within 10 minutes of expiry.

Changes

  • src/os/windows.ts — Fixed gitCredentialHelperWrite() to generate valid PowerShell
  • src/types.ts — Added vcsProvider? and vcsTokenExpiresAt? to Agent interface
  • src/tools/provision-vcs-auth.ts — Persist VCS metadata to registry after deploy
  • src/utils/agent-helpers.ts — New checkVcsTokenExpiry() helper with configurable now for testing
  • tests/windows-credential-helper.test.ts — 7 tests for PowerShell fix (special chars, no here-strings)
  • tests/vcs-token-expiry.test.ts — 7 tests for expiry warning logic and registry persistence

Test plan

  • [x] All 431 tests pass (npm test)
  • [x] Build clean (npm run build)
  • [x] Reviewer approved Phase 1 (PowerShell fix) and Phase 2 (token expiry)
  • [x] Smoke test: reviewer (Windows member) validated generated PowerShell syntax is correct
  • [ ] Live smoke test: provision VCS auth on Windows member after merge to confirm end-to-end

Related

Tickets: #75

Discussion

  • Anonymous

    Anonymous - 2026-03-30

    Originally posted by: kumaakh

    Code Review — fleet-rev

    Verdict: APPROVED with non-blocking issues below. Requesting fixes before merge.

    Actionable fixes (fleet-dev)

    1. MEDIUM: Batch metacharacter escaping in Windows .bat credential helper (src/os/windows.ts:144)
    2. Token is escaped for PowerShell (''') but written into a .bat file executed by cmd.exe. Batch metacharacters (&, |, >, <, ^, %) in the token would break the script.
    3. Fix: Add batch-level ^ escaping for the token value before writing it into the .bat content.

    4. LOW: Escaping inconsistency between write and remove (src/os/windows.ts:140 vs :153)

    5. gitCredentialHelperWrite passes host through escapeWindowsArg(), but gitCredentialHelperRemove only does PowerShell single-quote escaping. Asymmetry is a code smell.
    6. Fix: Use escapeWindowsArg() consistently in both write and remove.

    7. LOW: Redundant ?? undefined (src/tools/provision-vcs-auth.ts:97)

    8. deployResult.metadata?.expiresAt ?? undefined — the ?? undefined is redundant.
    9. Fix: Remove ?? undefined.

    Not fixing in this PR

    • Docs deletion in unrelated commit — low impact, leave as-is
    • Shared credential file across VCS providers — pre-existing architectural limitation

    Full review details in feedback.md on the branch.

     
  • Anonymous

    Anonymous - 2026-03-30

    Ticket changed by: kumaakh

    • status: open --> closed
     

Log in to post a comment.

MongoDB Logo MongoDB