Skip to content

refactor(engine): drop the per-service JWT secrets from the co-host - #881

Closed
Bagatur (baskaryan) wants to merge 76 commits into
v16-stablefrom
chore/collapse-service-jwt-secrets
Closed

Bagatur (baskaryan) wants to merge 76 commits into
v16-stablefrom
chore/collapse-service-jwt-secrets

Conversation

@baskaryan

Copy link
Copy Markdown
Contributor

Draft: do not merge until the checklist below is complete. Merging early breaks Engine dispatch — see Ordering.

What

Removes the three per-service JWT secret env vars from the co-hosted deployment and from smith-go:

  • SMITH_BACKEND_SERVICE_JWT_SECRET and SMITH_GO_SERVICE_JWT_SECRET from langsmith.insights.extraEnv (the co-host verifier side)
  • SMITH_GO_SERVICE_JWT_SECRET from langsmith.engine.smithGoEnv (the signer side)

All three were set from api_key_salt — the same value X_SERVICE_AUTH_JWT_SECRET already carries on every pod via conditionalEnvVars. Three variables, one secret.

This is the change that actually collapses the configuration. langchain-ai/langchainplus#31926 only makes it possible, by teaching combined_auth to fall back to the shared secret and by selecting the service-key mode from the assistant id rather than from the secret being present.

Ordering — the reason this is a draft

Against an image whose combined_auth predates the fallback, removing these leaves no accepted key, and every Engine dispatch returns 401. The failure is total and looks like a credentials problem rather than a config-ordering one.

  • langchain-ai/langchainplus#31926 is merged
  • the co-hosted image is rebuilt from a SHA containing it
  • self-hosted-ci's pinned engineInsightsAgentImage.tag is bumped to that image — it is marked # nobump, so it will not move on its own

I did not add a chart-side version guard: the image tags (0.16.22rc1-43cf05f3) are not valid semver, so semverCompare would either error or silently misjudge, and a guard that rejects legitimate tags is worse than a documented ordering.

Test Plan

  • Renders with engine + sandboxes enabled: zero references to either per-service secret, 21 to the shared one.
  • Engine-disabled path renders unchanged.
  • helm lint clean across the CI values files.
  • Verified on self-hosted-ci after the checklist above — not yet possible.

…ndboxes-helm

# Conflicts:
#	charts/langsmith/Chart.yaml
#	charts/langsmith/README.md
…ndboxes-helm

# Conflicts:
#	charts/langsmith/Chart.yaml
…ndboxes-helm

# Conflicts:
#	charts/langsmith/templates/_helpers.tpl
…ndboxes-helm

# Conflicts:
#	charts/langsmith/Chart.yaml
#	charts/langsmith/README.md
…ndboxes-helm

# Conflicts:
#	charts/langsmith/Chart.yaml
* Inline JuiceFS CSI resources for sandboxes

* Free disk before LangSmith chart install CI

* Remove legacy JuiceFS CSI values fallback

* Remove LangSmith CI disk cleanup step

* Scope sandbox JuiceFS CSI RBAC

* Clarify JuiceFS CSI namespace scope

* Roll sandbox pods on JuiceFS config changes

* Scope JuiceFS checksum rollouts to CSI pods

* Remove unused JuiceFS CSI resizer
Wires the co-hosted Insights+Engine LangGraph deployment for self-hosted,
gated behind engine.enabled (default false, so Insights-only installs are
unchanged).

- engine values block (enabled / intelligenceBaseUrl / encryptionKey) and
  engine_encryption_key in the chart secret.
- Engine pods get ENGINE_INTELLIGENCE_BASE_URL, ISSUES_AGENT_ENCRYPTION_KEY,
  and the per-service X-Service-Key JWT verifier secrets.
- smith-go (platform-backend + ingest-queue) gets FORGE_AGENT_LANGGRAPH_URL,
  SMITH_GO_SERVICE_JWT_SECRET, ISSUES_AGENT_ENCRYPTION_KEY, and
  FORGE_AGENT_ASSISTANT_ID=engine. The combined langgraph.json names the graph
  `engine`; combined_auth rejects smith-go's default `smith_issues_agent` (403).
- Insights image selects the combined langsmith-insights-engine image when
  engine.enabled, else standalone langsmith-clio.
- smith-backend (backend + queue) gets CLIO_ASSISTANT_ID=insights when
  engine.enabled: the combined image names the CLIO graph `insights`, not the
  default `agent`.
- validate.yaml: require insights.enabled, encryptionKey, and
  intelligenceBaseUrl when engine.enabled.

Based on v16-stable rather than main: this ships in v16, and main's
agent-bootstrap removal touches the same three files, so a cherry-pick back
would conflict.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Engine and Insights run as one image on one deployment, so their deployment
config now lives in a single block instead of two sibling product blocks.

- engine / insights stay as top-level product toggles (enabled, encryptionKey;
  engine also carries intelligenceBaseUrl). Both encryption keys stay per
  product: they are different secrets for different products.
- New engineInsightsAgent block holds the shared deployment config moved from
  insights: namePrefix, apiServer, queue, postgres, redis. namePrefix keeps its
  "standalone-insights" value so no Kubernetes resource is renamed and the
  postgres/redis StatefulSets keep their volumes.
- images.insightsAgentImage + images.insightsEngineImage collapse into
  images.engineInsightsAgentImage. validate.yaml requires the combined
  insights-engine image when engine.enabled.
- The shared deployment now renders when either product is on, so Engine can
  run with Insights disabled. Replaces the earlier "engine requires
  insights.enabled" guard.
- validate.yaml hard-fails on the moved keys rather than silently ignoring
  them, so an upgrade cannot quietly drop a customized database or resource
  setting. Agent-feature validation now checks encryptionKey per product and
  postgres/redis per deployment.

Rendered output for an Insights-only install is byte-identical to before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both keys are shared with a counterpart service (engine with smith-go,
insights with smith-backend), so rotating either was a flash cut: payloads
encrypted just before the swap fail to decrypt after it.

Add encryptionKeyPrevious for both products, written to the
engine_encryption_key_previous / insights_encryption_key_previous secret
fields and exposed as ISSUES_AGENT_ENCRYPTION_KEY_PREVIOUS /
CLIO_ENCRYPTION_KEY_PREVIOUS. Both are optional: true, since a previous key
is absent in the common case and the chart cannot see inside an existing
secret when config.existingSecretName is set.

smith-clio already accepted CLIO_ENCRYPTION_KEY_PREVIOUS but the chart never
wired it, so rotation was unreachable from Helm for either product. The agent
side for engine is langchainplus worktree-engine-key-rotation.

Also adds the engine.* rows missing from the README values table.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
api_key_salt is the API-key hashing and service-JWT signing salt, not an API
key. It was only set because smith-go rejected an empty
FORGE_AGENT_LANGSMITH_API_KEY even when authenticating with a service-key
JWT, which is the only mode self-hosted uses. Had the API-key fallback ever
been taken, the salt would have gone out as x-api-key.

smith-go now requires the key only in API-key mode (langchainplus
worktree-engine-service-key-auth), so drop the variable. Engine dispatch
authenticates with the x-service-key JWT signed from SMITH_GO_SERVICE_JWT_SECRET.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…m' into feat/engine-self-hosted-v16-sandboxes

# Conflicts:
#	charts/langsmith/README.md
#	charts/langsmith/templates/ingest-queue/deployment.yaml
#	charts/langsmith/templates/platform-backend/deployment.yaml
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SMITH_BACKEND_SERVICE_JWT_SECRET and SMITH_GO_SERVICE_JWT_SECRET were both
set from api_key_salt -- the same value X_SERVICE_AUTH_JWT_SECRET already
carries on every pod via conditionalEnvVars. Three variables, one secret.

langchainplus now falls back to the shared secret when a service's own
variable is unset, and the service-key mode is selected by the assistant id
rather than by the secret being present, so removing these changes neither
what verifies nor which auth mode smith-go picks.

Do not deploy ahead of that change: against an image whose combined_auth
predates the fallback, removing these leaves no accepted key and every Engine
dispatch 401s. self-hosted-ci pins an image tag, so bump it first.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* test(engine): set auth prerequisites on the engine validation tests

Both engine cases asserted on the engine failure message but tripped the
earlier apiKeySalt and auth guards first, so they passed for the wrong reason
and never reached the assertion under test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat(engine): wire the sandbox tenant id for self-hosted Engine

The Engine signs its sandbox calls with a service key that must carry a
tenant_id claim, read from ISSUES_AGENT_SANDBOX_TENANT_ID. The chart set no
such value, so a self-hosted Engine raised at sandbox-create time.

Engine sandboxes live in one designated workspace rather than the run's own
tenant: a sandbox in the customer's workspace would consume their sandbox
quota, be visible and stoppable by them, and hold a GitHub App token for the
repo under analysis. SaaS uses a LangChain-internal workspace; self-hosted has
no vendor-owned one and there is no system-tenant primitive yet, so the
operator designates one. The values comment spells out the consequences so
that choice is informed.

Required only when sandboxes are enabled, and validated at template time —
otherwise the failure surfaces as a runtime error on the first Engine run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(engine): require sandboxes, and the tenant, whenever engine is enabled

The tenant id was only required when config.sandboxes.enabled, but the agent
raises whenever X_SERVICE_AUTH_JWT_SECRET is set -- which is on every pod,
independent of that flag. So engine.enabled with sandboxes disabled passed
validation and then failed at the first sandbox call.

Engine cannot run without sandboxes at all: every run shells out to the
langsmith CLI inside one. Make that a guard of its own, which lets the tenant
guard key on engine.enabled alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* feat(engine): point the Engine at its sandbox snapshot and base image

Engine runs boot from a named snapshot the Engine builds itself on first use,
so a self-hosted install needs no deploy-time snapshot build — that build wants
egress to the Wolfi, astral.sh, npm and GitHub hosts, which a customer cluster
may not have.

The image goes through langsmith.image, so images.registry rewrites it for a
mirrored registry like every other chart image. Both values default, so this
adds no required configuration.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Revert "feat(engine): point the Engine at its sandbox snapshot and base image"

This reverts commit c36b767.

* test(engine): drop the sandboxes-disabled tenant assertion

Reverting the snapshot wiring restored this case, but the tenant env var is no
longer gated on config.sandboxes.enabled, and engine.enabled now requires
sandboxes — so the combination it asserts cannot render.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both defaulted to SaaS, so a self-hosted Engine created sandboxes against
api.smith.langchain.com and its in-sandbox CLI had nowhere reachable to call.

LANGSMITH_SANDBOX_ENDPOINT is derived in-cluster like GO_ENDPOINT -- that hop
is Engine pod to platform-backend, so the service URL is fine.

engine.cliEndpoint has to be supplied: the sandbox runs the langsmith CLI
against it, so it must be externally routable, and setting it is also what
adds the host to the sandbox egress allow-list and to the proxy's
credential-injection rules. The chart cannot infer that from ingress, base
path and custom domains, so it is required when engine.enabled.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…boxes' into chore/collapse-service-jwt-secrets
Base automatically changed from feat/engine-self-hosted-v16-sandboxes to v16-stable July 31, 2026 01:45
@baskaryan

Copy link
Copy Markdown
Contributor Author

Superseded by #893. This branch predated two v16-stable merges, so its diff had grown to a phantom 60 files against the moved base. #893 is the same three-line change against current v16-stable, with the safety argument re-verified: the dispatch switch now keys off FORGE_AGENT_ASSISTANT_ID rather than the secret's presence (langchainplus #31926, shipped in 0.16.27rc1).

@baskaryan
Bagatur (baskaryan) deleted the chore/collapse-service-jwt-secrets branch July 31, 2026 02:48
Bagatur (baskaryan) added a commit that referenced this pull request Jul 31, 2026
smith-go and the co-hosted agent both fall back to the shared
X_SERVICE_AUTH_JWT_SECRET, which every affected pod already has, so the
per-service copies are redundant.

Safe to remove because the engine dispatch switch no longer depends on the
secret's presence: EngineServiceKeyAuth() is
`assistantID == "engine" || secret != ""`, and the chart sets
FORGE_AGENT_ASSISTANT_ID=engine on the smith-go pods (langchainplus #31926,
in 0.16.27rc1 and later).

Replaces #881, whose branch predated two v16-stable merges.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants