feat: add code-signing to PKI by carlosmonastyrski · Pull Request #5682 · Infisical/infisical

carlosmonastyrski · 2026-03-13T05:24:13Z

Context

Adds code signing support to the cert-manager project type, allowing users to create signers backed by certificates with the codeSigning EKU and an approval policy. Signers can be enabled/disabled, and all signing operations require an active approval grant (supporting manual, time-window, and count-limited approval modes).

Screenshots

Steps to verify the change

Type

Checklist

Title follows the conventional commit format: type(scope): short description (scope is optional, e.g., fix: prevent crash on sync or fix(api): handle null response).
Tested locally
Updated docs (if needed)
Updated CLAUDE.md files (if needed)
Read the contributing guide

linear · 2026-03-13T05:24:16Z

PKI-149 Base platform work for code signing

maidul98 · 2026-03-13T05:24:53Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

greptile-apps · 2026-03-13T05:31:53Z

Greptile Summary

This PR introduces a complete code-signing subsystem for the cert-manager project type, including a Signers entity backed by a codeSigning EKU certificate, an approval-grant flow (manual, time-window, and n-signings modes), signing operation audit records, and corresponding frontend pages. The design is well-structured and integrates cleanly with the existing approval-policy factory pattern.

Key issues found:

Race condition in maxSignings enforcement (signer-service.ts:sign): Under PostgreSQL's default READ COMMITTED isolation, two concurrent requests sharing the same n-signings=1 (or Manual mode) grant can both read usedCount = 0 before either transaction commits, allowing more signings than the grant permits. A SELECT ... FOR UPDATE lock on the grant row within the transaction is needed to make the count check atomic.
Actor name lost on user deletion (signing-operation-dal.ts): The actorName column in signing_operations is always NULL at write time; names are resolved via joins at read time. If a user or identity is deleted, all their historical signing records permanently lose their actor name, breaking audit log immutability. The name should be captured at signing time.
Generic permission action names (project-permission.ts): The new ProjectPermissionCodeSigningActions enum uses generic names (Read, Create, Edit, Delete) alongside the well-named Sign action — consider more descriptive subject-specific names.
User-controlled clientMetadata.ip: The IP field in signing request metadata is entirely self-reported by the client and unverified, which could mislead security investigations.
No documentation in /docs: The code signing feature has no corresponding documentation in the /docs folder. How will customers discover this feature?

Confidence Score: 2/5

Not safe to merge — a concurrency race condition can allow a single-use approval grant to be used more than once under concurrent signing requests.
The TOCTOU race in sign() is a real security bypass: an n-signings=1 or Manual grant can be consumed multiple times if two requests arrive simultaneously, circumventing the core governance guarantee of the approval flow. The actor-name issue also undermines audit immutability. These should be addressed before merge.
backend/src/services/signer/signer-service.ts (race condition in sign), backend/src/services/signer/signing-operation-dal.ts (actorName not captured at write time)

Important Files Changed

Filename	Overview
backend/src/services/signer/signer-service.ts	Core signing service — contains a TOCTOU race condition in maxSignings enforcement and does not capture actorName at write time, which breaks audit log immutability on user deletion.
backend/src/db/migrations/20260309141236_code-signing-tables.ts	Idempotent migration using hasTable/hasColumn guards; creates Signers and SigningOperations tables and extends approval_request_grants with granteeMachineIdentityId. Down migration is correctly ordered.
backend/src/services/signer/signing-operation-dal.ts	Signing operation DAL with multi-table joins to resolve actor names at query time; actorName is never persisted at write time meaning it's lost on user/identity deletion.
backend/src/services/approval-policy/code-signing/code-signing-policy-factory.ts	Code signing approval policy factory implementing canAccess, validateConstraints, and postApprovalRoutine; canAccess parameter named userId is used for both user and identity grant lookups which is slightly misleading but functionally correct.
backend/src/server/routes/v1/signer-router.ts	Well-structured router with audit logging for all endpoints; data size validation in body schema matches the 128-byte service limit; clientMetadata.ip is user-controlled.
backend/src/ee/services/permission/project-permission.ts	Adds ProjectPermissionCodeSigningActions enum and CodeSigners subject; uses generic Read/Create/Edit/Delete action names alongside the specific Sign action.
backend/src/services/approval-policy/approval-policy-service.ts	Extends createRequest to support machine identity requesters by conditionally setting requesterUserId vs machineIdentityId; machineIdentityId comes from authenticated token not body.
backend/src/services/certificate/certificate-dal.ts	Adds extendedKeyUsage filter using parameterized whereRaw for safe PostgreSQL array containment check; properly applied to both count and list queries.

Comments Outside Diff (2)

backend/src/services/signer/signer-service.ts, line 613-664 (link)

TOCTOU race condition in maxSignings enforcement

Under PostgreSQL's default READ COMMITTED isolation level, two concurrent signing requests sharing the same grant (e.g., maxSignings: 1) can both bypass the limit:
1. Request A reads usedCount = 0 — proceeds
2. Request B reads usedCount = 0 (before A's transaction commits) — also proceeds
3. Both create a signing operation, resulting in two successful signings against a one-time grant
The countByGrantId check and the subsequent signingOperationDAL.create are not atomic at this isolation level.

Fix: Add SELECT ... FOR UPDATE on the grant row inside the transaction to pessimistically lock it before checking the count:
```
// After finding matchingGrant, lock it before the count check:
const [lockedGrant] = await (tx)(TableName.ApprovalRequestGrants)
  .where({ id: matchingGrant.id })
  .forUpdate();

if (!lockedGrant || lockedGrant.status !== ApprovalRequestGrantStatus.Active) {
  throw new ForbiddenRequestError({ message: "Grant is no longer active." });
}

// Now safe to count and sign
const usedCount = await signingOperationDAL.countByGrantId(matchingGrant.id, tx);
if (attrs.maxSignings && usedCount >= attrs.maxSignings) {
  await approvalRequestGrantsDAL.updateById(matchingGrant.id, { status: ApprovalRequestGrantStatus.Expired }, tx);
  throw new ForbiddenRequestError({ message: "Signing grant has been exhausted." });
}
```
This ensures only one transaction at a time can proceed past the count check for a given grant, eliminating the race window for NSignings and Manual (which sets maxSignings: 1) approval modes.
backend/src/ee/services/permission/project-permission.ts, line 395-401 (link)

Generic permission action names

Per the project's convention for new permission subjects, actions like Read, Create, Edit, and Delete are generic. Consider more descriptive, subject-specific action names that make it clearer what they govern. For example:

Sign is already well-named and specific. Making the others subject-specific (e.g., ReadSigner) improves clarity when these permissions appear in logs, role-definition UI, and policy configs.

Rule Used: When using parameters in API calls, ensure they ar... (source)

Learnt From
Infisical/infisical#3643

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

_{Last reviewed commit: 7bf1275}

backend/src/services/signer/signing-operation-dal.ts

backend/src/server/routes/v1/signer-router.ts

backend/src/@types/fastify.d.ts

backend/src/db/schemas/models.ts

backend/src/db/migrations/20260309141236_code-signing-tables.ts

backend/src/ee/services/audit-log/audit-log-types.ts

backend/src/server/routes/v1/signer-router.ts

backend/src/services/approval-policy/code-signing/code-signing-policy-factory.ts

backend/src/services/approval-policy/code-signing/code-signing-policy-schemas.ts

backend/src/services/approval-policy/approval-policy-enums.ts

backend/src/services/signer/signer-service.ts

backend/src/services/signer/signing-operation-dal.ts

frontend/src/pages/project/RoleDetailsBySlugPage/components/ProjectRoleModifySection.utils.tsx

frontend/src/pages/cert-manager/SignerDetailPage/SignerDetailPage.tsx

frontend/src/pages/cert-manager/SignerDetailPage/components/SigningOperationsTable.tsx

frontend/src/pages/cert-manager/SignerDetailPage/components/SignerOverviewSection.tsx

frontend/src/pages/cert-manager/CodeSigningPage/components/CreateSignerModal.tsx

...manager/ApprovalsPage/components/CodeSigningPolicyTab/components/CodeSigningPolicySchema.tsx

backend/src/db/migrations/20260309141236_code-signing-tables.ts

backend/src/lib/api-docs/constants.ts

backend/src/lib/crypto/sign/signing.ts

backend/src/server/routes/v1/approval-policy-routers/index.ts

gitguardian · 2026-03-17T17:47:24Z

⚠️ GitGuardian has uncovered 1 secret following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secret in your pull request

GitGuardian id	GitGuardian status	Secret	Commit	Filename
27411356	Triggered	Generic Database Assignment	`8f6c6bc`	backend/src/ee/services/pam-discovery/active-directory/active-directory-discovery-factory.ts	View secret

🛠 Guidelines to remediate hardcoded secrets

Understand the implications of revoking this secret by investigating where it is used in your code.
Replace and store your secret safely. Learn here the best practices.
Revoke and rotate this secret.
If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider

following these best practices for managing and storing secrets including API keys and other credentials
install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.

^{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.}

backend/src/server/routes/v1/approval-policy-routers/index.ts

frontend/src/pages/cert-manager/SignerDetailPage/components/SignerOverviewSection.tsx

docs/documentation/platform/pki/code-signing/overview.mdx

docs/documentation/platform/pki/code-signing/signing-policies.mdx

docs/documentation/platform/pki/code-signing/signing-requests.mdx

backend/src/@types/fastify.d.ts

Add code-signing to PKI

7bf1275

greptile-apps bot reviewed Mar 13, 2026

View reviewed changes

backend/src/services/signer/signing-operation-dal.ts Show resolved Hide resolved

backend/src/server/routes/v1/signer-router.ts Show resolved Hide resolved

carlosmonastyrski added 2 commits March 13, 2026 03:28

Address greptile comments

e037c41

Lint fixes

9d85ca2

carlosmonastyrski requested a review from sheensantoscapadngan March 13, 2026 14:41

Add code signing docs

e786346

sheensantoscapadngan reviewed Mar 14, 2026

View reviewed changes

backend/src/@types/fastify.d.ts Outdated Show resolved Hide resolved

sheensantoscapadngan reviewed Mar 14, 2026

View reviewed changes

backend/src/db/schemas/models.ts Outdated Show resolved Hide resolved

sheensantoscapadngan reviewed Mar 14, 2026

View reviewed changes

backend/src/db/migrations/20260309141236_code-signing-tables.ts Show resolved Hide resolved

sheensantoscapadngan reviewed Mar 14, 2026

View reviewed changes

backend/src/ee/services/audit-log/audit-log-types.ts Outdated Show resolved Hide resolved

sheensantoscapadngan reviewed Mar 14, 2026

View reviewed changes

backend/src/server/routes/v1/signer-router.ts Show resolved Hide resolved