Android cert SAN frontend by getvictor · Pull Request #44809 · fleetdm/fleet

getvictor · 2026-05-06T01:12:03Z

Related issue: Resolves #41472

Figma
Needs to merge to main after Add support for SAN in Android certificate templates. #44690

Checklist for submitter

Input data is properly validated, SELECT * is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.

Testing

Added/updated automated tests
QA'd all new/changed functionality manually

Summary by CodeRabbit

New Features
- Added Subject Alternative Name (SAN) field to the certificate management modal with comprehensive validation support.
Tests
- Expanded test coverage for the certificate modal with enhanced validation scenarios and server response handling.

- End-to-end implementation for `subject_alternative_name` in certificate templates. - Includes schema migration, variable expansion, frontend UI updates, GitOps support, and Android agent integration. - Limits SAN types to `DNS`, `EMAIL`, `UPN`, `IP`, and `URI`.

- Updated schema to use `TEXT` for `subject_alternative_name`, matching existing

Adds the optional subject_alternative_name field end to end on the server side: REST API request and response, persistence, GitOps apply and generate-gitops emit, per-host variable expansion, lightweight server-side validation, and Premium gating. The Android agent and frontend ship in separate PRs (per the OpenSpec migration plan). Changes: - Type changes on CertificateTemplate, CertificateRequestSpec, CertificateTemplateSpec and the response structs. - Migration adds a nullable TEXT column to certificate_templates, matching subject_name. schema.sql updated. - Datastore CRUD reads via COALESCE so absent SAN deserialises to ""; writes normalise whitespace-only input to NULL while preserving non-empty values verbatim for GitOps idempotency. - Service layer validates token shape, KEY allow-list (DNS, EMAIL, UPN, IP, URI), variable allow-list, and length cap (4096 bytes); rejects with BadRequestError. Premium check fires only on SAN-bearing payloads. - GetDeviceCertificateTemplate expands $FLEET_VAR_HOST_* in SAN with the same failure semantics as subject_name. - generate-gitops emits subject_alternative_name when non-empty; the fleetctl client propagates SAN through GitOps apply, including detecting SAN changes for re-creation. - Unit tests cover validation table, Premium gating, format failures, and variable allow-list. The generate-gitops compareDirs test now round-trips a SAN-bearing template. OpenSpec: openspec/changes/android-cert-san-attributes/

Verifies the end-to-end path that the Android Fleet agent will exercise: the fleetd certificate API expands $FLEET_VAR_HOST_END_USER_IDP_USERNAME inside both subject_name and subject_alternative_name when the host has an associated IdP account, and returns the rendered template ready for the agent to wrap in a CSR.

…ogic. - Added stricter validation for `subject_alternative_name`, including disallowing empty values, separator-only inputs, and malformed tokens. - Updated schema to enforce UTF-8 compliance for `subject_alternative_name` field. - Clarified Premium gating behavior for SAN-bearing templates. - Fixed propagation of challenges when setting cert status to "failed." - Aligned validation checks and migrations with spec updates.

# Conflicts: # server/service/integration_enterprise_test.go

getvictor · 2026-05-06T01:12:19Z

@coderabbitai full review

coderabbitai · 2026-05-06T01:12:23Z

✅ Actions performed

Full review triggered.

getvictor · 2026-05-06T01:12:24Z

/agentic_review

qodo-free-for-open-source-projects · 2026-05-06T01:12:29Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. ~~SAN trim/limit mismatch~~ ✓ Resolved 🐞 Bug ≡ Correctness

Description

The SAN max-length validation checks the raw subjectAlternativeName.length, but the API client
trims SAN and omits it when the trimmed value is empty. This can block form submission for SAN
values that would be trimmed down (or omitted) and accepted by the server (e.g., long
leading/trailing whitespace).

Code

frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertificateModal/helpers.ts[R110-121]

+    subjectAlternativeName: {
+      // SAN is optional; format is validated server-side. Only enforce the
+      // server's length cap so the user gets fast feedback on pathological inputs.
+      validations: [
+        {
+          name: "maxLength",
+          isValid: (formData: IAddCertFormData) => {
+            return formData.subjectAlternativeName.length <= SAN_MAX_LENGTH;
+          },
+          message: SAN_TOO_LONG_MSG,
+        },
+      ],

Evidence
Client-side validation rejects SAN based on the untrimmed length, while the request builder trims
the SAN and conditionally excludes it when empty; this creates inconsistent behavior where
whitespace can make the UI reject an input that would not be sent (or would be shorter) on the wire.
frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertificateModal/helpers.ts[110-121]
frontend/services/entities/certificates.ts[138-147]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The SAN field is validated against a 4096-character cap using the raw textarea value, but the API client trims SAN (and omits it entirely when the trimmed value is empty). This mismatch can cause the UI to block submissions that would result in an empty/short SAN on the request.
### Issue Context
- `validateFormData` enforces `subjectAlternativeName.length <= 4096`.
- `certificatesAPI.addCert` uses `subjectAlternativeName?.trim()` and omits `subject_alternative_name` when the trimmed string is empty.
### Fix Focus Areas
- frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertificateModal/helpers.ts[110-121]
- frontend/services/entities/certificates.ts[138-147]
### Implementation notes
- Update the SAN max-length validation to apply to the same representation that will be sent (e.g., `formData.subjectAlternativeName.trim().length <= 4096`).
- Optionally, consider treating whitespace-only SAN as empty/valid since it will be omitted from the request.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

codecov · 2026-05-06T01:14:55Z

Codecov Report

❌ Patch coverage is 90.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.70%. Comparing base (c79d33a) to head (a8f136a).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...onents/AddCertificateModal/AddCertificateModal.tsx	88.88%	2 Missing ⚠️
...ificates/components/AddCertificateModal/helpers.ts	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #44809      +/-   ##
==========================================
+ Coverage   66.66%   66.70%   +0.04%     
==========================================
  Files        2672     2673       +1     
  Lines      214700   215049     +349     
  Branches     9946     9800     -146     
==========================================
+ Hits       143137   143458     +321     
- Misses      58540    58563      +23     
- Partials    13023    13028       +5

Flag	Coverage Δ
frontend	`54.41% <90.00%> (+0.03%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 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.

Copilot

Pull request overview

Adds frontend support for Android certificate template SANs in the Manage Controls certificates flow, aligning the UI with the backend/API support for subject_alternative_name.

Changes:

Adds an optional SAN textarea to the Add certificate modal and sends SAN data in certificate create requests.
Refactors modal validation so required-field errors appear inline after submit attempts and SAN-specific 422 errors render on the SAN field.
Expands modal tests and marks the related OpenSpec frontend tasks complete.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`openspec/changes/android-cert-san-attributes/tasks.md`	Marks the frontend OpenSpec tasks as completed with implementation notes.
`frontend/services/entities/certificates.ts`	Extends certificate types and POST payload construction to include SAN.
`frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertificateModal/helpers.ts`	Adds SAN validation plus required-field messages and submit-attempt-aware validation behavior.
`frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertificateModal/AddCertificateModal.tsx`	Adds the SAN field, SAN server-error handling, and the always-enabled submit pattern.
`frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertificateModal/AddCertificateModal.tests.tsx`	Adds SAN and validation-focused Jest coverage for the modal.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

coderabbitai · 2026-05-06T01:19:33Z

Walkthrough

This PR adds Subject Alternative Name (SAN) field support to the AddCertificateModal component for Android certificate management. Changes include adding a SAN input field with client-side and server-side validation, extending form data interfaces to include the SAN property, implementing validation gating with an attemptedSubmit flag, handling SAN-specific server errors, and comprehensive test coverage for new functionality including placeholder presence, validation errors, length limits, and full submission flows.

Possibly related PRs

fleetdm/fleet#37765: Both PRs modify the AddCertificateModal form validation logic in helpers.ts, with the former adding SAN support and expanding validation/required-field behavior.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The pull request description is incomplete and missing critical checklist items required by the repository template.	Complete the checklist by checking or removing irrelevant items. Specifically add: changes file confirmation, database migration checks (schema/collation/timestamps), manual QA confirmation, and clarify any new Fleet configuration settings or GitOps requirements.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR implements frontend SAN support across the AddCertificateModal component, helpers, and certificate service entities, aligning with the issue's UI design and API contract requirements for SAN field inclusion.
Out of Scope Changes check	✅ Passed	All code changes are directly related to implementing SAN support in the certificate modal and form validation, with no unrelated modifications or scope creep detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check	✅ Passed	The title 'Android cert SAN frontend' directly reflects the main change: adding Subject Alternative Name (SAN) support to the certificate modal frontend component for Android.

_{✏️ 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 victor/41472-phase2

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

claude

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

_{Tip: disable this comment in your organization's Code Review settings.}

# Conflicts: # server/datastore/mysql/schema.sql # server/service/certificates.go # server/service/client.go # server/service/integration_enterprise_test.go

getvictor added 19 commits May 4, 2026 13:51

Refine SAN handling in Android certificate templates.

30480bc

- Updated schema to use `TEXT` for `subject_alternative_name`, matching existing

Merge remote-tracking branch 'origin/main' into victor/41472-phase1

608ee5d

Updated schema.sql

87f40cd

Review fixes

3e89805

Fix cloner

1e94550

Updated tasks.md

8809a6a

Cleaning up errors.

3aca1d5

Fixing issues.

9e66b8b

Fixing issues.

849eb73

Final fixes.

d47e819

Added changes

6fc61cf

Final fixes?

b3cba3c

Merge remote-tracking branch 'origin/main' into victor/41472-phase1

acdaaaa

# Conflicts: # server/service/integration_enterprise_test.go

Final fixes?

5b16bff

SAN frontend

e49a54d

getvictor temporarily deployed to Docker Hub May 6, 2026 01:12 — with GitHub Actions Inactive

getvictor requested a review from Copilot May 6, 2026 01:12

Copilot started reviewing on behalf of getvictor May 6, 2026 01:13 View session

Copilot AI reviewed May 6, 2026

View reviewed changes

Code review fixes.

04e73dd

getvictor temporarily deployed to Docker Hub May 6, 2026 01:41 — with GitHub Actions Inactive

Code review fixes.

1261999

getvictor temporarily deployed to Docker Hub May 6, 2026 16:39 — with GitHub Actions Inactive

Code review fixes.

07abdae

getvictor temporarily deployed to Docker Hub May 6, 2026 16:57 — with GitHub Actions Inactive

getvictor marked this pull request as ready for review May 6, 2026 17:05

getvictor requested a review from a team as a code owner May 6, 2026 17:05

claude Bot reviewed May 6, 2026

View reviewed changes

getvictor changed the title ~~SAN frontend~~ Android cert SAN frontend May 6, 2026

Base automatically changed from victor/41472-phase1 to main May 6, 2026 19:43

getvictor requested review from a team and lukeheath as code owners May 6, 2026 19:43

Merge remote-tracking branch 'origin/main' into victor/41472-phase2

a8f136a

# Conflicts: # server/datastore/mysql/schema.sql # server/service/certificates.go # server/service/client.go # server/service/integration_enterprise_test.go

getvictor temporarily deployed to Docker Hub May 6, 2026 20:00 — with GitHub Actions Inactive

getvictor assigned lukeheath May 6, 2026

lukeheath approved these changes May 7, 2026

View reviewed changes

getvictor merged commit d9b17cf into main May 7, 2026
24 checks passed

getvictor deleted the victor/41472-phase2 branch May 7, 2026 18:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Android cert SAN frontend#44809

Android cert SAN frontend#44809
getvictor merged 23 commits into
mainfrom
victor/41472-phase2

getvictor commented May 6, 2026 •

edited

Loading

Uh oh!

getvictor commented May 6, 2026

Uh oh!

coderabbitai Bot commented May 6, 2026

Uh oh!

getvictor commented May 6, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented May 6, 2026 •

edited

Loading

Uh oh!

codecov Bot commented May 6, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot commented May 6, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

getvictor commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist for submitter

Testing

Summary by CodeRabbit

Uh oh!

getvictor commented May 6, 2026

Uh oh!

coderabbitai Bot commented May 6, 2026

Uh oh!

getvictor commented May 6, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

codecov Bot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Possibly related PRs

❌ Failed checks (1 warning)

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Claude Code Review

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

getvictor commented May 6, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented May 6, 2026 •

edited

Loading

codecov Bot commented May 6, 2026 •

edited

Loading

coderabbitai Bot commented May 6, 2026 •

edited

Loading