Feature: Certificate Validation

[sephynox]

Summary

Adds an optional, on-chain certificate-chain gate to Anchor HTTP services.

[Copilot]

Pull request overview

Adds an optional, on-chain certificate-chain gate to Anchor HTTP services (and wires it into the Storage service) so authenticated callers can be required to present a published certificate chaining to a trusted issuer, with structured error reporting and storage client tests covering the new behavior.

Changes:

• Introduced requireCertificateChain server config and a resolved/cached requirement on KeetaNetAnchorHTTPServer.
• Implemented certificate-chain verification utilities + a new structured user error (KeetaAnchorCertificateRequiredError) for missing/untrusted chains.
• Enforced the gate across Storage authenticated endpoints and added Storage client tests for 401/403 failure modes.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/services/storage/server.ts	Threads resolved certificate-chain requirement through auth helpers to enforce the gate on signed requests.
src/services/storage/common.ts	Exposes CertificateRequired in Storage Errors for client-side deserialization/matching.
src/services/storage/client.test.ts	Adds a harness to mint/publish cert chains and tests missing/untrusted rejection behavior.
src/lib/http-server/index.ts	Adds requireCertificateChain config, resolves it once, and provides a helper to enforce it.
src/lib/certificates.ts	Implements chain verification + new KeetaAnchorCertificateRequiredError + config resolution helpers.
src/lib/certificates.generated.ts	Adds typia assertion for the new error JSON payload shape.

[chpzcch]

Review of PR 295 — Certificate Validation Gate

Issue 1 — server.ts bypasses the base class assertCertificateChain() method

The base class adds protected async assertCertificateChain(account: Account) precisely so subclasses don't need to import and wire the underlying function manually. But KeetaNetStorageAnchorHTTPServer imports assertAccountCertificateChain directly from certificates.ts and threads resolvedCertificateChainRequirement through verifyBodyAuth / verifyURLAuth as a parameter.

This creates two parallel paths for the same gate. The assertCertificateChain method on the base class is dead code for the storage service. Future subclasses will have to decide which pattern to follow.

Suggest: Either remove assertCertificateChain from the base class (if the parameter-threading pattern is the intended one), or refactor verifyBodyAuth / verifyURLAuth to be instance methods that use this.assertCertificateChain() so the base class method is actually used.

---

Issue 2 — Silent swallow of Certificate constructor errors in verifyAccountCertificateChain

try {
    candidate = new Certificate(record.certificate.toPEM(), {
        store: { root: rootSet, intermediate }
    });
} catch {
    continue;  // ← any construction error silently treated as "cert doesn't count"
}

A malformed or structurally invalid on-chain certificate silently falls through and does not contribute to chain verification. If an attacker could publish a cert that throws during construction but the account also has no valid cert, they'd still get 'untrusted' rather than leaking information — so this isn't a security regression.

However, the silent swallow makes debugging hard: an operator who publishes a cert with an unexpected format will see 401/403 with no signal about why their cert was rejected. Consider logging the error at debug level before continuing.

---

Issue 3 — Missing test: public route still serves when requireCertificateChain is set

The existing tests confirm that authenticated routes correctly enforce the cert gate (401 for missing, 403 for untrusted). But there's no test that GET /api/public/** continues to serve anonymous callers when requireCertificateChain is configured.

The route intentionally does not call assertAccountCertificateChain — the signer of a pre-signed public URL may have no cert published (e.g. a service account), and the fetch is done by a third party with no account at all. This is the correct design.

But without a test, the exemption is invisible. A future refactor that accidentally wires requireCertificateChain into the public route would have no regression safety net.

Suggest: Add a test case in client.test.ts that, with { useCertChain: true } configured, generates a pre-signed public URL (signed by the cert-holding account) and confirms an anonymous caller can fetch it successfully.

---

Issue 4 — implements Required<KeetaAnchorHTTPServerConfig> with an optional field

KeetaAnchorHTTPServerConfig.requireCertificateChain is CertificateChainConfig | undefined. The class declares readonly requireCertificateChain: CertificateChainConfig | undefined, which satisfies Required<> because Required<> only removes the ? modifier — it does not exclude undefined from the value type. So TypeScript is happy here.

This is worth noting because it can surprise maintainers: Required<T> does not mean "never undefined", it means "not optional". This is already the pattern used by logger (Logger | undefined). No change needed, but the JSDoc on the interface field makes this slightly confusing — consider a brief note that undefined is the off state.

---

No analytics files are touched. SonarCloud passed at 91.7% coverage on new code.

[chpzcch]

Code Review Summary

Issues reviewed: No linked issue found in the PR body. The PR summary describes the feature directly: optional on-chain certificate-chain gate for Anchor HTTP services.

Threads addressed: 2 threads reviewed (both already resolved before this review)

• Thread on src/services/storage/server.ts line 486 (@copilot-pull-request-reviewer): Resolved — author closed with "No." Public route bypass is intentional by design.
• Thread on src/lib/http-server/index.ts line 81 (@copilot-pull-request-reviewer): Resolved — PR description has since been updated.

New issues found:

Must Fix

None

Consider Simplifying

1. server.ts bypasses protected assertCertificateChain() from the base class — the base class adds this method specifically for subclasses to use, but KeetaNetStorageAnchorHTTPServer imports assertAccountCertificateChain directly and threads resolvedCertificateChainRequirement through every verifyBodyAuth / verifyURLAuth call. This leaves assertCertificateChain as dead code in the base class. Either remove it, or make verifyBodyAuth/verifyURLAuth instance methods that call this.assertCertificateChain(). The parameter-threading approach isn't wrong — it's actually cleaner for module-level functions — but the two patterns should be reconciled.

2. Missing test: public route succeeds when cert chain is required — GET /api/public/** intentionally skips cert enforcement (anonymous callers fetch pre-signed public URLs). This is the correct design, but there's no regression test to prove the public route still works when requireCertificateChain is configured. A future change that accidentally wires the gate into the public route would not be caught.

Minor / Optional

• Silent swallow of Certificate constructor errors in verifyAccountCertificateChain — malformed certs are silently skipped (catch { continue }). Not a security issue, but unhelpful for diagnostics. Consider a debug-level log before continuing.
• implements Required<KeetaAnchorHTTPServerConfig> with requireCertificateChain: CertificateChainConfig | undefined — TypeScript allows this (Required<> removes ?, not undefined). Matches the existing logger pattern. A brief JSDoc note that undefined is the "off" state would reduce confusion for future maintainers.

⚠️ Analytics Impact

Event registry (/shared/analytics.ts ↔ /docs/ANALYTICS_EVENTS.md):
Not applicable — this is a library/service repo (KeetaNetwork/anchor), not the ukcoin2 application. No /shared/analytics.ts or /docs/ANALYTICS_EVENTS.md exist in this repository. No analytics event calls appear anywhere in the diff.

PostHog events and funnels:
None detected.

Admin analytics:
None detected.

What's working well

• The gate is genuinely additive and backward-compatible: omitting requireCertificateChain from the config leaves all existing behaviour unchanged. No existing tests needed modification beyond renaming one test to reflect the new enforcement.
• The error class design is solid: KeetaAnchorCertificateRequiredError distinguishes 'missing' (401) from 'untrusted' (403), serializes cleanly via toJSON/fromJSON, and uses the object-type-ID pattern consistent with the rest of the error hierarchy.
• resolveCertificateChainConfig validates eagerly at construction time (throws if trustedIssuers is empty), which surfaces misconfiguration before any request is served.
• acceptedIssuerDNs is pre-computed once in resolveCertificateChainConfig so error payloads don't re-walk the issuer list on every rejected request.
• The typia.createAssertEquals validator on KeetaAnchorCertificateRequiredErrorJSONProperties in the generated file gives strict deserialization at the client side — excess properties are rejected.
• SonarCloud: Quality Gate passed, 0 new issues, 91.7% coverage on new code, 0% duplication.
• Test coverage: 401 (missing cert) and 403 (untrusted issuer) are both exercised with a realistic root → intermediate → leaf chain.

Overall assessment:
The PR correctly implements an optional certificate-chain gate across all authenticated storage routes (PUT, GET, DELETE, metadata, search, quota), leaving unauthenticated/public routes deliberately unaffected. The implementation is safe to merge as-is — no must-fix issues. The two "consider simplifying" items are design consistency concerns worth addressing in a follow-up: the dual-path inconsistency between assertCertificateChain() (base class) and direct import (storage server) will confuse the next developer implementing a new anchor service, and the missing public-route regression test is a low-cost safety net worth adding. Neither is a blocker.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
94.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud