Jwt runner auth 2 by fiftin · Pull Request #3978 · semaphoreui/semaphore

fiftin · 2026-06-18T11:10:03Z

No description provided.

This commit includes the bare minimum implementation to issue a JWT per task execution. There is room for improvement like the bound claims should be reviewed or the subject in the token. (does it make sense to use "task:%d"? Is that a unique value over all projects?)

cursor

Stale comment

Security review — PR #3978 (JWT runner auth)

Outcome: No medium, high, or critical vulnerabilities identified in the added/modified code.

Reviewed the JWT issuance flow end-to-end: template jwt_params validation, signer initialization/key storage, runner job delivery, local task execution (SEMAPHORE_JWT), and the public JWKS endpoint. Prior automation threads were not present on this PR.

Areas reviewed

Authn/authz: JWTs are minted only when the global signer is initialized (jwt_enabled) and the template has jwt_params.enabled. Runners still require X-Runner-Token; template changes require CanManageProjectResources. No unauthenticated minting path found.

Input validation: TTL is bounded at save time (TemplateJWTParams.Validate) and again at sign time (Signer.Sign clamps to MaxTTL). Audience size is capped (32 entries).

Key handling: Signing key is persisted via existing AccessKeyEncryption machinery (same pattern as access keys). JWKS exposes only the public key, gated on jwt_enabled.

Dependencies: go-jose/v4 is pinned to v4.1.4 (includes the JWE panic fix); this PR uses JWS signing only, not JWE decryption of untrusted input.

Secret delivery: JWT is delivered to runners/tasks alongside existing job secrets; no new unauthenticated exposure beyond the established runner API contract.

Residual notes (not reported as findings)

These are operational/design considerations rather than exploitable defects in this diff:

Project managers can configure JWT aud values for their templates; downstream services must enforce their own authorization (e.g., validate aud, iss, and scope claims such as project_id).

When access_key_encryption is unset, the signing key is stored with the same base64-only encoding used for other secrets — DB/admin access remains the control, consistent with pre-existing behavior.

No inline finding comments were added.

_{Sent by Cursor Automation: Find vulnerabilities}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1e4fb26fff

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-06-18T11:16:04Z

 	encryptionService server.AccessKeyEncryptionService,
 	keyInstallationService server.AccessKeyInstallationService,
 	logWriteService pro_interfaces.LogWriteService,
+	signer jwt.Signer,


Keep CreateTaskPool call sites compiling

Adding this required parameter leaves existing call sites unupdated: services/tasks/TaskRunner_test.go and api/cluster_test.go still call CreateTaskPool with seven arguments. Those packages will fail to compile under go test before any tests run, so either update the callers to pass the signer or avoid making the constructor signature incompatible.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-06-18T11:16:04Z

+	if err := store.SetOption(jwtSigningKeyOption, encrypted); err != nil {
+		return nil, fmt.Errorf("persist signing key: %w", err)
+	}


Widen option storage before saving signing keys

When JWT is enabled on SQL-backed installs, this persists the generated PEM through the existing option.value column, which is defined as varchar(255) in the SQL migrations. Even an unencrypted base64-encoded P-256 private-key PEM is over 300 bytes, and AES-GCM adds more, so first startup with JWT enabled will fail or store a truncated key on MySQL/Postgres; the key needs a larger column or a different storage path before calling SetOption here.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-06-18T11:16:04Z

+  jwt_enabled:
+    type: boolean
+    description: >
+      When true, Semaphore mints a short-lived JWT for each task run and exposes
+      its public key via /.well-known/jwks.json.
+  jwt_issuer:
+    type: string
+    description: Value emitted in the `iss` claim of issued JWTs.
+  jwt_default_ttl:


Document JWT config under the actual jwt object

These schema properties are top-level jwt_enabled/jwt_issuer keys, but the new config field is JWT *JWTConfig with JSON name jwt, so the runtime expects jwt: { enabled, issuer, default_ttl, max_ttl }. Users following this schema can create a validating config that is silently ignored by ConfigInit, leaving JWT disabled and the UI hidden.

Useful? React with 👍 / 👎.

cursor

Stale comment

Security review — PR #3978 (JWT runner auth)

Outcome: No medium, high, or critical vulnerabilities identified in the added/modified code.

Re-reviewed the synchronized PR diff end-to-end and validated prior automation findings; no unresolved security threads remain.

Areas reviewed

Authn/authz: JWTs are minted only when the global signer is initialized (jwt.enabled) and a template has jwt_params.enabled. Runners still require X-Runner-Token; template changes require CanManageProjectResources. No unauthenticated minting path found.

Input validation: TTL is bounded at save time (TemplateJWTParams.Validate) and again at sign time (Signer.Sign clamps to MaxTTL). Audience size is capped (32 entries).

Key handling: Signing key is persisted via existing AccessKeyEncryption machinery. JWKS exposes only the public key, gated on jwt.enabled.

Dependencies: go-jose/v4 v4.1.4 is used for JWS signing only; no untrusted JWE/JWS verification path was added.

Secret delivery: JWT is delivered to runners/tasks alongside existing job secrets under the established runner API contract.

Residual notes (not reported as findings)

Project managers can configure JWT aud values for their templates; downstream services must enforce their own authorization (validate aud, iss, and scope claims such as project_id).

When access_key_encryption is unset, the signing key uses the same base64-only encoding as other secrets — DB/admin access remains the control, consistent with pre-existing behavior.

The option.value varchar(255) column may be too small for the encrypted signing key on SQL backends; this is a reliability/deployment concern, not an attacker-controlled exploit path.

No inline finding comments were added.

Slack summary: PR #3978 security review complete — no medium/high/critical vulnerabilities found. JWT issuance, key storage, runner delivery, and JWKS endpoint were reviewed; controls (runner auth, template validation, TTL clamping, encrypted key storage) appear sufficient. Residual risk is operational: downstream services must validate JWT claims, and SQL option column sizing may block key persistence.

_{Sent by Cursor Automation: Find vulnerabilities}

cursor

Security review — PR #3978 (JWT runner auth)

Outcome: No medium, high, or critical vulnerabilities identified in the added/modified code.

Re-reviewed the synchronized PR diff end-to-end and validated prior automation assessments; no unresolved Cursor security finding threads remain.

Areas reviewed

Authn/authz: JWTs are minted only when the global signer is initialized (jwt.enabled) and a template has jwt_params.enabled. Runners still require X-Runner-Token; template changes require CanManageProjectResources. No unauthenticated minting path found.
Input validation: TTL is bounded at save time (TemplateJWTParams.Validate) and again at sign time (Signer.Sign clamps to MaxTTL). Audience size is capped (32 entries).
Key handling: Signing key is persisted via existing AccessKeyEncryption machinery. JWKS exposes only the public key, gated on jwt.enabled.
Dependencies: go-jose/v4 v4.1.4 is used for JWS signing only; no untrusted JWE/JWS verification path was added.
Secret delivery: JWT is delivered to runners/tasks alongside existing job secrets under the established runner API contract.

Residual notes (not reported as findings)

Project managers can configure JWT aud values for their templates; downstream services must enforce their own authorization (validate aud, iss, and scope claims such as project_id).
When access_key_encryption is unset, the signing key uses the same base64-only encoding as other secrets — DB/admin access remains the control, consistent with pre-existing behavior.

No inline finding comments were added.

Slack summary: PR #3978 security review complete — no medium/high/critical vulnerabilities found. JWT issuance, key storage, runner delivery, and JWKS endpoint were reviewed; controls (runner auth, template validation, TTL clamping, encrypted key storage) appear sufficient. Residual risk is operational: downstream services must validate JWT claims.

_{Sent by Cursor Automation: Find vulnerabilities}

jon4hz and others added 19 commits May 27, 2026 14:56

feat(jwt): store per jwt options per template in db

40de7b0

feat(jwt): multiple aud and ttl per template

095421f

feat(jwt): jwt options in task template page

272d73a

fix(jwt): only IDs in claims

4ff3a9d

feat(jwt): improve wording, use ecdsa instead of rsa

e818991

chore(jwt): add some unit tests

83c7100

refactor(jwt): move duplicated crypto functions to utils

9ee2759

chore(jwt): dont rename imports

7d944e3

chore(jwt): revert changes to package-lock.json

b6cfb2b

chore(jwt): update config schema

e0a8b3c

chore(jwt): simple option key

f0c6481

chore(jwt): revert unwanted formatting changes in config schema

64e6036

refactor(jwt): change config structure

adbde92

feat(templates): use dropdown card for jwt and schedule

a95ac48

test(jwt): fix config

14f5d72

refactor(jwt): remove global variable

5f644c0

Update devcontainer.json

5fb0e5d

refactor(jwt): remove global variable

1e4fb26

cursor Bot reviewed Jun 18, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Jun 18, 2026

View reviewed changes

fix: merge conflict

bbacd8b

cursor Bot reviewed Jun 18, 2026

View reviewed changes

agents(jwt): add plan for new encryption key configs

7d6c0d1

cursor Bot reviewed Jun 18, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Jwt runner auth 2#3978

Jwt runner auth 2#3978
fiftin wants to merge 21 commits into
developfrom
jwt-runner-auth-2

fiftin commented Jun 18, 2026

Uh oh!

cursor Bot left a comment •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jun 18, 2026

Uh oh!

chatgpt-codex-connector Bot Jun 18, 2026

Uh oh!

chatgpt-codex-connector Bot Jun 18, 2026

Uh oh!

cursor Bot left a comment •

edited

Loading

Uh oh!

cursor Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

fiftin commented Jun 18, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Security review — PR #3978 (JWT runner auth)

Areas reviewed

Residual notes (not reported as findings)

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Security review — PR #3978 (JWT runner auth)

Areas reviewed

Residual notes (not reported as findings)

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Security review — PR #3978 (JWT runner auth)

Areas reviewed

Residual notes (not reported as findings)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading