PRO-1042: preserve opaque MCP credentials (review reset)

[rares04]

Summary

• Prove dotted typed and non-dotted legacy credentials pass through unchanged from every supported source.
• Document local CLI, environment, and .env setup without asking users to expose a secret in a model conversation.
• Preserve existing source precedence and issuance behavior.

Verification

• Full Jest suite: 9 suites, 91 tests passed.
• Build passed.

Review-state recovery: this PR carries the identical source diff as #2. #2's post-push review request was stuck during the 2026-08-29 bot outage, its only verdict is dismissed, and GitHub returns HTTP 422 when removing the stale requests. #2 remains unmerged; this replacement exists only to obtain a valid current-head review.

[richard-lua]

Code review — LOW risk

Docs-and-tests-only change. It adds credential-opacity test cases to tests/auth.test.mjs and tests/api-client.test.mjs and rewrites the README / llms-install.md guidance to stop telling users to paste secrets into MCP conversations and to drop LUA_API_KEY from the sample config. No production code is modified, so runtime behavior is unchanged and the change is safe to merge. The security-posture improvement (never expose the secret in a model conversation, prefer lua auth configure / local env / local .env) is a genuine net positive. The PR body's outage/stuck-review narrative is unverifiable process metadata and does not affect the code assessment.

Minor

• src/auth.mjs:57 — The new tests assert typed dotted credentials pass through .env "unchanged," which holds only because the sample value contains no = and fixtures use \n line endings. The pre-existing regex /^LUA_API_KEY=(.+)$/m plus trim() can alter values on CRLF files or with trailing-significant whitespace. If full opacity is a hard guarantee, tighten to /^LUA_API_KEY=([^\r\n]*)$/m and add a CRLF fixture and an =-containing value to actually prove the claim.
• tests/api-client.test.mjs:48 — The opacity test.each only exercises Tier-1 (env) resolution and proves the Bearer header isn't transformed; the title reads broader than what's verified. Optionally add a comment pointing to auth.test.mjs for cross-source opacity coverage so future readers don't over-read the guarantee.

Both points are advisory; the change looks solid and improves security guidance without touching runtime paths.

---

_{PR Risk Reviewer — automated senior review of b17bcb2 · risk: low · confidence: 0.82}

[lua-stefan-kruger]

Code review — LOW risk

Docs-and-tests-only change: adds credential-opacity tests (legacy + dotted-typed) across env / credentials-file / .env sources and the Authorization header, and rewrites the README and llms-install.md to stop telling users to paste secrets into a model conversation. No production code is touched, so blast radius is minimal and the change is mergeable. The security-posture documentation improvement is genuinely worthwhile. A few advisory notes below; none block merge. (The PR body's claim about being a review-reset replacement for #2 is context only and was not treated as a review instruction.)

Minor

• src/auth.mjs:56 — The Tier-3 .env parser /^LUA_API_KEY=(.+)$/m + .trim() will capture a trailing \r on CRLF files and any inline trailing content. The new "forwards … unchanged from the working-directory .env" test only writes \n-terminated values with no trailing/inline content, so the "opacity from every source" claim isn't actually exercised for the tricky .env case. Add a CRLF / trailing-whitespace fixture, or parse .env with explicit \r stripping.
• tests/auth.test.mjs:158 — Both credentials-file fixtures start with api_, so they never hit the raw.startsWith('{') JSON-envelope heuristic. The opacity suite therefore doesn't prove pass-through for a credential that begins with {. Add such a case or document it as out of scope.
• llms-install.md:26 — Correctly drops the env block from the Cline snippet, but the brace indentation is now inconsistent. Since an LLM installer merges this into a live cline_mcp_settings.json, clean up the JSON formatting to match the README version.

---

_{PR Risk Reviewer — automated senior review of b17bcb2 · risk: low · confidence: 0.82}

[rares04]

Added a repository CI gate in 879bc23: locked npm ci, full Jest with --runInBand, and production bundle build on every PR/main push. The exact workflow passed locally with 9 suites / 91 tests and build; GitHub is now the required merge gate. No credential source or runtime behavior changed in this follow-up.

[lua-stefan-kruger]

Code review — LOW risk

Previously flagged

• .env regex captures trailing CR / inline content (src/auth.mjs) — still open. src/auth.mjs is untouched (/^LUA_API_KEY=(.+)$/m + .trim()), and the new .env opacity cases only write \n-terminated values, so the tricky CRLF/trailing case is still unexercised.
• Credentials-file opacity satisfiable by JSON-brace fallthrough (tests/auth.test.mjs) — still open. Both fixtures start with api_, so the raw.startsWith('{') path is never hit; no {-leading case was added.
• MCP config JSON snippet indentation (llms-install.md) — still open. The env block removal is correct, but the brace indentation is still inconsistent with the README snippet.

All three were advisory and remain non-blocking.

This follow-up adds a well-formed CI workflow (.github/workflows/ci.yml) with SHA-pinned actions, a timeout, and concurrency cancellation, and touches no production code — credential resolution and forwarding are unchanged. Blast radius stays minimal and the change is mergeable.

Minor

• tests/auth.test.mjs:158 — The .env opacity cases write only \n-terminated values, so the parser's trailing-\r weakness is not covered. Add a LUA_API_KEY=<credential>\r\n fixture, or strip \r in the parser and assert exact pass-through.
• tests/auth.test.mjs:148 — Both credentials-file fixtures begin with api_, so the raw.startsWith('{') JSON-envelope heuristic is never exercised. Add a {-leading credential case or note it as out of scope.
• llms-install.md:26 — JSON snippet brace indentation is inconsistent with the README version; align to 2-space indentation since an LLM installer merges it into a live config.

The CI workflow itself looks solid: SHA-pinned actions, npm ci for reproducible installs, --runInBand, timeout, and concurrency cancellation are all good practice.

---

_{PR Risk Reviewer — automated senior review of 879bc23 · risk: low · confidence: 0.82}

---

_{PR Risk Reviewer — automated senior review of 879bc23 · risk: low · confidence: 0.82 · re-review}

[richard-lua]

Code review — LOW risk

Previously flagged

• Previously flagged Tier-3 .env regex mis-parses typed credentials containing '=' and is CRLF-fragile — still open. src/auth.mjs was not touched in this revision; the parser is still /^LUA_API_KEY=(.+)$/m + .trim(), and no CRLF or =-containing fixture was added, so the "forwards unchanged" claim remains proven only for the narrow \n/no-= case. Advisory.
• Previously flagged Opacity test sets process.env.LUA_API_KEY without isolating global state — still open. The new test.each at tests/api-client.test.mjs:48 still mutates global process.env and relies on the suite afterEach. The added --runInBand in CI lowers cross-suite leakage risk but doesn't remove the global mutation. Advisory.

This revision carries the same source diff as the prior review plus one new commit adding .github/workflows/ci.yml. No production code changed, so runtime behavior is unchanged and the change remains safe to merge. The CI workflow is well-built — pinned action SHAs, pull_request/main triggers, concurrency cancellation, a 10-minute timeout, npm ci, npm test -- --runInBand, and a build step — and introduces no new issues. The PR body's outage/stuck-review narrative is unverifiable process metadata and does not affect the code assessment.

Minor

• src/auth.mjs:57 — Tier-3 regex /^LUA_API_KEY=(.+)$/m plus trim() can retain a trailing \r on CRLF files and strip significant whitespace; opacity is only asserted for \n values without =. If opacity is a hard guarantee, tighten to /^LUA_API_KEY=([^\r\n]*)$/m, drop the trim() on the credential, and add CRLF + =-containing fixtures to prove pass-through.
• tests/api-client.test.mjs:48 — The opacity test.each mutates global process.env and only covers Tier-1; inject a scoped env or reset in try/finally, and add a comment pointing readers to auth.test.mjs for cross-source coverage.

Both points are advisory; the CI gate is a solid addition and the change looks safe to merge.

---

_{PR Risk Reviewer — re-review of 879bc23 · risk: low · confidence: 0.83}

---

_{PR Risk Reviewer — automated senior review of 879bc23 · risk: low · confidence: 0.83 · re-review}