Feature/auth and quality hub api by mrdailey99 · Pull Request #116 · ProvarTesting/provardx-cli

mrdailey99 · 2026-04-10T22:39:31Z

No description provided.

Adds src/services/auth/credentials.ts as the single source of truth for Provar API key storage (~/.provar/credentials.json) and resolution. Priority: PROVAR_API_KEY env var > stored file > null. Empty/whitespace env var treated as unset. Phase 2 fields (username, tier, expires_at) defined as optional from the start to avoid schema migration later. Full unit test coverage in test/unit/services/auth/credentials.test.ts. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Adds three new commands under sf provar auth: - set-key --key <pv_k_...>: stores key to ~/.provar/credentials.json - status: reports key source (env var / file / not configured) - clear: removes stored credentials with local-fallback warning Registers auth subtopic in package.json oclif config and creates accompanying messages/*.md files for all three commands. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

… key-based routing Phase 1 sections 1.3 + 1.4 (and lint fixes for previously staged Phase 1 files): - src/services/qualityHub/client.ts: stub validateTestCaseViaApi, normaliseApiResponse mapping AWS API response to internal format (per AWS memo 2026-04-10): valid->is_valid, errors[]/warnings[]->issues[], quality_metrics.quality_score->quality_score. Added getInfraKey() for PROVAR_INFRA_KEY env var (separate from user pv_k_ key). - src/mcp/tools/testCaseValidate.ts: async handler, resolveApiKey routing, quality_hub/local/local_fallback validation_source, onboarding/fallback warnings - test/unit/services/qualityHub/client.test.ts: 11 tests for normaliseApiResponse, 2 for getInfraKey - test/unit/mcp/testCaseValidate.test.ts: 6 handler-level tests (no-key, success, 3 fallback paths) - docs/auth-cli-plan.md: updated header contract (x-provar-key + x-api-key) and response shape table - Lint fixes across all Phase 1 src/commands and test/unit/commands/provar/auth/* files Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Adds Phase 1 support for Provar API-key based Quality Hub validation: credential storage + CLI auth commands, a stubbed Quality Hub API client with response normalisation, and MCP tool integration that prefers the API when a key is present and otherwise falls back to local validation with user guidance.

Changes:

Introduces credential persistence/resolution (~/.provar/credentials.json) and new sf provar auth commands (set-key, status, clear).
Adds a Quality Hub client layer (stubbed HTTP call) plus normaliseApiResponse() and typed API errors for fallback logic.
Updates provar.testcase.validate to use Quality Hub when configured, emitting validation_source + optional validation_warning, and expands unit tests accordingly.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
`src/services/auth/credentials.ts`	Adds storage, clearing, and resolution of the Provar API key.
`src/commands/provar/auth/set-key.ts`	Adds CLI command to store the API key on disk.
`src/commands/provar/auth/status.ts`	Adds CLI command to report key configuration source and mode.
`src/commands/provar/auth/clear.ts`	Adds CLI command to remove stored credentials.
`src/services/qualityHub/client.ts`	Introduces Quality Hub client stub, response normalisation, and typed errors.
`src/mcp/tools/testCaseValidate.ts`	Makes handler async; integrates API-first validation with local fallback + warnings.
`package.json`	Registers `provar auth` as an OCLIF subtopic.
`messages/sf.provar.auth.set-key.md`	Help text for `sf provar auth set-key`.
`messages/sf.provar.auth.status.md`	Help text for `sf provar auth status`.
`messages/sf.provar.auth.clear.md`	Help text for `sf provar auth clear`.
`test/unit/services/auth/credentials.test.ts`	Unit tests for credentials storage/clearing/resolution.
`test/unit/commands/provar/auth/set-key.test.ts`	Tests set-key behavior via credentials layer.
`test/unit/commands/provar/auth/status.test.ts`	Tests status/source detection logic via credentials layer.
`test/unit/commands/provar/auth/clear.test.ts`	Tests clear behavior via credentials layer.
`test/unit/services/qualityHub/client.test.ts`	Tests Quality Hub API response normalisation + infra key getter.
`test/unit/mcp/testCaseValidate.test.ts`	Adds handler-level tests for API success and fallback scenarios.
`docs/auth-cli-plan.md`	Adds/updates implementation plan for auth + Quality Hub integration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-10T22:44:11Z

src/mcp/tools/testCaseValidate.ts

+            const result = {
+              requestId,
+              ...apiResult,
+              step_count: 0, // API result — step count not returned by API
+              error_count: apiResult.issues.filter((i) => i.severity === 'ERROR').length,
+              warning_count: apiResult.issues.filter((i) => i.severity === 'WARNING').length,
+              test_case_id: null as string | null,
+              test_case_name: null as string | null,


In the Quality Hub success path, the returned result hard-codes step_count to 0 and sets test_case_id/test_case_name to null. This makes API-mode responses inconsistent with local validation and can regress consumers that rely on these fields. Consider parsing the XML locally (e.g., a lightweight extraction or reusing validateTestCase() just to derive id/name/step_count) and include those values alongside the API scores/issues.

Suggested change

const result = {

requestId,

...apiResult,

step_count: 0, // API result — step count not returned by API

error_count: apiResult.issues.filter((i) => i.severity === 'ERROR').length,

warning_count: apiResult.issues.filter((i) => i.severity === 'WARNING').length,

test_case_id: null as string | null,

test_case_name: null as string | null,

const localMetadata = validateTestCase(source);

const result = {

requestId,

...apiResult,

step_count: localMetadata.step_count,

error_count: apiResult.issues.filter((i) => i.severity === 'ERROR').length,

warning_count: apiResult.issues.filter((i) => i.severity === 'WARNING').length,

test_case_id: localMetadata.test_case_id,

test_case_name: localMetadata.test_case_name,

Copilot · 2026-04-10T22:44:11Z

test/unit/services/auth/credentials.test.ts

+let origHome: string;
+let tempDir: string;
+
+function useTemp(): void {
+  tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'provar-cred-test-'));
+  origHome = os.homedir();
+  // Monkey-patch homedir for the duration of the test block
+  (os as unknown as { homedir: () => string }).homedir = (): string => tempDir;
+}
+
+function restoreHome(): void {
+  (os as unknown as { homedir: () => string }).homedir = (): string => origHome;


These tests monkey-patch os.homedir(), but restoreHome() only restores the returned string (origHome) rather than restoring the original homedir function implementation. After this suite, os.homedir remains a stub returning a constant, which can leak into other tests. Save the original function reference (e.g., const originalHomedir = os.homedir) and restore it in afterEach.

Suggested change

let origHome: string;

let tempDir: string;

function useTemp(): void {

tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'provar-cred-test-'));

origHome = os.homedir();

// Monkey-patch homedir for the duration of the test block

(os as unknown as { homedir: () => string }).homedir = (): string => tempDir;

}

function restoreHome(): void {

(os as unknown as { homedir: () => string }).homedir = (): string => origHome;

const originalHomedir = os.homedir;

let tempDir: string;

function useTemp(): void {

tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'provar-cred-test-'));

// Monkey-patch homedir for the duration of the test block

(os as unknown as { homedir: () => string }).homedir = (): string => tempDir;

}

function restoreHome(): void {

(os as unknown as { homedir: () => string }).homedir = originalHomedir;

Copilot · 2026-04-10T22:44:11Z

test/unit/commands/provar/auth/status.test.ts

+let tempDir: string;
+let savedEnv: string | undefined;
+
+function useTemp(): void {
+  tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'provar-status-test-'));
+  origHome = os.homedir();
+  (os as unknown as { homedir: () => string }).homedir = (): string => tempDir;
+}
+
+function restoreHome(): void {
+  (os as unknown as { homedir: () => string }).homedir = (): string => origHome;


These tests monkey-patch os.homedir(), but restoreHome() restores by assigning a new stub that returns origHome rather than restoring the original os.homedir function. This can leak into other test files. Save the original function reference and restore it in afterEach.

Suggested change

let tempDir: string;

let savedEnv: string | undefined;

function useTemp(): void {

tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'provar-status-test-'));

origHome = os.homedir();

(os as unknown as { homedir: () => string }).homedir = (): string => tempDir;

}

function restoreHome(): void {

(os as unknown as { homedir: () => string }).homedir = (): string => origHome;

let origHomedir: typeof os.homedir;

let tempDir: string;

let savedEnv: string | undefined;

function useTemp(): void {

tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'provar-status-test-'));

origHomedir = os.homedir;

origHome = os.homedir();

(os as unknown as { homedir: () => string }).homedir = (): string => tempDir;

}

function restoreHome(): void {

(os as unknown as { homedir: typeof os.homedir }).homedir = origHomedir;

Copilot · 2026-04-10T22:44:12Z

test/unit/commands/provar/auth/set-key.test.ts

+let origHome: string;
+let tempDir: string;
+
+function useTemp(): void {
+  tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'provar-setkey-test-'));
+  origHome = os.homedir();
+  (os as unknown as { homedir: () => string }).homedir = (): string => tempDir;
+}
+
+function restoreHome(): void {
+  (os as unknown as { homedir: () => string }).homedir = (): string => origHome;


These tests monkey-patch os.homedir(), but restoreHome() restores by assigning a new stub that returns origHome rather than restoring the original os.homedir function implementation. Save the original os.homedir function reference and restore it after each test to avoid cross-suite leakage.

Suggested change

let origHome: string;

let tempDir: string;

function useTemp(): void {

tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'provar-setkey-test-'));

origHome = os.homedir();

(os as unknown as { homedir: () => string }).homedir = (): string => tempDir;

}

function restoreHome(): void {

(os as unknown as { homedir: () => string }).homedir = (): string => origHome;

let origHomedir: typeof os.homedir;

let tempDir: string;

function useTemp(): void {

tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'provar-setkey-test-'));

origHomedir = os.homedir;

(os as unknown as { homedir: () => string }).homedir = (): string => tempDir;

}

function restoreHome(): void {

(os as unknown as { homedir: typeof os.homedir }).homedir = origHomedir;

Copilot · 2026-04-10T22:44:12Z

test/unit/commands/provar/auth/clear.test.ts

+function useTemp(): void {
+  tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'provar-clear-test-'));
+  origHome = os.homedir();
+  (os as unknown as { homedir: () => string }).homedir = (): string => tempDir;
+}
+
+function restoreHome(): void {
+  (os as unknown as { homedir: () => string }).homedir = (): string => origHome;
+  fs.rmSync(tempDir, { recursive: true, force: true });
+}


These tests monkey-patch os.homedir(), but restoreHome() restores by assigning a new stub that returns origHome rather than restoring the original os.homedir function. Save the original function reference (os.homedir) and restore it in afterEach to prevent leaking the stub into other tests.

Copilot · 2026-04-10T22:44:12Z

test/unit/mcp/testCaseValidate.test.ts

+  beforeEach(() => {
+    // Redirect home so readStoredCredentials() finds no file
+    tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'provar-handler-test-'));
+    savedHome = os.homedir();
+    (os as unknown as { homedir: () => string }).homedir = (): string => tempDir;
+
+    savedApiKey = process.env.PROVAR_API_KEY;
+    delete process.env.PROVAR_API_KEY;
+
+    capServer = new CapturingServer();
+    registerTestCaseValidate(capServer as unknown as McpServer, { allowedPaths: [] });
+  });
+
+  afterEach(() => {
+    (os as unknown as { homedir: () => string }).homedir = (): string => savedHome;
+    fs.rmSync(tempDir, { recursive: true, force: true });
+    if (savedApiKey !== undefined) {


This suite monkey-patches os.homedir() in beforeEach/afterEach, but the afterEach restores by replacing homedir with a new stub returning savedHome instead of restoring the original function implementation. That can leak into other tests. Save the original os.homedir function reference and restore it in afterEach.

Copilot · 2026-04-10T22:44:12Z

src/commands/provar/auth/set-key.ts

+    const { flags } = await this.parse(SfProvarAuthSetKey);
+    const key = flags.key;
+
+    if (!key.startsWith('pv_k_')) {
+      this.error(
+        'Invalid API key format. Keys must start with "pv_k_". Get your key from https://success.provartesting.com.',
+        { exit: 1 }
+      );
+    }
+
+    const prefix = key.substring(0, 12);
+    writeCredentials(key, prefix, 'manual');


Consider trimming the provided --key value before validating/storing it (e.g., key.trim()). As-is, accidental whitespace will cause startsWith('pv_k_') checks to fail or can be persisted to disk, leading to confusing auth behavior.

Copilot · 2026-04-10T22:44:13Z

src/services/auth/credentials.ts

+export function resolveApiKey(): string | null {
+  const envKey = process.env.PROVAR_API_KEY?.trim();
+  if (envKey) return envKey;
+  const stored = readStoredCredentials();
+  return stored?.api_key ?? null;


resolveApiKey() trims PROVAR_API_KEY but does not validate the expected pv_k_ prefix. That means an invalid env var value will be treated as configured and used for API calls. To keep behavior consistent with writeCredentials()/set-key validation, consider ignoring (or explicitly rejecting) env values that don't start with pv_k_.

Copilot · 2026-04-10T22:44:13Z

src/commands/provar/auth/status.ts

+    const envKey = process.env.PROVAR_API_KEY?.trim();
+
+    if (envKey) {
+      this.log('API key configured');
+      this.log('  Source:   environment variable (PROVAR_API_KEY)');
+      this.log(`  Prefix:   ${envKey.substring(0, 12)}`);
+      this.log('');
+      this.log('  Validation mode: Quality Hub API');
+      return;


The status command treats any non-empty PROVAR_API_KEY as a configured Provar key. If the env var is set to an unexpected value (wrong prefix/typo), status will still report "Validation mode: Quality Hub API". Consider validating the pv_k_ prefix here (or delegating to a shared validator) and reporting an invalid configuration instead.

- Restore original os.homedir function reference in all 5 test files instead of replacing it with a new closure (prevents cross-suite stub leakage) - Add resolveApiKey() test for invalid pv_k_ prefix filtering - Add resolveApiKey() test for ignored env var without pv_k_ prefix in status tests - credentials.ts: ignore PROVAR_API_KEY env vars that lack pv_k_ prefix - set-key.ts: trim whitespace from --key flag before validation/storage - status.ts: detect and report invalid env key prefix as misconfiguration - testCaseValidate.ts: extract local metadata (id, name, step_count) from XML and merge into Quality Hub API response so consumers get consistent fields - Update Quality Hub handler test to assert merged metadata fields - Add NUT tests for sf provar auth set-key, status, and clear commands - Extend test:nuts glob patterns to discover new auth NUT files Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

substring(0,12) on 'pv_k_nuttest...' yields 'pv_k_nuttest' (12 chars), not 'pv_k_nuttest12' (14). Same off-by-two for status test key prefix. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- status.nut.ts: assert on 'Prefix:' label rather than a hardcoded prefix string (avoids off-by-one errors in substring arithmetic) - CI_Execution.yml: add 'if: always()' to the artifact upload step so mochawesome report is published even when NUT tests fail Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Auth flow confirmed as PKCE / Hosted UI (Option B) - Token strategy: exchange immediately, store only pv_k_, discard Cognito tokens - Document three registered callback ports (1717, 7890, 8080) and port-selection logic - Add full PKCE implementation sketch (code verifier, challenge, localhost listener) - Note Cognito endpoint config env vars (PROVAR_COGNITO_DOMAIN, PROVAR_COGNITO_CLIENT_ID) - Phase 1 CLI infrastructure unchanged — credentials.ts/set-key/resolveApiKey unaffected - Update Phase 2 Done criteria to include token-not-on-disk assertion Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

- Document three confirmed endpoints on shared base URL: POST /auth/exchange, GET /auth/status, POST /auth/revoke - Add client.ts stubs for exchangeTokenForKey, fetchKeyStatus, revokeKey - Plan sf provar auth status live check via /auth/status (graceful offline fallback) - Plan sf provar auth clear revoke via /auth/revoke (best-effort, deletes locally regardless) - Renumber Phase 2 sections to accommodate new 2.2/2.3/2.4 client/status/clear updates Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ase 2) - Add loginFlow service: PKCE pair generation, port selection from registered callbacks (1717/7890/8080), browser open, localhost callback server, and HTTPS code-for-tokens exchange - Add login command: full OAuth 2.0 Authorization Code + PKCE flow against Cognito Hosted UI, with Quality Hub token exchange at the end - Extend qualityHubClient: exchangeTokenForKey, fetchKeyStatus, revokeKey using node:https (no DOM fetch dependency) - Update status command: prefix validation for env var keys, live key check via fetchKeyStatus with silent offline fallback - Update clear command: best-effort server-side revoke before clearing local credentials - Add unit tests for loginFlow (generatePkce, listenForCallback, credential writing, exchangeTokenForKey stubs) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…p beta.4 POST /auth/exchange expects { "access_token": "..." } in the JSON body with x-api-key infra header — not an Authorization: Bearer header. Corrected based on AWS team handoff (2026-04-11). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…p beta.4 /auth/exchange, /auth/status, /auth/revoke no longer require the API Gateway infra key — Cognito JWT and pv_k_ keys are sufficient. POST /auth/exchange now sends the Cognito access token as { "access_token": "..." } in the request body. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…all docs - README: add sf provar auth login/set-key/status/clear command entries; update MCP section to describe local vs Quality Hub validation modes - docs/mcp.md: add Authentication section (validation modes, key setup, env vars, CI/CD); add validation_source/validation_warning to testcase.validate output table - docs/mcp-pilot-guide.md: add Scenario 8 (Quality Hub API validation); update Scenario 2 tip; expand Credential handling to cover pv_k_ key - docs/provar-mcp-public-docs.md: add Step 3 auth setup; update validate-a-test-case section with validation_source and mode note - docs/university-of-provar-mcp-course.md: add Lab 2.5 (auth login); expand Module 4 with validation modes table; add knowledge check Q4 - docs/auth-cli-plan.md: removed (internal planning doc) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

us-east-1qpfw was a misread — correct domain is us-east-1xpfwzwmop. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Cognito requires a nonce when using the openid scope (OIDC spec replay prevention). Also drops the profile scope which was not configured in the App Client, and corrects the scope to openid email only. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…aged Login Cognito Managed Login requires state (CSRF protection) and behaves more reliably at /login than /oauth2/authorize. Both state and nonce are now generated per-request. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

/login is just the UI page. /oauth2/authorize with state + nonce + PKCE is the correct OAuth endpoint and confirmed working in browser testing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

cmd.exe interprets '&' in URLs as a command separator, so only the first query parameter was reaching the browser. PowerShell Start-Process passes the full URL as a single uninterpreted argument. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…saging Cognito's GetUser API requires the access token to carry the aws.cognito.signin.user.admin scope — without it the Lambda receives a valid JWT but GetUser returns NotAuthorizedException. Added to the scope parameter in the authorize URL. Also updated auth clear output to suggest sf provar auth login as the primary reconfiguration path and mention PROVAR_API_KEY for CI/CD. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

The docs said "set PROVAR_API_KEY for CI/CD" but never explained how to get the value or that the key expires. Added the full workflow: run sf provar auth login locally, copy api_key from credentials.json, store as pipeline secret, rotate every ~90 days. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Replaces the stub with a real POST /validate call using x-provar-key for user auth. /validate has no infra-gate x-api-key requirement. Removed getInfraKey() — dead code in the CLI. The batch validator that requires the infra key is in the managed package, not here. 401 always uses our own message (never the API's) to avoid surfacing sf provar auth set-key which does not exist. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

No AWS route backs this command and keys can only be obtained via sf provar auth login. set-key was clutter in --help. Removed: src command, messages file, unit tests, NUT file. Updated: status.ts, testCaseValidate.ts, README, mcp.md, mcp-pilot-guide.md all point to sf provar auth login instead. clear/status NUTs now seed credentials.json directly rather than depending on set-key as a test fixture. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Implements /auth/rotate endpoint: atomically replaces the stored pv_k_ key with a new one without going through the browser login flow. Old key is invalidated immediately on success. - src/commands/provar/auth/rotate.ts — new SfProvarAuthRotate command - messages/sf.provar.auth.rotate.md — summary, description, examples - src/services/qualityHub/client.ts — rotateKey() function + indirection entry - test/unit/commands/provar/auth/rotate.test.ts — 5 unit tests (599 total) - README.md, docs/mcp.md — rotate command documentation Root cause of test ERROR: null debugged and fixed — ts-node/esm surfaces noUnusedLocals TS6133 as a null-prototype error when a module-level sinon stub variable is declared but never read (sinon.restore() cleans up without referencing it). Fixed by inlining stubs inside each it() block. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 25 out of 26 changed files in this pull request and generated 9 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-13T15:30:02Z

src/services/auth/loginFlow.ts

+      const parsed = new URL(req.url ?? '/', `http://localhost:${port}`);
+      const code = parsed.searchParams.get('code');
+      const error = parsed.searchParams.get('error');
+      const description = parsed.searchParams.get('error_description');
+


The callback handler does not validate the OAuth state parameter (CSRF protection). login.ts generates state and includes it in the authorize URL, but listenForCallback() never checks it on the redirect, so a forged callback could be accepted. Consider passing the expected state into listenForCallback and rejecting when it’s missing/mismatched.

Copilot · 2026-04-13T15:30:02Z

src/services/auth/loginFlow.ts

+      // PowerShell's Start-Process passes the URL as a single argument without shell interpretation.
+      execFile('powershell.exe', ['-NoProfile', '-Command', `Start-Process '${url}'`]);


On Windows, openBrowser() builds a PowerShell command string with the URL embedded in single quotes. If the URL ever contains a single quote, this will break quoting (and can reintroduce injection risk). Prefer passing the URL as a separate argument (or escape single quotes per PowerShell rules) rather than interpolating into -Command.

Suggested change

// PowerShell's Start-Process passes the URL as a single argument without shell interpretation.

execFile('powershell.exe', ['-NoProfile', '-Command', `Start-Process '${url}'`]);

// Pass the URL as a separate PowerShell argument so it is not interpolated

// into the -Command string, avoiding quote-breaking and injection issues.

execFile('powershell.exe', ['-NoProfile', '-Command', 'Start-Process $args[0]', url]);

Copilot · 2026-04-13T15:30:03Z

src/commands/provar/auth/status.ts

+    if (envKey) {
+      if (!envKey.startsWith('pv_k_')) {
+        this.log('API key misconfigured.');
+        this.log('  Source:   environment variable (PROVAR_API_KEY)');
+        this.log(`  Value:    "${envKey.substring(0, 10)}..." does not start with "pv_k_"`);
+        this.log('');
+        this.log('  Validation mode: local only (invalid key — not used for API calls)');
+        this.log('  Fix: update PROVAR_API_KEY to a valid pv_k_ key from https://success.provartesting.com');
+        return;


If PROVAR_API_KEY is set but misconfigured (doesn’t start with pv_k_), this command returns early and never checks ~/.provar/credentials.json. That’s inconsistent with resolveApiKey() (which ignores invalid env keys and falls back to the stored key), and may incorrectly report “local only” even when a valid stored key exists. Consider continuing to the stored-credentials branch when the env var is invalid.

Copilot · 2026-04-13T15:30:03Z

src/commands/provar/auth/status.ts

+      let liveValid: boolean | undefined;
+      try {
+        const live = await qualityHubClient.fetchKeyStatus(stored.api_key, getQualityHubBaseUrl());
+        liveValid = live.valid;


fetchKeyStatus() can return username, but the result isn’t applied to stored (only tier/expires_at). If the intent is to show “Account” info in auth status, consider populating stored.username from the live response (and/or persisting these fields back to the credentials file if that’s desired).

Suggested change

liveValid = live.valid;

liveValid = live.valid;

if (live.username) stored.username = live.username;

Copilot · 2026-04-13T15:30:03Z

src/commands/provar/auth/login.ts

+    // ── Step 6: Exchange Cognito access token for pv_k_ key ─────────────────
+    // Cognito tokens are held in memory only — discarded after this call.
+    const keyData = await qualityHubClient.exchangeTokenForKey(tokens.access_token, baseUrl);
+
+    // ── Step 7: Persist the pv_k_ key ──────────────────────────────────────
+    writeCredentials(keyData.api_key, keyData.prefix, 'cognito');
+
+    this.log(`\nAuthenticated as ${keyData.username} (${keyData.tier} tier)`);
+    this.log(`API key stored (prefix: ${keyData.prefix}). Valid until ${keyData.expires_at}.`);
+    this.log("  Run 'sf provar auth status' to check at any time.");


login receives username, tier, and expires_at from the exchange response, but only writes api_key/prefix to disk. Since StoredCredentials already supports these optional fields and auth status attempts to display them, consider persisting them at login time so status works offline and the “Account/Tier/Expires” lines aren’t effectively dead code.

Copilot · 2026-04-13T15:30:03Z

docs/mcp.md

+| Variable | Purpose | Default |
+|---|---|---|
+| `PROVAR_API_KEY` | API key for Quality Hub validation | None — falls back to `~/.provar/credentials.json` |
+| `PROVAR_QUALITY_HUB_URL` | Override the Quality Hub API base URL | Production URL |


This docs table says the default for PROVAR_QUALITY_HUB_URL is “Production URL”, but getQualityHubBaseUrl() currently defaults to a /dev API Gateway URL. Please align the documentation with the actual default (or update the code default if production is intended).

Suggested change

| `PROVAR_QUALITY_HUB_URL` | Override the Quality Hub API base URL | Production URL |

| `PROVAR_QUALITY_HUB_URL` | Override the Quality Hub API base URL | Dev API Gateway URL (`/dev`) |

Copilot · 2026-04-13T15:30:04Z

docs/mcp.md

 | `best_practices_rules_evaluated` | integer        | How many best-practices rules were checked                                |
+| `validation_source`              | string         | `quality_hub`, `local`, or `local_fallback` — see Authentication section  |
+| `validation_warning`             | string         | Present when `validation_source` is `local_fallback` — explains why       |



The docs say validation_warning is only present for local_fallback, but provar.testcase.validate also returns an onboarding validation_warning when validation_source is local (no key configured). Update this description to reflect that the field can be present for both local and local_fallback (with different messages).

Copilot · 2026-04-13T15:30:04Z

src/services/qualityHub/client.ts

+    const parsed = new NodeURL(url);
+    const opts = {
+      hostname: parsed.hostname,
+      path: parsed.pathname + parsed.search,
+      method,


httpsRequest() ignores the URL’s port (and protocol). Since baseUrl is user-configurable (flag/env var) for non-prod environments, URLs like https://localhost:8443 will silently use port 443 and fail. Consider wiring parsed.port into the request options (and/or supporting http: when needed).

Copilot · 2026-04-13T15:30:04Z

src/services/qualityHub/client.ts

+    const req = https.request(opts, (res) => {
+      let data = '';
+      res.on('data', (chunk: Buffer) => {
+        data += chunk.toString('utf-8');
+      });


The HTTPS helper has no request/response timeout. A stalled connection could hang sf provar auth status, auth clear, or MCP validation indefinitely. Consider setting a reasonable timeout (e.g., via req.setTimeout / AbortSignal.timeout) and rejecting with a clear error so callers can fall back gracefully.

- Validate OAuth state parameter in listenForCallback (CSRF protection) - Fix PowerShell URL injection in openBrowser by passing URL via $args[0] - Fix status command to fall through to stored credentials on invalid env var - Apply username from live fetchKeyStatus response in status command - Persist username/tier/expires_at from login and rotate exchange responses - Fix httpsRequest to respect URL port and add 30s request timeout - Fix docs: QH URL default and validation_warning scope Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add REQUEST_ACCESS_URL constant and display it at every auth dead-end: - sf provar auth login: catch 401 from exchange and show request URL - sf provar auth status: "no key configured" block includes request URL - MCP ONBOARDING_MESSAGE and AUTH_WARNING include request URL - QualityHubAuthError from /auth/exchange includes request URL - docs/mcp.md: "Don't have an account?" in Authentication section - README.md: "Get Access" badge + inline link in MCP section - messages/sf.provar.auth.login.md and status.md updated Note: provar-mcp-public-docs.md and university-of-provar-mcp-course.md are maintained separately — flag for manual update. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Change install command to sf plugins install @provartesting/provardx-cli@beta across README.md and docs/mcp-pilot-guide.md - Add 5 NitroX (Hybrid Model) tools to the TOOLS EXPOSED list in README: provar.nitrox.discover, read, validate, generate, patch (present since beta.2, missing from docs) Note: provar-mcp-public-docs.md and university-of-provar-mcp-course.md are maintained separately — flag for manual update of install tag. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Node 25 removed SlowBuffer from the buffer module, crashing the transitive dependency buffer-equal-constant-time (via jsonwebtoken). This breaks sf provar auth *, lint, and tests. - package.json engines: cap at <25.0.0 - README: Node version note in Installation section - docs/mcp.md: add Prerequisites section with Node requirement - docs/mcp-pilot-guide.md: update Node row to 18-24 with warning Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- Add Quick start sections to both README and docs/mcp.md with numbered steps and a provardx.ping verify step - Fix Claude Code section: replace non-existent /mcp add slash command and .claude/mcp.json path with correct `claude mcp add -s user|project|local` commands and real config file locations (.mcp.json, settings.local.json) - Move license requirement before client configuration in docs/mcp.md since it is a startup blocker - Add Windows note: use sf.cmd in Claude Desktop when sf is not found Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

mrdailey99 and others added 3 commits April 10, 2026 16:12

Copilot AI review requested due to automatic review settings April 10, 2026 22:39

Copilot started reviewing on behalf of mrdailey99 April 10, 2026 22:40 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

mrdailey99 and others added 20 commits April 10, 2026 18:39

fix(nuts): correct prefix length assertions in auth NUT tests

e8d58ad

substring(0,12) on 'pv_k_nuttest...' yields 'pv_k_nuttest' (12 chars), not 'pv_k_nuttest12' (14). Same off-by-two for status test key prefix. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

fix(auth): correct Cognito Hosted UI domain

a2fe327

us-east-1qpfw was a misread — correct domain is us-east-1xpfwzwmop. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

fix(auth): revert to /oauth2/authorize — state was the missing piece

6925b19

/login is just the UI page. /oauth2/authorize with state + nonce + PKCE is the correct OAuth endpoint and confirmed working in browser testing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

fix(test): add eslint-disable camelcase to auth NUT fixtures

0fa223b

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

mrdailey99 requested a review from Copilot April 13, 2026 15:25

Copilot started reviewing on behalf of mrdailey99 April 13, 2026 15:25 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

mrdailey99 and others added 6 commits April 13, 2026 14:42

fix(test): update NUT assertion for revised status warning wording

970cdde

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Update badge label for Quality Hub API access

f4d34c9

mrdailey99 merged commit 329fb21 into develop Apr 13, 2026
2 checks passed

		// PowerShell's Start-Process passes the URL as a single argument without shell interpretation.
		execFile('powershell.exe', ['-NoProfile', '-Command', `Start-Process '${url}'`]);

-      // PowerShell's Start-Process passes the URL as a single argument without shell interpretation.
-      execFile('powershell.exe', ['-NoProfile', '-Command', `Start-Process '${url}'`]);
+      // Pass the URL as a separate PowerShell argument so it is not interpolated
+      // into the -Command string, avoiding quote-breaking and injection issues.
+      execFile('powershell.exe', ['-NoProfile', '-Command', 'Start-Process $args[0]', url]);

	liveValid = live.valid;
	liveValid = live.valid;
	if (live.username) stored.username = live.username;

	\| `PROVAR_QUALITY_HUB_URL` \| Override the Quality Hub API base URL \| Production URL \|
	\| `PROVAR_QUALITY_HUB_URL` \| Override the Quality Hub API base URL \| Dev API Gateway URL (`/dev`) \|

Conversation

mrdailey99 commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants