fix(jwt): name the issuing instance in every signed token

[geovannewashington]

What

Every JWT ShellHub signs now names the instance that signed it, derived from SHELLHUB_DOMAIN and SHELLHUB_AUTO_SSL.

Why

iss was empty on user and device tokens (both carried a TODO asking how to get the value) and absent entirely on enrollment callback and SSH web session tokens. It costs nothing while one system mints tokens and one key signs them, and becomes ambiguous as soon as there is a second issuer, which is what embedding Dex brings. Doing it now is the cheap ordering: every token in circulation is blank, so whatever validation lands later must tolerate blanks, and the sooner we stop minting them the shorter that window stays open.

Changes

• The three jwttoken encoders take an issuer and write it. EncodeEnrollmentDecisionClaims gains the field it never had.
• The SSH web session signer takes it too, and its dead *rsa.PrivateKey parameter is dropped.
• The service receives it through a new WithIssuer option, set once in Setup. An option rather than a constructor argument because that constructor has 1 production call site against 129 in tests.
• The scheme rule the SSH approval banner already used is exported as session.ConsoleURL, so there is one copy of it rather than two.
• The decode path is untouched, so tokens already issued keep working. Turning on jwt.WithIssuer would reject all of them, so it needs a transition window and its own issue.

Testing

• TestEncodedClaimsCarryTheIssuer covers all three encoders, TestNewTokenCarriesTheIssuer the SSH web signer, TestInstanceIssuer the scheme decision and the wiring.
• The device-auth tests build the service with the same issuer they re-encode with, so broken wiring fails their exact-string comparison.
• go build, gofmt -s -l and golangci-lint run are clean. ./api/... ./app/... ./ssh/... pass except TestInstallScriptRendersAValidShellScript, which fails identically on master (it cannot find install.sh from the container's working directory).

Two limitations, both recorded in the commit message rather than fixed here. The scheme is derived from SHELLHUB_AUTO_SSL, which describes the bundled gateway, so an instance terminating TLS upstream is named http while served over https; the claim is unread until issuer validation exists, which is when the value has to be right. And the SSH web session token's signed form never leaves the process today, so that claim is groundwork.

Closes #7111

[github-actions]

Claude finished @geovannewashington's task in 10m 12s —— View job

---

Code Review Complete

Reviewed 14 files. 4 inline issues posted. Standards: 0 violations, 2 judgement calls. Spec: 1. Correctness: 1.

Standards

No documented rule is broken. The worst judgement call is NewToken at server/ssh/web/pkg/token/token.go:23-25, which keeps a *rsa.PrivateKey parameter it never reads while the branch rewrites that very signature and its doc comment — and the new test passes nil for it, pinning the deadness as a tested fact.

Correctness

No security, access-control, concurrency or context defects. The one finding is that the iss added at server/ssh/web/pkg/token/token.go:30 is unobservable: the handler returns token.ID, not the signed Data, so that half of the PR fills a claim nothing can read.

Cross-repo check is clean. cloud/ imports neither pkg/api/jwttoken nor server/ssh/web, so none of the four changed signatures has a call site there, and cloud inherits the issuer through the core Setup.

Spec

• Partial — "SHELLHUB_DOMAIN is a bare hostname, so the scheme has to come from somewhere: https in production, http on a dev stack." issuerURL keys off AutoSSL, which reports whether the bundled gateway terminates TLS itself. An instance terminating TLS upstream mints iss: "http://<domain>" while reachable only over HTTPS.
• Met — all three encoders set the issuer, including EncodeEnrollmentDecisionClaims, which had no Issuer field at all.
• Met — "Leave validation out." pkg/api/jwttoken/encoder.go is untouched; decodeClaims still parses with only jwt.WithValidMethods, so tokens already in circulation keep working.
• Met — the issuer derives from SHELLHUB_DOMAIN, and localhost in development yields http://localhost.
• Met — configuration reaches the signing package through WithIssuer and web.Config.Issuer; jwttoken still sees no configuration.

Additional notes

• The SSH web session token is a fourth signing site the issue's evidence section never names, so it sits outside "all three encoders." Judged a faithful reading of the issue's own opening claim about every token rather than scope creep — noted only so the difference is on the record.
• Three candidate findings were dropped in validation, not by a cap: the SHELLHUB_DOMAIN="" → iss: "http://" path (real, but consoleURL already degrades identically on master, so the branch doesn't introduce it); a Data Clumps argument for a Signer type over the three encoders (accurate but out of proportion to a targeted fix, and no such precedent exists in pkg/); and a spec finding about WithIssuer having no default (the quoted paragraph describes the difficulty, it doesn't require a guard). The last one has a residue worth a sentence: nothing covers the single append at server/app/server.go:196, so a refactor that drops it reverts the issue with every test still green. The repo already has the pattern for this — licenseEvaluatorOption was extracted from Setup precisely so TestLicenseEvaluatorWiring could reach it.
• No caps dropped anything.

---

If you've addressed the feedback and want a new review, tag @shellhub-io/admin and a team member can trigger it.
• Branch: fix/jwt-issuer-claim

[github-actions]

Documentation preview: https://6a6aeaa1.shellhub-docs.pages.dev

[otavio]

/review

[github-actions]

Claude finished @otavio's task in 7m 35s —— View job

---

Code Review Complete

Reviewed 16 files. 2 inline issues posted. Standards: 1 violation, 1 judgement call. Spec: 0 new. Correctness: 0 new.

Standards

The doc comment added to NewToken (server/ssh/web/pkg/token/token.go:23-24) states a false contract: it says the token is "signed with the SSH host key", but line 31 signs with magickey.GetReference() — a sync.OnceValue RSA key generated in-process at first use (server/ssh/pkg/magickey/magickey.go:14). The real host key is read from config.HostKeyFile at server/ssh/web/web.go:64 and never reaches this function. The comment was added to explain the dropped *rsa.PrivateKey parameter, which was the right instinct on the wrong key; it now tells a caller the signature verifies anywhere that key is held, when in fact only this process can verify it.

Correctness

No new defects. Verified directly: the only JWT signing sites in the repo are pkg/api/jwttoken/encoder.go:11 and server/ssh/web/pkg/token/token.go:28, and all four entry points now carry an issuer; s.issuer is assigned at server/app/server.go:198, before both services.NewService (201) and setupSSH (230), and setupSSH has no other caller, so it cannot be read before assignment; the decode path passes no jwt.WithIssuer and never reads .Issuer, so tokens already in circulation keep validating; omitting WithIssuer degrades to exactly the previous Issuer: ""; and authorizer.UserClaims/DeviceClaims carry no field tagged iss, so the newly-populated embedded RegisteredClaims.Issuer is not dropped by encoding/json's equal-depth conflict rule.

Cross-repo check is clean. cloud/ has no call site for jwttoken.*, token.NewToken, web.Config, ConsoleURL, or the core services.NewService — it reaches the core service through the CoreService interface and the Register* hooks, so no Cloud path mints a blank issuer.

Spec

Every requirement in issue #7111 is met, with one partial that is not new: instanceIssuer still takes the scheme from SHELLHUB_AUTO_SSL, so an instance terminating TLS upstream is named http://<domain> while served over HTTPS, against "https in production, http on a dev stack." That was the previous round's Spec finding; you answered it, corrected your own reasoning about the self-signed-certificate case, and recorded the limitation in the commit message. Deduplicated rather than re-raised.

Met: all three encoders set the issuer, EncodeEnrollmentDecisionClaims gaining the field it never had; validation stays out, with pkg/api/jwttoken/encoder.go untouched; the issuer derives from SHELLHUB_DOMAIN; localhost yields http://localhost; and the spec's central question — "how configuration reaches a low-level signing package" — is answered through WithIssuer and web.Config.Issuer, with jwttoken still seeing no configuration.

Additional notes

• Deduplicated against the previous round, beyond the Spec partial above: the dead *rsa.PrivateKey parameter (removed in aed7cbb1d), the unobservable iss on the web session token (recorded as groundwork in the commit message), and the duplicated scheme rule (now one copy, session.ConsoleURL). The Data Clumps argument for a Signer type over the three encoders surfaced again and was dropped again, for the same reason as last time.
• One judgement call was raised and dropped in validation: exporting consoleURL puts a server/app → server/ssh/session edge in place for a non-SSH purpose, and a neutral helper both packages call would avoid it. Dropped because the previous round explicitly recommended exporting one copy and deleting the other, and you did exactly that — re-litigating the seam now would be churn.
• No caps dropped anything.
• Nothing was committed; this run only read the branch.

---

If you've addressed the feedback and want a new review, tag @shellhub-io/admin and a team member can trigger it.
• Branch: fix/jwt-issuer-claim

[otavio]

/review

[github-actions]

Claude finished @otavio's task in 14m 17s —— View job

---

Code Review Complete

Reviewed 16 files on all three axes: Standards (repo rules, Go conventions, tests, code smells), Spec (does it implement issue #7111?) and Correctness (security and runtime defects). No new issues found, and no inline comments were posted. The code looks good as-is.

Standards

No violations. Every comment the diff adds is a doc comment on an exported declaration and each states a contract rather than restating a signature — including NewToken's, which now names magickey.GetReference correctly, and Config's, which now covers both fields. Both // TODO: how can we get the correct issuer? comments are gone. The three things with nowhere in the code to live all reached the commit message in full: the upstream-TLS scheme limitation, why decoding still ignores iss, and why the issuer is an Option rather than a constructor argument.

Correctness

Verified and clean. issuer sits in the same position in all three encoder signatures and every one of the four production call sites passes it correctly; jwt.NewWithClaims/SignedString appear in exactly two non-test places repo-wide, both covered. s.env is assigned in app.go before Setup, so both instanceIssuer() calls are safe, and WithIssuer is appended last so no later option overwrites it. No JSON tag collision — UserClaims, DeviceClaims and EnrollmentDecisionClaims share no field name with RegisteredClaims, so the newly-populated Issuer survives marshalling. jwt.WithIssuer appears nowhere in either repo, so tokens already in circulation still validate.

Cross-repo is clean. cloud/ has zero references to jwttoken.*, token.NewToken, web.Config, session.ConsoleURL or the core services.NewService; it reaches the core service through WithCoreService, i.e. the instance Setup builds, so Cloud tokens carry the issuer too. The services.Service interface is unchanged, so no mock needs regenerating.

Spec

Every requirement in #7111 is met, with one partial that is not new: instanceIssuer takes the scheme from SHELLHUB_AUTO_SSL, so an instance terminating TLS upstream is named http://<domain> while served over HTTPS, against "SHELLHUB_DOMAIN is a bare hostname, so the scheme has to come from somewhere: https in production, http on a dev stack." Raised in round 1, deduplicated in round 2, answered, and recorded in the commit message. Not re-posted — see the note below for what is new about it.

Met: all three encoders set the issuer, EncodeEnrollmentDecisionClaims gaining the field it never had; validation stays out, with pkg/api/jwttoken/encoder.go untouched; the issuer derives from SHELLHUB_DOMAIN, which reaches the server container directly (docker-compose.yml:31) rather than only the gateway; localhost yields http://localhost; and "how configuration reaches a low-level signing package" is answered through WithIssuer and web.Config.Issuer.

Additional notes

Two things surfaced and were dropped in validation, recorded because they are useful to whoever picks up issuer validation.

• The scheme partial has a sharper form than either previous round stated. The contradiction now sits inside one of the functions this PR changes: enrollmentCallbackURL (server/api/services/enrollment.go:222-245) builds the returned URL from req.ForwardedProto/req.ForwardedHost, defaulting to https, while minting that same token's iss from the AutoSSL-derived s.issuer. A proxied instance therefore emits https://host/api/devices/enroll/callback/<token> carrying a token that names itself http://host. The forwarded-header rule is also the established one for absolute URLs elsewhere (invitation.go:151-161, member.go). Not re-raised as a finding, for two reasons: it is a concrete instance of the limitation the commit message already states, and the fix it points at is the wrong one — X-Forwarded-Proto/X-Forwarded-Host are client-adjacent, so deriving an identity claim from them trades a wrong scheme for an attacker-influenceable issuer. Worth carrying into the validation issue as the case that has to be decided, not fixed here.
• A judgement call about WithIssuer was raised and dropped. The issuer is read unconditionally on all four signing paths, so it is a mandatory construction value expressed as an optional one, and the Option doc comment had to widen to fit it. The failure mode is silent: deleting the single append at server/app/server.go:197 reverts Every JWT ShellHub issues carries a blank iss claim #7111 with the suite green — TestInstanceIssuer exercises the helper in isolation, and the services tests pass WithIssuer themselves, so they pin the option, not the wiring. Dropped because no remedy survives inspection. The constructor argument is answered by 129 test call sites against 1 production one; a non-empty guard in NewService would break the test sites that do not pass WithIssuer; and following the licenseEvaluatorOption/TestLicenseEvaluatorWiring precedent does not actually close the hole, since a test on an extracted issuerOption() stays green when the append is deleted. Round 1 noted the same residue and it is still true; there is no cheap way to pin that line, which is worth knowing rather than worth changing.

No caps dropped anything. Nothing was committed; this run only read the branch.

---

If you've addressed the feedback and want a new review, tag @shellhub-io/admin and a team member can trigger it.
• Branch: fix/jwt-issuer-claim