test(crypto): cover RS256 JWT signing for PKCS8 and PKCS1 keys

[jaso0n0818]

Summary

Adds unit coverage for signRs256Jwt in src/utils/crypto.ts, including PKCS#8 PEM keys and legacy PKCS#1 RSA PEM keys with escaped newlines.

Why

GitHub App JWT signing is security-sensitive; the helper had no direct regression tests outside indirect github-app mocks.

Validation

npx vitest run test/unit/crypto-jwt.test.ts

No product behavior changes; tests only.

[ghost]

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal	Result	Evidence	Action
Gate result	⚠️ Skipped	#725 is no longer open.	No action.

---

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[ghost]

gittensory · advisory review

Reviewed 1 changed file(s) — two independent AI reviewers.

Changed files: test/unit/crypto-jwt.test.ts

Suggested action: 🛠️ Request changes. (reviewers split: request changes / merge) Address the suggestions below before merging.

Reviewer A · gpt-oss-120b — recommends 🛠️ request changes
The PR adds a new unit test for RS256 JWT signing covering PKCS#8 and PKCS#1 PEM keys, but the test imports are incorrect and will cause a runtime error.

Suggestions

• Import the Web Crypto API correctly, e.g. import { webcrypto as crypto } from "node:crypto" or use globalThis.crypto if available.
• Remove the unused generateKeyPairSync import if not needed, or use it consistently for both key types.
• Consider adding a test case for an invalid PEM to ensure proper error handling.

Worth double-checking

• The test will fail during CI due to undefined crypto, breaking the test suite.
• If the test passes without proper key generation, it may give false confidence in the signing implementation.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
The PR adds a new unit test file for the RS256 JWT signing function, covering both PKCS#8 and PKCS#1 key formats. The tests are well-structured, use appropriate async handling for Web Crypto API, and verify JWT structure and encoding. No changes to production code are made, and the test file adheres to project testing conventions.

Suggestions

• Consider avoiding the non-null assertion operator (!) in the test by using a temporary variable after the truthy check for improved clarity.
• The PKCS#1 test uses synchronous key generation inside an async test; while acceptable for unit tests, ensure this doesn't inadvertently block the test suite in CI if key generation becomes expensive.

Worth double-checking

• The test assumes the signRs256Jwt function correctly handles escaped newlines in PKCS#1 PEM strings (by unescaping). Verify the implementation matches this expectation.
• The test does not verify cryptographic signature validity (only token structure), which is acceptable for a unit test focused on input handling but ensures signature verification is covered elsewhere.

[JSONbored]

PR #725 — test(crypto): cover RS256 JWT signing for PKCS8 and PKCS1 keys
Author: jaso0n0818 | CI: ❌ validate FAIL | Status: Concerns
ACTION: REQUEST CHANGES

Highlights:

• Genuinely useful security-sensitive coverage for signRs256Jwt (GitHub App JWT) across PKCS#8 and legacy PKCS#1-with-escaped-newlines — a real gap.
• Clean conventional title, clear what/why, exact validation command listed.

Change requests / reasons:

• Latent bug: test uses crypto.subtle.generateKey but only imports generateKeyPairSync from node:crypto — global crypto not imported → ReferenceError where there's no crypto global. Use import { webcrypto as crypto } from "node:crypto".
• CI validate failing

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.69%. Comparing base (eb440f1) to head (72d4584).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #725   +/-   ##
=======================================
  Coverage   96.69%   96.69%           
=======================================
  Files          94       94           
  Lines       13845    13845           
  Branches     5050     5050           
=======================================
  Hits        13388    13388           
  Misses         88       88           
  Partials      369      369           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[JSONbored]

PR #725 — test(crypto): cover RS256 JWT signing for PKCS8 and PKCS1 keys
Verdict: MERGE
Resolves issue: N/A (test-only, no linked issue)
CI: All green (test 1/2, lint, workers, codecov). Gittensory Context neutral.

Highlights:

• Meaningful, not padding: exercises both real branches of importPkcs8PrivateKey — direct PKCS#8 import and the legacy PKCS#1 path through the hand-rolled wrapPkcs1RsaPrivateKey DER-wrapper — with \n-escaped-newline normalization. Genuinely under-tested security-sensitive code (previously only via github-app mocks).
• Asserts the signing contract, not just "didn't throw": 3-part token, base64url signature with no +/= chars, header/payload decode to expected alg/iss. crypto.subtle is fine under vitest node env (confirmed by green shards).

Concerns / required changes:

• Optional only: tests verify token structure but don't cryptographically round-trip-verify the signature against the public key (would prove the PKCS#1 DER-wrapping is byte-correct). Not required — importKey would reject a malformed pkcs8.

Completeness vs issue: No linked issue; as test-only coverage it is correct, meaningful, and in-scope (src/utils/crypto.ts). No product behavior change.