🎯 Recommended Merge Targets

Based on JIRA ticket TT-17295: Support loading of certificates in gateway with certificate content

Fix Version: Tyk 5.14.0

⚠️ Warning: Expected release branches not found in repository

Required:

• master - No matching release branches found. Fix will be included in future releases.

📋 Workflow

1. Merge this PR to master first

This PR introduces the capability for the Gateway to load TLS certificates directly from PEM-encoded strings embedded within API definitions. This adds a third certificate source, alongside the existing methods of loading from a file path or a certificate store ID (SHA256).

The primary goal is to improve operational security and simplify certificate management by allowing secrets management systems (like Vault) to inject certificate content directly at load time, without needing to persist private keys in Redis.

Key changes include:

• certs/manager.go: The CertificateManager.List() method is updated to detect if a given string is PEM content (using a new IsPEMContent helper). If so, it parses, validates, and caches the certificate in-memory. This includes a fallback to normalize PEMs that have had their line breaks collapsed into a single line.
• gateway/cert_usage_tracker.go: The certificate usage tracker, which identifies certificates for synchronization, is modified to explicitly ignore these embedded PEM strings. Since they are inline content, they do not need to be fetched or synced.
• gateway/api_definition.go: The secret substitution logic (for env://, vault://, etc.) is enhanced to correctly JSON-escape multiline values. This is critical to ensure that injected PEM blocks do not invalidate the API definition's JSON structure.
• Testing: The PR is supported by extensive tests, including new unit tests in certs/manager_test.go and a new suite of end-to-end integration tests in gateway/cert_embedded_pem_test.go that validate the functionality across all affected certificate fields (server certs, mTLS, upstream certs, etc.).

Files Changed Analysis

The changes span 8 files, with a significant number of additions (920) versus deletions (26). This ratio is driven by the addition of comprehensive tests, which account for the majority of the new code.

• Core Logic (certs/manager.go): Contains the primary feature implementation, adding the new branch to detect, parse, and cache inline PEM content.
• Testing (certs/manager_test.go, gateway/cert_embedded_pem_test.go): The bulk of the changes. A new integration test file (cert_embedded_pem_test.go) provides robust end-to-end validation for all affected certificate use cases. Unit tests in manager_test.go cover edge cases like malformed PEMs, whitespace, and caching logic.
• Supporting Changes (gateway/api_definition.go, gateway/util.go): A crucial enhancement was made to JSON-escape secrets during substitution, ensuring that multiline PEM values don't break the API definition's structure.
• System Integration (gateway/cert_usage_tracker.go): The certificate usage tracker is updated to ignore inline PEMs, correctly identifying them as non-syncable, ephemeral content.

Architecture & Impact Assessment

• What this PR accomplishes: It decouples certificate loading from persistent storage (filesystem or Redis) by allowing certificates to be provided as ephemeral, inline content within an API definition. This is a foundational step for enabling seamless integration with external secret management systems.

• Key technical changes introduced:

  1. A new certificate source type (inline PEM) is added to CertificateManager.List() and CertPool().
  2. Parsed inline PEMs are cached in-memory, with a cache key derived from a hash of the PEM content.
  3. Secret substitution logic is updated to handle multiline strings by properly escaping them for JSON.
  4. The certificate usage tracker for MDCB sync is updated to exclude these inline certificates.

• Affected system components: The change impacts all systems that resolve certificates through the CertificateManager:

  • Gateway TLS Termination (Global SSLCertificates and Per-API certificates)
  • Mutual TLS (mTLS) for client authentication (client_certificates)
  • Upstream mTLS (upstream_certificates)
  • Control API mTLS (Security.Certificates.ControlAPI)
  • Pinned Public Keys (pinned_public_keys)

Certificate Loading Flow

graph TD
    A[API Definition Cert Field] --> B["CertificateManager.List()"];
    B --> C{Is it PEM content?};
    C -- Yes --> D[Parse & Cache In-Memory];
    C -- No --> E{Is it a file path?};
    E -- Yes --> F[Load from Filesystem];
    E -- No --> G["Fetch from Storage (Redis)"];
    D --> H[Return tls.Certificate];
    F --> H;
    G --> H;

Scope Discovery & Context Expansion

The scope is well-contained within the gateway's certificate management and API loading logic. The author has correctly identified and addressed the downstream impact on the cert_usage_tracker, preventing large PEM strings from being treated as syncable certificate IDs.

The related changes in gateway/api_definition.go to handle JSON escaping for secrets are critical for the feature to work with secret substitution (vault://, env://), which is a primary use case. This demonstrates a thorough understanding of the broader context. The PR effectively enables a future where secret resolvers can fetch a PEM from a vault and inject it directly into the API definition before it's loaded by the gateway.

References

• certs/manager.go: Core implementation of embedded PEM handling.
• gateway/cert_embedded_pem_test.go: End-to-end tests demonstrating usage in various certificate fields.
• gateway/cert_usage_tracker.go: Exclusion of PEM content from the certificate usage map.
• gateway/api_definition.go: Secret substitution logic updated for multiline values.

Powered by Visor from Probelabs

Last updated: 2026-06-11T12:24:14.902Z | Triggered by: pr_updated | Commit: 3d34818

💡 TIP: You can chat with Visor using /visor ask <your question>

Security Issues (2)

Architecture Issues (1)

Performance Issues (1)

Security Issues (2)

Quality Issues (3)

Powered by Visor from Probelabs

Last updated: 2026-06-11T12:24:07.027Z | Triggered by: pr_updated | Commit: 3d34818

💡 TIP: You can chat with Visor using /visor ask <your question>

@kofoworola — yes, that's exactly what gateway/cert_embedded_pem_test.go does. Each test spins up the full gateway via StartTest, builds an API definition with the PEM embedded directly into the relevant field, loads it via BuildAndLoadAPI / LoadAPI, then drives a real HTTP/TLS request through the gateway with ts.Run and asserts behaviour. No file paths, no CertificateManager.Add() calls — the PEM lives in the API definition itself.

Specifically:

• TestEmbeddedPEM_ClientCertificates — API def with spec.ClientCertificates = []string{string(clientCertPem)}, spec.UseMutualTLSAuth = true. Asserts the gateway accepts a client signed by the embedded CA and rejects a client without a cert.
• TestEmbeddedPEM_UpstreamCertificates — API def with spec.UpstreamCertificates = map[string]string{\"*.target:<port>\": string(combinedClientPEM)} proxying to an mTLS-protected upstream. Asserts the upstream handshake succeeds using the embedded client PEM.
• TestEmbeddedPEM_DomainCertificates — API def with spec.Certificates = []string{string(combinedPEM)} for per-API TLS termination. Asserts a TLS client gets 200 OK.
• TestEmbeddedPEM_GlobalSSLCertificates — global HttpServerOptions.SSLCertificates = []string{string(combinedPEM)}. Asserts the gateway boots with the embedded server cert and serves HTTPS.
• TestEmbeddedPEM_ControlAPIClientCAs — global Security.Certificates.ControlAPI = []string{string(clientCertPem)} (exercises CertPool() rather than just List()). Asserts the management API accepts a client signed by the embedded CA.
• TestEmbeddedPEM_BackwardCompatibility — a table test that resolves the same cert through all three sources (file path, cert ID via Add(), embedded PEM) and asserts they coexist.

Happy to add more coverage if you want a specific scenario that isn't there — e.g. an OAS API definition path, or a hot-reload scenario where an embedded PEM changes on LoadAPI.

QA review by Quinn · TT-17295 · PR #8256 · 2026-06-04

PR Summary

PR #8256 adds a third certificate-source mode to CertificateManager.List() and CertPool(): inline PEM content. The change touches certs/manager.go (new IsPEMContent helper, appendEmbeddedPEM method, and the embedded-PEM branch inside List()) and gateway/cert_usage_tracker.go (excludes raw PEM strings from MDCB cert-sync tracking). Backward compatibility is preserved — existing cert-ID and file-path flows are untouched. The PR aligns with TT-17295 and its three acceptance criteria.

Jira Alignment

Criteria check:

Unit & Integration Test Coverage

Test quality notes:

• Naming follows TestFunctionName_Scenario convention throughout. ✅
• Table-driven pattern used for TestEmbeddedPEM_BackwardCompatibility. ✅
• TestList_EmbeddedPEM_MalformedBodyReturnsNil and TestList_EmbeddedPEM_TruncatedReturnsNil explicitly test the nil-sentinel error path. ✅
• TestList_EmbeddedPEM_CacheKeyUsesContentHash validates cache key uniqueness by content. ✅

UI Test Coverage (Playwright)

No frontend files changed — section not applicable.

Security Findings

PR-specific security review (no issues found):

• PEM content is NOT logged on parse failure — appendEmbeddedPEM logs only the error, not the PEM body. PR description explicitly calls this out. ✅
• IsPEMContent substring match is permissive by design; ParsePEMCertificate performs strict downstream validation. The comment documenting why false positives are safe is clear and will survive future readers. ✅
• embeddedPEMCacheKeyPrefix = "embedded-pem-" contains - which disqualifies it from being a hex-only SHA256 cert ID, eliminating org-prefixed cache shadow attacks. ✅
• SHA256 of trimmed PEM content as cache key: content-addressed deduplication across API definitions; collision risk is computationally infeasible. ✅
• Embedded PEM correctly excluded from MDCB cert-sync tracker — raw PEM strings would bloat the sync map with multi-KB keys and have no sync target. ✅
• Cache Set/Get is mutex-guarded — concurrent List() calls for the same PEM at worst duplicate one parse then converge. ✅

Code Quality Issues

All other quality checklist items pass: errors use WithError(err) via logrus (consistent with codebase style), new constant is documented, no goroutine leaks, no fmt.Println.

Tyk Domain Issues

What Was Done Well

• Comprehensive unit tests with idiomatic subtests: 10 t.Run subtests cover every meaningful variant — happy path, combined cert+key PEM, mixed batch (PEM + cert ID + file path), whitespace tolerance, CRLF normalization, malformed input, truncated input, content-hash caching, multi-cert chain, and CertPool integration. This is above the bar for new caching/parsing code.
• Content-addressed caching design: SHA256 of PEM content as cache key deduplicates shared embedded certs across multiple API definitions using the same material. Correct and efficient.
• Proactive safety documentation in the PR description: the author explicitly addresses detection ambiguity, private-key logging safety, cache-key collision analysis, and concurrency — before being asked. This is the standard.
• Correct exclusion from MDCB cert usage tracker: embedded PEM strings carry no cert-store ID and would produce multi-KB unusable sync entries. Filtering them with IsPEMContent at the tracker boundary is exactly right.
• Backward-compatibility integration test: TestEmbeddedPEM_BackwardCompatibility uses a three-row table test — file path, cert ID, embedded PEM — that directly maps to all three acceptance criteria in a single test function.

Gap Summary

Verdict

NEEDS DISCUSSION

The implementation is clean and well-reasoned. The SentinelOne failure is entirely pre-existing and not a concern for this PR. The one open question before approving: confirm that cert_embedded_pem_test.go exercises at least one OAS-style API definition — the Jira acceptance criterion explicitly names "Tyk Classic and Tyk OAS." Because CertificateManager.List() is shared infrastructure, the risk of an actual functional gap is low, but OAS test coverage (or a clear note explaining why Classic coverage is sufficient here) is needed to mark criterion 1 fully met. If the author can confirm this — either by pointing to the relevant BuildAPI call or adding a single OAS-flavoured test variant — this can move straight to Approve.

Review by Quinn · Tyk QA · 2026-06-04

🚨 Jira Linter Failed

Commit: 3d34818
Failed at: 2026-06-11 12:22:37 UTC

The Jira linter failed to validate your PR. Please check the error details below:

failed to get Jira issue: failed to fetch Jira issue TT-17295: Issue does not exist or you do not have permission to see it.: request failed. Please analyze the request body for more details. Status code: 404

Next Steps

• Ensure your branch name contains a valid Jira ticket ID (e.g., ABC-123)
• Verify your PR title matches the branch's Jira ticket ID
• Check that the Jira ticket exists and is accessible

This comment will be automatically deleted once the linter passes.

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud