Skip to content

feat: adds possibility to specify cert serial number#274

Merged
stalniy merged 1 commit intomainfrom
feat/adds-cert-serial
Mar 31, 2026
Merged

feat: adds possibility to specify cert serial number#274
stalniy merged 1 commit intomainfrom
feat/adds-cert-serial

Conversation

@stalniy
Copy link
Copy Markdown
Contributor

@stalniy stalniy commented Mar 31, 2026

📝 Description

To make it possible to generate certificate with custom serial

🔧 Purpose of the Change

  • New feature implementation
  • Bug fix
  • Documentation update
  • Code refactoring
  • Dependency upgrade
  • Other: [specify]

📌 Related Issues

  • Closes #ISSUE_NUMBER
  • References #ISSUE_NUMBER

✅ Checklist

  • I've updated relevant documentation
  • Code follows Akash Network's style guide
  • I've added/updated relevant unit tests
  • Dependencies have been properly updated
  • I agree and adhered to the Contribution Guidelines

📎 Notes for Reviewers

[Include any additional context, architectural decisions, or specific areas to focus on]

@stalniy stalniy requested a review from a team as a code owner March 31, 2026 08:32
@github-actions github-actions bot added the C:ts label Mar 31, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 31, 2026

Walkthrough

An optional serial?: number field is added to the ValidityRangeOptions interface in the Certificate Manager. When provided, this serial number is used for X.509 certificate generation; otherwise, the system defaults to a time-based value derived from the current timestamp.

Changes

Cohort / File(s) Summary
Certificate Serial Configuration
ts/src/sdk/provider/auth/mtls/CertificateManager.ts
Added optional serial field to ValidityRangeOptions interface; certificate generation now respects this field when present, falling back to timestamp-based calculation if absent.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A serial field hops into place,
No more time-bound rabbit race!
Options now guide the cert's name,
With custom serials—what a game! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding the ability to specify certificate serial numbers.
Description check ✅ Passed The description covers the main purpose and includes required checklist items, but lacks detail about what problem is solved and leaves related issues and reviewer notes as placeholders.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/adds-cert-serial

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
ts/src/sdk/provider/auth/mtls/CertificateManager.ts (2)

82-82: Consider validating that serial is a positive integer.

X.509 serial numbers must be positive integers per RFC 5280. If a caller passes 0, a negative number, or a non-integer, the generated certificate would be non-compliant. Since this is a public API, defensive validation would prevent subtle bugs.

🛡️ Suggested validation

Add validation at the start of generatePEM:

   async generatePEM(address: string, options?: ValidityRangeOptions): Promise<CertificatePem> {
+    if (options?.serial !== undefined && (!Number.isInteger(options.serial) || options.serial <= 0)) {
+      throw new Error("Certificate serial number must be a positive integer");
+    }
     const rs = await getRSASignLib();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ts/src/sdk/provider/auth/mtls/CertificateManager.ts` at line 82, The
generatePEM method in CertificateManager.ts must validate the options.serial
value to ensure X.509 compliance: check the provided serial (in generatePEM) is
a finite integer greater than zero (reject 0, negative, non-integer, NaN, or
non-number); if absent use the existing default Math.floor(Date.now() * 1000);
on invalid input throw a clear error (e.g., RangeError or TypeError) with a
concise message mentioning "serial" so callers can correct it. Ensure the check
runs before constructing the serial field so serial: { int: ... } always
receives a valid positive integer.

28-32: Add JSDoc for the new serial field.

The other fields (validFrom, validTo) don't have inline docs either, but since this is a new addition, consider documenting the expected constraints: the serial should be a positive integer per X.509/RFC 5280 requirements.

📝 Suggested documentation
 /**
  * Options for specifying the validity range of a certificate.
  */
 export interface ValidityRangeOptions {
+  /**
+   * Custom serial number for the certificate. Must be a positive integer.
+   * Defaults to a time-based value if not provided.
+   */
   serial?: number;
   validFrom?: Date;
   validTo?: Date;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ts/src/sdk/provider/auth/mtls/CertificateManager.ts` around lines 28 - 32,
Add JSDoc comments to the ValidityRangeOptions interface documenting the new
serial field and expected constraints: annotate serial as a positive integer
(per X.509 / RFC 5280) and optional, and briefly document validFrom and validTo
as optional Date values representing the certificate's notBefore and notAfter
times; update the JSDoc above the ValidityRangeOptions interface (and inline
above serial, validFrom, validTo) to state types, optionality, units/semantics,
and the serial must be > 0.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@ts/src/sdk/provider/auth/mtls/CertificateManager.ts`:
- Line 82: The generatePEM method in CertificateManager.ts must validate the
options.serial value to ensure X.509 compliance: check the provided serial (in
generatePEM) is a finite integer greater than zero (reject 0, negative,
non-integer, NaN, or non-number); if absent use the existing default
Math.floor(Date.now() * 1000); on invalid input throw a clear error (e.g.,
RangeError or TypeError) with a concise message mentioning "serial" so callers
can correct it. Ensure the check runs before constructing the serial field so
serial: { int: ... } always receives a valid positive integer.
- Around line 28-32: Add JSDoc comments to the ValidityRangeOptions interface
documenting the new serial field and expected constraints: annotate serial as a
positive integer (per X.509 / RFC 5280) and optional, and briefly document
validFrom and validTo as optional Date values representing the certificate's
notBefore and notAfter times; update the JSDoc above the ValidityRangeOptions
interface (and inline above serial, validFrom, validTo) to state types,
optionality, units/semantics, and the serial must be > 0.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3679191f-e006-4e9b-b92e-ccc68ad18658

📥 Commits

Reviewing files that changed from the base of the PR and between 215c3de and e752516.

📒 Files selected for processing (1)
  • ts/src/sdk/provider/auth/mtls/CertificateManager.ts

@stalniy stalniy merged commit 5322905 into main Mar 31, 2026
7 checks passed
@stalniy stalniy deleted the feat/adds-cert-serial branch March 31, 2026 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants