From 1df8dfb02b37e82c75a6f25dc9d41d028ef4c666 Mon Sep 17 00:00:00 2001 From: Joris Wouter Jonkers Date: Fri, 18 Sep 2026 14:16:37 +0200 Subject: [PATCH] feat(agents): run Claude and Codex in-container off the home volume ADR 0002: an Agent Login is a property of the user, not of a Workspace. The user signs in once from a terminal in an Agent Session, the CLI writes its own login files under $HOME, and the home volume (fleet-infra#326) keeps them across restarts. Everything agents-api did to capture, store, validate and inject a credential is therefore dead weight, and it goes here. Claude and Codex join Shell in-container. The gateway spawns bare `claude` or `codex` in tmux as `agent` through run-as-agent, which sets HOME=/home/agent, so the CLI finds its own login with nothing injected. Bare, not `claude -p` / `codex exec`: the interactive TUI is what a user can sign in from, and it is the only place a login is ever created. The headless forms belong to #66. `GET /api/v1/agent-logins` replaces `/api/v1/credentials/status`. The old endpoint reported what agents-api had captured and stored for a user; this reports whether a provider's CLI has written a login, read straight off the volume. Presence only -- nothing derived from the login reaches the wire. No `X-User-Id` either: credentials were per-user rows, but a home volume is one per container, so a user header would imply an answer this cannot give. A missing login is not an error and does not block a session. The CLI prompts for sign-in itself; agents-ui adds the hint alongside it (#64's third criterion, the agents-ui half still to come). Removed: the browser credential proxy and its five endpoints, the @Hidden internal ingest endpoint and the filter guarding it, HttpCredentialWorkerClient, CredentialValidator, AgentOauthCredential and its repository, and RunnerCredentialSecretManager with the Secret it stamped into every runner Pod. V28 drops `agent_oauth_credentials`. Forward-only and deliberately so: the rows are OAuth tokens nothing reads, and keeping them would leave live credentials in the database purely so a rollback could use a mechanism this release removes. The recovery for a bad release is to sign in again from an Agent Session, which is the point of putting the login on a volume. `destroy()` still deletes `agent-runner-credentials-`. It no longer creates one, but the Secrets an earlier release wrote hold OAuth tokens and nothing else reaps them. Two scope decisions, neither of them free: - Claude and Codex bind in-container for a **Scratch** Workspace only. A Repo-backed one still needs the Pod entrypoint's clone, so routing it here would start an Agent Session in an empty directory. The cost is that a Repo-backed Claude session has no Agent Login during the transition, because this removes the Secret injection that used to supply one. #67 closes it by moving the whole path in-container. - The Agent Kind no longer selects a binder. A Workspace binds in-container when it is Scratch **and** has no runner Pod: a Scratch Workspace created before #62 can still hold a Pod-bound Claude or Codex session, and those rows survive this deploy. Routing one in-container would strand it -- restart() throws, and ensureBound() answers with this container's directory instead of the Pod's /workspace. Three places encode that same test and all three now agree: RunnerSessionBindingRouter, AgentGatewayClientRouter, and AttachPreconditionChecker. The last one mattered most and was the easiest to miss: it still required `session.kind == SHELL`, so a Claude session bound in-container was resolved as remote, found no gateway endpoint -- a Scratch Workspace never provisions a Pod -- and the WebSocket attach was rejected with "workspace has no gateway endpoint". The tmux process started, the API returned Bound, and the terminal never opened, which removes the only place an Agent Login can be created. Covered now by a test proven to fail against the old gate. Verified: :api:test 617 passed / 2 skipped, :api:integrationTest 106 passed / 2 skipped, both with --rerun-tasks (a green UP-TO-DATE run is not a result). detekt and ktlint clean on main/test/integrationTest. The exported spec has no `/api/v1/credentials/**` path left and does have `/api/v1/agent-logins`, read back from the file rather than inferred. `oasdiff breaking --fail-on WARN` was run against both possible bases. Against origin/main it reports 5 x `api-path-removed-without-deprecation`; against the deprecation commit it reports none. That is the whole reason this is a second PR and not one -- api-contract-checks has no waiver. Part of #64 --- .../agents/contract/OpenApiSpecExportTest.kt | 31 +- ...8AgentRunnerOrchestratorIntegrationTest.kt | 423 +----------------- ...gentCredentialRepositoryIntegrationTest.kt | 103 ----- .../InContainerSessionBindingService.kt | 26 +- .../RunnerSessionBindingRouter.kt | 47 +- .../agents/config/AgentRuntimeProperties.kt | 19 +- .../agents/config/SecurityConfig.kt | 9 - .../domain/model/AgentOauthCredential.kt | 38 -- .../domain/port/AgentCredentialRepository.kt | 39 -- .../agents/domain/port/AgentLoginStore.kt | 27 ++ .../credentials/CredentialValidator.kt | 49 -- .../integration/AgentGatewayClientRouter.kt | 26 +- .../integration/HttpCredentialWorkerClient.kt | 125 ------ .../InContainerAgentGatewayClient.kt | 26 +- .../k8s/Fabric8AgentRunnerOrchestrator.kt | 22 +- .../k8s/RunnerCredentialSecretManager.kt | 115 ----- .../k8s/RunnerPodSpecBuilder.kt | 123 +---- .../login/HomeVolumeAgentLoginStore.kt | 55 +++ .../JooqAgentCredentialRepository.kt | 123 ----- .../web/AgentLoginController.kt | 44 ++ .../web/CredentialController.kt | 134 ------ .../web/InternalCredentialController.kt | 81 ---- .../infrastructure/web/dto/AgentLoginDtos.kt | 30 ++ .../infrastructure/web/dto/CredentialDtos.kt | 125 ------ .../ws/AttachPreconditionChecker.kt | 16 +- api/src/main/resources/application.yml | 14 +- .../V28__drop_agent_oauth_credentials.sql | 14 + .../InContainerSessionBindingServiceTest.kt | 31 +- .../RunnerSessionBindingRouterTest.kt | 69 ++- .../AgentRuntimePropertiesBindingTest.kt | 4 +- .../config/InternalBearerAuthFilterTest.kt | 6 +- .../credentials/CredentialValidatorTest.kt | 19 - .../InContainerAgentGatewayClientTest.kt | 54 ++- .../login/HomeVolumeAgentLoginStoreTest.kt | 92 ++++ .../web/AgentLoginControllerTest.kt | 73 +++ .../web/CredentialControllerTest.kt | 280 ------------ .../web/InternalCredentialControllerTest.kt | 234 ---------- .../ws/AttachPreconditionCheckerTest.kt | 34 ++ client-spec/openapi/agents-api-client.json | 259 ++--------- client-spec/openapi/agents-api.json | 259 ++--------- 40 files changed, 770 insertions(+), 2528 deletions(-) delete mode 100644 api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/agents/persistence/JooqAgentCredentialRepositoryIntegrationTest.kt delete mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/domain/model/AgentOauthCredential.kt delete mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/domain/port/AgentCredentialRepository.kt create mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/domain/port/AgentLoginStore.kt delete mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/credentials/CredentialValidator.kt delete mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/HttpCredentialWorkerClient.kt delete mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerCredentialSecretManager.kt create mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/login/HomeVolumeAgentLoginStore.kt delete mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/persistence/JooqAgentCredentialRepository.kt create mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/AgentLoginController.kt delete mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/CredentialController.kt delete mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/InternalCredentialController.kt create mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/dto/AgentLoginDtos.kt delete mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/dto/CredentialDtos.kt create mode 100644 api/src/main/resources/db/migration/V28__drop_agent_oauth_credentials.sql delete mode 100644 api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/credentials/CredentialValidatorTest.kt create mode 100644 api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/login/HomeVolumeAgentLoginStoreTest.kt create mode 100644 api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/AgentLoginControllerTest.kt delete mode 100644 api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/CredentialControllerTest.kt delete mode 100644 api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/InternalCredentialControllerTest.kt diff --git a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/agents/contract/OpenApiSpecExportTest.kt b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/agents/contract/OpenApiSpecExportTest.kt index 0116f6a..8d02459 100644 --- a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/agents/contract/OpenApiSpecExportTest.kt +++ b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/agents/contract/OpenApiSpecExportTest.kt @@ -18,27 +18,24 @@ import com.jorisjonkers.personalstack.agents.application.setup.AgentSetupDiffSer import com.jorisjonkers.personalstack.agents.application.setup.AgentSetupValidationService import com.jorisjonkers.personalstack.agents.application.workspacerunner.WorkspaceRunnerLifecycleService import com.jorisjonkers.personalstack.agents.config.OpenApiConfig -import com.jorisjonkers.personalstack.agents.domain.port.AgentCredentialRepository import com.jorisjonkers.personalstack.agents.domain.port.AgentGatewayClient +import com.jorisjonkers.personalstack.agents.domain.port.AgentLoginStore import com.jorisjonkers.personalstack.agents.domain.port.AgentSessionRepository import com.jorisjonkers.personalstack.agents.domain.port.AgentSetupRepository import com.jorisjonkers.personalstack.agents.domain.port.GithubLinkRepository import com.jorisjonkers.personalstack.agents.domain.port.SetupRestartEventRepository import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceRepository -import com.jorisjonkers.personalstack.agents.infrastructure.credentials.CredentialValidator import com.jorisjonkers.personalstack.agents.infrastructure.integration.GitHubAppInstallationTokenClient -import com.jorisjonkers.personalstack.agents.infrastructure.integration.HttpCredentialWorkerClient import com.jorisjonkers.personalstack.agents.infrastructure.web.AdminRunnerController +import com.jorisjonkers.personalstack.agents.infrastructure.web.AgentLoginController import com.jorisjonkers.personalstack.agents.infrastructure.web.AgentRunnerUnavailableExceptionHandler import com.jorisjonkers.personalstack.agents.infrastructure.web.AgentSessionController import com.jorisjonkers.personalstack.agents.infrastructure.web.AgentSetupController import com.jorisjonkers.personalstack.agents.infrastructure.web.AgentSetupExceptionHandler import com.jorisjonkers.personalstack.agents.infrastructure.web.ChatSessionController import com.jorisjonkers.personalstack.agents.infrastructure.web.ConversationController -import com.jorisjonkers.personalstack.agents.infrastructure.web.CredentialController import com.jorisjonkers.personalstack.agents.infrastructure.web.GitController import com.jorisjonkers.personalstack.agents.infrastructure.web.HealthController -import com.jorisjonkers.personalstack.agents.infrastructure.web.InternalCredentialController import com.jorisjonkers.personalstack.agents.infrastructure.web.InternalGitHubTokenController import com.jorisjonkers.personalstack.agents.infrastructure.web.KubernetesExceptionHandler import com.jorisjonkers.personalstack.agents.infrastructure.web.ProjectController @@ -75,14 +72,13 @@ import java.nio.file.Paths @WebMvcTest( controllers = [ AdminRunnerController::class, + AgentLoginController::class, AgentSetupController::class, AgentSessionController::class, ChatSessionController::class, ConversationController::class, - CredentialController::class, GitController::class, HealthController::class, - InternalCredentialController::class, InternalGitHubTokenController::class, ProjectController::class, RepositoryController::class, @@ -111,14 +107,13 @@ import java.nio.file.Paths KubernetesExceptionHandler::class, RepositoryAccessDeniedExceptionHandler::class, AdminRunnerController::class, + AgentLoginController::class, AgentSetupController::class, AgentSessionController::class, ChatSessionController::class, ConversationController::class, - CredentialController::class, GitController::class, HealthController::class, - InternalCredentialController::class, InternalGitHubTokenController::class, ProjectController::class, RepositoryController::class, @@ -145,12 +140,18 @@ class OpenApiSpecExportTest .andExpect(jsonPath("$['paths']['/api/v1/sessions/events']").doesNotExist()) } + // #64 retired the whole credential-capture surface: the browser proxy, + // the @Hidden internal ingest endpoint, and the stored-credential + // status it reported. `/api/v1/agent-logins` replaces the last of those + // and reports presence only, read off the home volume (ADR 0002). @Test - fun internalCredentialEndpointIsHiddenWhileBrowserCredentialStatusRemainsExported() { + fun credentialEndpointsAreGoneAndAgentLoginStatusReplacesThem() { mockMvc .perform(get("/api/v1/api-docs")) .andExpect(jsonPath("$['paths']['/api/v1/internal/credentials']").doesNotExist()) - .andExpect(jsonPath("$['paths']['/api/v1/credentials/status']").exists()) + .andExpect(jsonPath("$['paths']['/api/v1/credentials/status']").doesNotExist()) + .andExpect(jsonPath("$['paths']['/api/v1/credentials/sessions']").doesNotExist()) + .andExpect(jsonPath("$['paths']['/api/v1/agent-logins']").exists()) } @Test @@ -269,9 +270,6 @@ class OpenApiSpecExportTest @TestConfiguration(proxyBeanMethods = false) class RepositoryCollaborators { - @Bean - fun agentCredentialRepository(): AgentCredentialRepository = mockk(relaxed = true) - @Bean fun agentSetupRepository(): AgentSetupRepository = mockk(relaxed = true) @@ -291,14 +289,11 @@ class OpenApiSpecExportTest @TestConfiguration(proxyBeanMethods = false) class InfrastructureCollaborators { @Bean - fun credentialValidator(): CredentialValidator = mockk(relaxed = true) + fun agentLoginStore(): AgentLoginStore = mockk(relaxed = true) @Bean fun githubAppInstallationTokenClient(): GitHubAppInstallationTokenClient = mockk(relaxed = true) - @Bean - fun httpCredentialWorkerClient(): HttpCredentialWorkerClient = mockk(relaxed = true) - @Bean fun workspaceRunnerLifecycleService(): WorkspaceRunnerLifecycleService = mockk(relaxed = true) } diff --git a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/agents/k8s/Fabric8AgentRunnerOrchestratorIntegrationTest.kt b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/agents/k8s/Fabric8AgentRunnerOrchestratorIntegrationTest.kt index d6d5e1d..3bbc460 100644 --- a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/agents/k8s/Fabric8AgentRunnerOrchestratorIntegrationTest.kt +++ b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/agents/k8s/Fabric8AgentRunnerOrchestratorIntegrationTest.kt @@ -1,8 +1,6 @@ package com.jorisjonkers.personalstack.agents.k8s import com.jorisjonkers.personalstack.agents.config.AgentRuntimeProperties -import com.jorisjonkers.personalstack.agents.domain.model.AgentCredentialProvider -import com.jorisjonkers.personalstack.agents.domain.model.AgentOauthCredential import com.jorisjonkers.personalstack.agents.domain.model.AgentSetupId import com.jorisjonkers.personalstack.agents.domain.model.AgentSetupVersion import com.jorisjonkers.personalstack.agents.domain.model.RunnerSetupProvisioningSpec @@ -10,7 +8,6 @@ import com.jorisjonkers.personalstack.agents.domain.model.RunnerState import com.jorisjonkers.personalstack.agents.domain.model.Workspace import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceId import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceStatus -import com.jorisjonkers.personalstack.agents.domain.port.AgentCredentialRepository import com.jorisjonkers.personalstack.agents.domain.port.RepositoryRepository import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceRepositoryRepository import com.jorisjonkers.personalstack.agents.infrastructure.k8s.Fabric8AgentRunnerOrchestrator @@ -52,7 +49,6 @@ import java.time.Instant * 2. provision with pre-#372 restricted RBAC fails with patch-forbidden * 3. provision is idempotent (server-side apply semantics) * 4. destroy removes the resources - * 5. provision stamps owner credentials into a workspace-scoped Secret */ @Tag("integration") @Testcontainers @@ -144,14 +140,12 @@ open class Fabric8AgentRunnerOrchestratorIntegrationSupport { protected fun orchestrator( client: KubernetesClient, - credentials: ObjectProvider = empty(), workspaceRepos: ObjectProvider = empty(), repositories: ObjectProvider = empty(), ): Fabric8AgentRunnerOrchestrator = Fabric8AgentRunnerOrchestrator( client = client, props = testProps(), - credentialsProvider = credentials, workspaceRepos = workspaceRepos, repositories = repositories, ) @@ -235,76 +229,6 @@ open class Fabric8AgentRunnerOrchestratorIntegrationSupport { } } - protected class StaticAgentCredentialRepository( - private val owner: String, - ) : AgentCredentialRepository { - var claude: String? = null - var claudeCredentialsJson: String? = null - var claudeAccountJson: String? = null - var codexAuthJson: String? = null - var codexConfigToml: String? = null - var valid: Boolean? = true - - override fun upsert(credential: AgentOauthCredential): AgentOauthCredential = error("not used in this test") - - override fun find( - userId: String, - provider: AgentCredentialProvider, - ): AgentOauthCredential? { - if (userId != owner) return null - val payload = - when (provider) { - AgentCredentialProvider.CLAUDE -> - buildMap { - claude?.let { put("oauth_token", it) } - claudeCredentialsJson?.let { put("credentials_json", it) } - claudeAccountJson?.let { put("account_json", it) } - }.takeIf { it.isNotEmpty() } - AgentCredentialProvider.CODEX -> - buildMap { - codexAuthJson?.let { put("auth_json", it) } - codexConfigToml?.let { put("config_toml", it) } - }.takeIf { it.isNotEmpty() } - } ?: return null - return AgentOauthCredential( - userId = userId, - provider = provider, - payload = payload, - valid = valid, - validatedAt = null, - updatedAt = Instant.now(), - updatedBy = userId, - ) - } - - override fun markValidity( - userId: String, - provider: AgentCredentialProvider, - valid: Boolean, - ) = error("not used in this test") - - override fun statusFor(userId: String): List = - error("not used in this test") - } - - protected class FailingAgentCredentialRepository : AgentCredentialRepository { - override fun upsert(credential: AgentOauthCredential): AgentOauthCredential = error("not used in this test") - - override fun find( - userId: String, - provider: AgentCredentialProvider, - ): AgentOauthCredential? = error("store unavailable") - - override fun markValidity( - userId: String, - provider: AgentCredentialProvider, - valid: Boolean, - ) = error("not used in this test") - - override fun statusFor(userId: String): List = - error("not used in this test") - } - /** * Minimal `ObjectProvider` impl. The orchestrator only ever * reads `.ifAvailable`, so the unused defaults are fine — but @@ -708,31 +632,6 @@ class Fabric8AgentRunnerOrchestratorProvisioningLifecycleIntegrationTest : } } - @Test - @DisplayName("scaleDown keeps the per-workspace credential Secret for wake-up reprovision") - fun scaledownKeepsCredentialSecretForWakeUpReprovision() { - K3sTestSupport.applyProductionRbac(admin) - saScoped = K3sTestSupport.createServiceAccountScopedClient(k3s) - val owner = "user-scale-down" - val orchestrator = - orchestrator( - saScoped, - credentials = wrap(StaticAgentCredentialRepository(owner).apply { claude = "claude-scale-down" }), - ) - val workspace = adHocWorkspace().copy(ownerUserId = owner) - orchestrator.provision(workspace) - - orchestrator.scaleDown(workspace) - - assertThat( - admin - .secrets() - .inNamespace(K3sTestSupport.AGENTS_NAMESPACE) - .withName("agent-runner-credentials-${workspace.id.short()}") - .get(), - ).isNotNull - } - @Test @DisplayName("destroy removes the PVC, Pod, and Service") fun destroyRemovesTheFourResources() { @@ -782,19 +681,31 @@ class Fabric8AgentRunnerOrchestratorProvisioningLifecycleIntegrationTest : } class Fabric8AgentRunnerOrchestratorCredentialIntegrationTest : Fabric8AgentRunnerOrchestratorIntegrationSupport() { + // #64 stopped creating this Secret, and destroy() still reaps it: the ones + // an earlier release wrote hold OAuth tokens and nothing else deletes them. + // Seeded by hand here, because provision no longer produces one. @Test - @DisplayName("destroy removes the per-workspace credential Secret") + @DisplayName("destroy removes a per-workspace credential Secret left by an earlier release") fun destroyRemovesCredentialSecret() { K3sTestSupport.applyProductionRbac(admin) saScoped = K3sTestSupport.createServiceAccountScopedClient(k3s) val owner = "user-destroy" - val orchestrator = - orchestrator( - saScoped, - credentials = wrap(StaticAgentCredentialRepository(owner).apply { claude = "claude-destroy" }), - ) + val orchestrator = orchestrator(saScoped) val workspace = adHocWorkspace().copy(ownerUserId = owner) orchestrator.provision(workspace) + admin + .secrets() + .inNamespace(K3sTestSupport.AGENTS_NAMESPACE) + .resource( + io.fabric8.kubernetes.api.model + .SecretBuilder() + .withNewMetadata() + .withName("agent-runner-credentials-${workspace.id.short()}") + .withNamespace(K3sTestSupport.AGENTS_NAMESPACE) + .endMetadata() + .withStringData(mapOf("claude_oauth_token" to "left-behind")) + .build(), + ).serverSideApply() orchestrator.destroy(workspace) @@ -809,304 +720,6 @@ class Fabric8AgentRunnerOrchestratorCredentialIntegrationTest : Fabric8AgentRunn ?.deletionTimestamp } } - - @Test - @DisplayName("provision injects full Claude credential files without OAuth env override") - fun provisionInjectsFullClaudeCredentialsWithoutOauthEnvOverride() { - K3sTestSupport.applyProductionRbac(admin) - saScoped = K3sTestSupport.createServiceAccountScopedClient(k3s) - val owner = "user-full-claude" - val orchestrator = - orchestrator( - saScoped, - credentials = - wrap( - StaticAgentCredentialRepository(owner).apply { - claude = "legacy-token" - claudeCredentialsJson = - """{"claudeAiOauth":{"accessToken":"current","refreshToken":"refresh"}}""" - claudeAccountJson = """{"billingType":"subscription","seatTier":"max"}""" - }, - ), - ) - val workspace = adHocWorkspace().copy(ownerUserId = owner) - - orchestrator.provision(workspace) - - val short = workspace.id.short() - val secret = - admin - .secrets() - .inNamespace(K3sTestSupport.AGENTS_NAMESPACE) - .withName("agent-runner-credentials-$short") - .get() - assertThat(secret).isNotNull - assertThat(secret.data).containsKeys( - "claude_oauth_token", - "claude_credentials_json", - "claude_account_json", - ) - val pod = - admin - .pods() - .inNamespace(K3sTestSupport.AGENTS_NAMESPACE) - .withName("agent-runner-$short") - .get() - val container = pod.spec.containers.single() - val env = container.env.associateBy { it.name } - assertThat(env["AGENT_CLAUDE_CREDENTIALS_FILE"]?.value) - .isEqualTo("/var/run/secrets/agents/credentials/claude_credentials_json") - assertThat(env["AGENT_CLAUDE_ACCOUNT_FILE"]?.value) - .isEqualTo("/var/run/secrets/agents/credentials/claude_account_json") - assertThat(env).doesNotContainKey("CLAUDE_CODE_OAUTH_TOKEN") - val mount = container.volumeMounts.single { it.name == "agent-credentials" } - assertThat(mount.mountPath).isEqualTo("/var/run/secrets/agents/credentials") - assertThat(mount.readOnly).isTrue() - } - - @Test - @DisplayName("provision injects owner Claude and Codex credentials through a per-workspace Secret") - fun provisionInjectsOwnerCredentialsThroughWorkspaceSecret() { - K3sTestSupport.applyProductionRbac(admin) - saScoped = K3sTestSupport.createServiceAccountScopedClient(k3s) - val owner = "user-credentials" - val orchestrator = - orchestrator( - saScoped, - credentials = - wrap( - StaticAgentCredentialRepository(owner).apply { - claude = "claude-current" - codexAuthJson = """{"tokens":"current"}""" - codexConfigToml = "profile = \"current\"" - }, - ), - ) - val workspace = adHocWorkspace().copy(ownerUserId = owner) - - orchestrator.provision(workspace) - - val short = workspace.id.short() - val secret = - admin - .secrets() - .inNamespace(K3sTestSupport.AGENTS_NAMESPACE) - .withName("agent-runner-credentials-$short") - .get() - assertThat(secret).isNotNull - assertThat(secret.data).containsKeys("claude_oauth_token", "codex_auth_json", "codex_config_toml") - val pod = - admin - .pods() - .inNamespace(K3sTestSupport.AGENTS_NAMESPACE) - .withName("agent-runner-$short") - .get() - val container = pod.spec.containers.single() - val env = container.env.associateBy { it.name } - assertThat(env["CLAUDE_CODE_OAUTH_TOKEN"]?.value).isNull() - assertThat(env["CLAUDE_CODE_OAUTH_TOKEN"]?.valueFrom?.secretKeyRef?.name) - .isEqualTo("agent-runner-credentials-$short") - assertThat(env["CLAUDE_CODE_OAUTH_TOKEN"]?.valueFrom?.secretKeyRef?.key).isEqualTo("claude_oauth_token") - assertThat(env["AGENT_CODEX_AUTH_JSON_FILE"]?.value) - .isEqualTo("/var/run/secrets/agents/credentials/codex_auth_json") - assertThat(env["AGENT_CODEX_CONFIG_TOML_FILE"]?.value) - .isEqualTo("/var/run/secrets/agents/credentials/codex_config_toml") - val mount = container.volumeMounts.single { it.name == "agent-credentials" } - assertThat(mount.mountPath).isEqualTo("/var/run/secrets/agents/credentials") - assertThat(mount.readOnly).isTrue() - assertAgentStatePersistence(pod) - assertThat( - pod.spec.volumes - .single { it.name == "agent-credentials" } - .secret.secretName, - ).isEqualTo("agent-runner-credentials-$short") - } - - @Test - @DisplayName("provision injects a codex credential with only auth.json (config.toml optional)") - fun provisionInjectsCodexCredentialWithoutConfigToml() { - K3sTestSupport.applyProductionRbac(admin) - saScoped = K3sTestSupport.createServiceAccountScopedClient(k3s) - val owner = "user-codex-auth-only" - val orchestrator = - orchestrator( - saScoped, - // `codex login` writes auth.json but not config.toml; the credential must - // still be injected (the runner self-provisions a config.toml when absent). - credentials = - wrap( - StaticAgentCredentialRepository(owner).apply { - codexAuthJson = """{"tokens":"current"}""" - }, - ), - ) - val workspace = adHocWorkspace().copy(ownerUserId = owner) - - orchestrator.provision(workspace) - - val short = workspace.id.short() - val secret = - admin - .secrets() - .inNamespace(K3sTestSupport.AGENTS_NAMESPACE) - .withName("agent-runner-credentials-$short") - .get() - assertThat(secret).isNotNull - assertThat(secret.data).containsKey("codex_auth_json") - assertThat(secret.data).doesNotContainKey("codex_config_toml") - val container = - admin - .pods() - .inNamespace(K3sTestSupport.AGENTS_NAMESPACE) - .withName("agent-runner-$short") - .get() - .spec.containers - .single() - val env = container.env.associateBy { it.name } - assertThat(env["AGENT_CODEX_AUTH_JSON_FILE"]?.value) - .isEqualTo("/var/run/secrets/agents/credentials/codex_auth_json") - assertThat(env).doesNotContainKey("AGENT_CODEX_CONFIG_TOML_FILE") - } - - @Test - @DisplayName("provision skips workspace credential Secret when owner or required payload is missing") - fun provisionSkipsCredentialSecretWhenOwnerOrCredentialMissing() { - K3sTestSupport.applyProductionRbac(admin) - saScoped = K3sTestSupport.createServiceAccountScopedClient(k3s) - val owner = "user-missing" - val orchestrator = - orchestrator( - saScoped, - credentials = wrap(StaticAgentCredentialRepository(owner)), - ) - val noOwner = adHocWorkspace() - val missingPayload = adHocWorkspace().copy(ownerUserId = owner) - - orchestrator.provision(noOwner) - orchestrator.provision(missingPayload) - - assertThat( - admin - .secrets() - .inNamespace(K3sTestSupport.AGENTS_NAMESPACE) - .withName("agent-runner-credentials-${noOwner.id.short()}") - .get(), - ).isNull() - assertThat( - admin - .secrets() - .inNamespace(K3sTestSupport.AGENTS_NAMESPACE) - .withName("agent-runner-credentials-${missingPayload.id.short()}") - .get(), - ).isNull() - } - - @Test - @DisplayName("credential store read failure skips credential Secret without failing provision") - fun credentialStoreReadFailureSkipsCredentialSecret() { - K3sTestSupport.applyProductionRbac(admin) - saScoped = K3sTestSupport.createServiceAccountScopedClient(k3s) - val orchestrator = - orchestrator( - saScoped, - credentials = wrap(FailingAgentCredentialRepository()), - ) - val workspace = adHocWorkspace().copy(ownerUserId = "user-store-failure") - - orchestrator.provision(workspace) - - assertThat( - admin - .secrets() - .inNamespace(K3sTestSupport.AGENTS_NAMESPACE) - .withName("agent-runner-credentials-${workspace.id.short()}") - .get(), - ).isNull() - assertThat( - admin - .pods() - .inNamespace(K3sTestSupport.AGENTS_NAMESPACE) - .withName("agent-runner-${workspace.id.short()}") - .get(), - ).isNotNull - } - - @Test - @DisplayName("re-provision updates the per-workspace credential Secret") - fun reprovisionUpdatesCredentialSecret() { - K3sTestSupport.applyProductionRbac(admin) - saScoped = K3sTestSupport.createServiceAccountScopedClient(k3s) - val owner = "user-update" - val credentials = StaticAgentCredentialRepository(owner).apply { claude = "old" } - val orchestrator = - orchestrator( - saScoped, - credentials = wrap(credentials), - ) - val workspace = adHocWorkspace().copy(ownerUserId = owner) - - orchestrator.provision(workspace) - credentials.claude = "new" - orchestrator.provision(workspace) - - val encoded = - admin - .secrets() - .inNamespace(K3sTestSupport.AGENTS_NAMESPACE) - .withName("agent-runner-credentials-${workspace.id.short()}") - .get() - .data["claude_oauth_token"] - assertThat( - String( - java.util.Base64 - .getDecoder() - .decode(encoded), - ), - ).isEqualTo("new") - } - - @Test - @DisplayName("invalid owner credential is skipped without cluster-wide OAuth fallback") - fun invalidCredentialSkipsSecretAndClusterWideOauthFallback() { - K3sTestSupport.applyProductionRbac(admin) - saScoped = K3sTestSupport.createServiceAccountScopedClient(k3s) - val owner = "user-invalid" - val orchestrator = - orchestrator( - saScoped, - credentials = - wrap( - StaticAgentCredentialRepository(owner).apply { - claude = "invalid" - valid = false - }, - ), - ) - val workspace = adHocWorkspace().copy(ownerUserId = owner) - - orchestrator.provision(workspace) - - val short = workspace.id.short() - assertThat( - admin - .secrets() - .inNamespace(K3sTestSupport.AGENTS_NAMESPACE) - .withName("agent-runner-credentials-$short") - .get(), - ).isNull() - val env = - admin - .pods() - .inNamespace(K3sTestSupport.AGENTS_NAMESPACE) - .withName("agent-runner-$short") - .get() - .spec - .containers - .single() - .env - assertThat(env.map { it.name }).doesNotContain("CLAUDE_CODE_OAUTH_TOKEN") - assertThat(env.mapNotNull { it.valueFrom?.secretKeyRef?.name }).doesNotContain("agents-claude-oauth") - } } class Fabric8AgentRunnerOrchestratorStateIntegrationTest : Fabric8AgentRunnerOrchestratorIntegrationSupport() { diff --git a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/agents/persistence/JooqAgentCredentialRepositoryIntegrationTest.kt b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/agents/persistence/JooqAgentCredentialRepositoryIntegrationTest.kt deleted file mode 100644 index f1cd412..0000000 --- a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/agents/persistence/JooqAgentCredentialRepositoryIntegrationTest.kt +++ /dev/null @@ -1,103 +0,0 @@ -package com.jorisjonkers.personalstack.agents.persistence - -import com.jorisjonkers.personalstack.agents.IntegrationTestBase -import com.jorisjonkers.personalstack.agents.domain.model.AgentCredentialProvider -import com.jorisjonkers.personalstack.agents.domain.model.AgentOauthCredential -import com.jorisjonkers.personalstack.agents.domain.port.AgentCredentialRepository -import org.assertj.core.api.Assertions.assertThat -import org.junit.jupiter.api.Test -import org.springframework.beans.factory.annotation.Autowired -import java.time.Instant -import java.util.UUID - -class JooqAgentCredentialRepositoryIntegrationTest - @Autowired - constructor( - private val store: AgentCredentialRepository, - ) : IntegrationTestBase { - private fun userId() = "user-${UUID.randomUUID()}" - - @Test - fun upsertThenFindRoundTripsThePayloadAndResetsValidity() { - val user = userId() - store.upsert( - AgentOauthCredential( - userId = user, - provider = AgentCredentialProvider.CLAUDE, - payload = mapOf("oauth_token" to "sk-ant-oat01-abc"), - valid = true, - validatedAt = Instant.now(), - updatedAt = Instant.now(), - updatedBy = user, - ), - ) - - val loaded = (store.find(user, AgentCredentialProvider.CLAUDE)).required() - assertThat(loaded.payload["oauth_token"]).isEqualTo("sk-ant-oat01-abc") - // Upsert resets validity until the next probe. - assertThat(loaded.valid).isNull() - assertThat(loaded.validatedAt).isNull() - } - - @Test - fun upsertIsPerUserAndProvider() { - val u1 = userId() - val u2 = userId() - store.upsert(cred(u1, AgentCredentialProvider.CLAUDE, "t1")) - store.upsert(cred(u2, AgentCredentialProvider.CLAUDE, "t2")) - store.upsert(cred(u1, AgentCredentialProvider.CODEX, "c1")) - - assertThat(store.find(u1, AgentCredentialProvider.CLAUDE).required().payload["oauth_token"]).isEqualTo("t1") - assertThat(store.find(u2, AgentCredentialProvider.CLAUDE).required().payload["oauth_token"]).isEqualTo("t2") - assertThat(store.find(u1, AgentCredentialProvider.CODEX).required().payload["oauth_token"]).isEqualTo("c1") - } - - @Test - fun reUpsertOverwritesInPlaceNoDuplicateRowsAndMarkValidityFlipsTheFlag() { - val user = userId() - store.upsert(cred(user, AgentCredentialProvider.CLAUDE, "old")) - store.upsert(cred(user, AgentCredentialProvider.CLAUDE, "new")) - assertThat( - store.find(user, AgentCredentialProvider.CLAUDE).required().payload["oauth_token"], - ).isEqualTo("new") - - store.markValidity(user, AgentCredentialProvider.CLAUDE, true) - val validated = store.find(user, AgentCredentialProvider.CLAUDE).required() - assertThat(validated.valid).isTrue() - assertThat(validated.validatedAt).isNotNull() - - val status = store.statusFor(user) - assertThat(status).hasSize(2) - val claude = status.single { it.provider == AgentCredentialProvider.CLAUDE } - val codex = status.single { it.provider == AgentCredentialProvider.CODEX } - assertThat(claude.stored).isTrue() - assertThat(claude.valid).isTrue() - assertThat(claude.validatedAt).isNotNull() - assertThat(codex.stored).isFalse() - assertThat(codex.valid).isNull() - } - - @Test - fun findReturnsNullAndStatusMarksBothProvidersAbsentWhenNothingStored() { - val user = userId() - val status = store.statusFor(user) - - assertThat(store.find(user, AgentCredentialProvider.CLAUDE)).isNull() - assertThat(status).hasSize(2) - assertThat(status.all { !it.stored }).isTrue() - } - - private fun cred( - user: String, - provider: AgentCredentialProvider, - token: String, - ) = AgentOauthCredential( - userId = user, - provider = provider, - payload = mapOf("oauth_token" to token), - valid = null, - validatedAt = null, - updatedAt = Instant.now(), - updatedBy = user, - ) - } diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/InContainerSessionBindingService.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/InContainerSessionBindingService.kt index 24d264f..70dbc84 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/InContainerSessionBindingService.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/InContainerSessionBindingService.kt @@ -4,7 +4,6 @@ import com.jorisjonkers.personalstack.agents.application.sessionstatus.SessionSt import com.jorisjonkers.personalstack.agents.application.workspace.WorkspaceDirectoryService import com.jorisjonkers.personalstack.agents.domain.model.AgentSession import com.jorisjonkers.personalstack.agents.domain.model.AgentSessionStatus -import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentKind import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceKind import com.jorisjonkers.personalstack.agents.domain.port.AgentGatewayClient import com.jorisjonkers.personalstack.agents.domain.port.AgentSessionRepository @@ -15,14 +14,16 @@ import org.springframework.stereotype.Component import java.time.Instant /** - * Binds a Shell Agent Session running in-container: no runner Pod, no - * setup catalog, no boot lease — the tmux session either starts or it - * doesn't, synchronously. [RunnerSessionBinder]'s CAS/generation/setup - * machinery exists for the Pod path's provisioning races; there is - * nothing to provision here, so this stays deliberately small. + * Binds an Agent Session running in-container: no runner Pod, no setup + * catalog, no boot lease — the tmux session either starts or it doesn't, + * synchronously. [RunnerSessionBinder]'s CAS/generation/setup machinery + * exists for the Pod path's provisioning races; there is nothing to + * provision here, so this stays deliberately small. * - * Selected by [RunnerSessionBindingRouter] for a Shell Agent Session in - * a Scratch Workspace. + * Selected by [RunnerSessionBindingRouter] for any Agent Session in a + * Scratch Workspace. Claude and Codex joined Shell here in #64: the CLI + * reads its Agent Login from the home volume, which only this container + * has, so an Agent Session that needs a login has to run here. */ @Component class InContainerSessionBindingService( @@ -41,9 +42,6 @@ class InContainerSessionBindingService( require(workspace.kind == WorkspaceKind.SCRATCH) { "the in-container binding service only starts sessions in a Scratch Workspace: ${workspace.id.value}" } - require(request.kind == WorkspaceAgentKind.SHELL) { - "the in-container binding service only starts Shell Agent Sessions; requested kind=${request.kind}" - } val now = Instant.now() val session = AgentSession( @@ -93,8 +91,8 @@ class InContainerSessionBindingService( // tracer-bullet's scope (#62); it lands with the idle/suspend sweep (#65). override fun restart(request: RestartRunnerSessionBindingInput): RunnerSessionBindingResult = throw UnsupportedOperationException( - "restart of an in-container Shell Agent Session is not implemented (tracer bullet #62 scope; " + - "Suspend/Resume for Shell lands with #65)", + "restart of an in-container Agent Session is not implemented (tracer bullet #62 scope; " + + "Suspend/Resume lands with #65)", ) override fun ensureBound(request: EnsureRunnerSessionBoundInput): RunnerSessionBindingResult { @@ -108,6 +106,8 @@ class InContainerSessionBindingService( // The tmux session is only ever bound at start() and unbound at stop(); // a RUNNING session with no gatewayAgentId lost its process (a container // restart killed tmux) and isn't resumable in this scope — see restart(). + // The Agent Login itself does survive that restart, on the home volume: + // it is the tmux process that is gone, not the sign-in. if (gatewayAgentId == null || session.status != AgentSessionStatus.RUNNING) { return RunnerSessionBindingResult.Unavailable( workspaceId = workspace.id, diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSessionBindingRouter.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSessionBindingRouter.kt index 5c17f65..110110b 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSessionBindingRouter.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSessionBindingRouter.kt @@ -1,8 +1,7 @@ package com.jorisjonkers.personalstack.agents.application.sessionbinding -import com.jorisjonkers.personalstack.agents.domain.model.AgentSession import com.jorisjonkers.personalstack.agents.domain.model.AgentSessionId -import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentKind +import com.jorisjonkers.personalstack.agents.domain.model.Workspace import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceId import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceKind import com.jorisjonkers.personalstack.agents.domain.port.AgentSessionRepository @@ -13,9 +12,9 @@ import org.springframework.stereotype.Component /** * Picks the [RunnerSessionBindingService] the same way * [com.jorisjonkers.personalstack.agents.infrastructure.integration.AgentGatewayClientRouter] - * picks a gateway: a Shell Agent Session in a Scratch Workspace binds - * in-container; everything else keeps going through [RunnerSessionBinder] - * and its Pod provisioning. + * picks a gateway: any Agent Session in a Scratch Workspace binds + * in-container; a Repo-backed Workspace keeps going through + * [RunnerSessionBinder] and its Pod provisioning. */ @Primary @Component @@ -27,7 +26,7 @@ class RunnerSessionBindingRouter( ) : RunnerSessionBindingService { override fun start(request: StartRunnerSessionBindingInput): RunnerSessionBindingResult { val workspace = workspaces.findById(request.workspaceId) ?: return podBinding.start(request) - return targetFor(workspace.kind, request.kind).start(request) + return targetFor(workspace).start(request) } override fun restart(request: RestartRunnerSessionBindingInput): RunnerSessionBindingResult = @@ -36,30 +35,40 @@ class RunnerSessionBindingRouter( override fun ensureBound(request: EnsureRunnerSessionBoundInput): RunnerSessionBindingResult { val session = sessions.findById(request.sessionId) ?: return podBinding.ensureBound(request) val workspaceId = request.workspaceId ?: session.workspaceId - return (targetForExisting(workspaceId, session) ?: podBinding).ensureBound(request) + return (targetForWorkspace(workspaceId) ?: podBinding).ensureBound(request) } + // The session lookup is a guard, not a routing input: since #64 the Agent + // Kind no longer picks the binder, but a request naming a session that does + // not exist still belongs to the Pod binder, which is where that error is + // already shaped. private fun targetForExisting( workspaceId: WorkspaceId, sessionId: AgentSessionId, ): RunnerSessionBindingService? { - val session = sessions.findById(sessionId) ?: return null - return targetForExisting(workspaceId, session) + sessions.findById(sessionId) ?: return null + return targetForWorkspace(workspaceId) } - private fun targetForExisting( - workspaceId: WorkspaceId, - session: AgentSession, - ): RunnerSessionBindingService? { + private fun targetForWorkspace(workspaceId: WorkspaceId): RunnerSessionBindingService? { val workspace = workspaces.findById(workspaceId) ?: return null - return targetFor(workspace.kind, session.kind) + return targetFor(workspace) } - private fun targetFor( - workspaceKind: WorkspaceKind, - agentKind: WorkspaceAgentKind, - ): RunnerSessionBindingService = - if (workspaceKind == WorkspaceKind.SCRATCH && agentKind == WorkspaceAgentKind.SHELL) { + // Every Agent Kind in a Scratch Workspace, not just Shell (#64): Claude and + // Codex read their Agent Login from the home volume, which only exists in + // this container. A Repo-backed Workspace still needs the Pod entrypoint's + // clone, so it stays on the Pod path until #67. + // + // podName is the second half of that test, and it is not redundant. A + // Scratch Workspace created before #62 can still hold a Claude or Codex + // session bound to a runner Pod, and those rows survive this deploy. + // Routing one here would send restart() into an UnsupportedOperationException + // and ensureBound() into the in-container workspace directory instead of the + // Pod's /workspace -- stranding a session that was working. A Workspace this + // container binds never provisions a Pod, so its podName stays null. + private fun targetFor(workspace: Workspace): RunnerSessionBindingService = + if (workspace.kind == WorkspaceKind.SCRATCH && workspace.podName == null) { inContainerBinding } else { podBinding diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/config/AgentRuntimeProperties.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/config/AgentRuntimeProperties.kt index dadf76f..bca8279 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/config/AgentRuntimeProperties.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/config/AgentRuntimeProperties.kt @@ -99,10 +99,6 @@ data class AgentRuntimeProperties( // starting with the `gh` wrapper degrading to a no-op. val githubAppBearerSecret: String = "github-app", val githubAppBearerSecretKey: String = "token-bearer", - // Shared bearer for the internal credential-ingest callback. Kept - // separate from githubAppTokenBearer so the login worker cannot mint - // GitHub App tokens if one secret is exposed. - val credentialIngestBearer: String = "", val durableSessionRetentionSeconds: Long = 604_800, val durableSessionCleanupBatchSize: Int = 25, // Scratch Workspace directory root inside this container; a Workspace's @@ -120,16 +116,11 @@ data class AgentRuntimeProperties( // Socket name for the single shared tmux server backing every in-container // Shell Agent Session — see infrastructure/shell/InContainerTmuxClient. val shellTmuxSocketName: String = "agents-api", - // In-cluster ClusterIP of the credential-worker that drives the - // Claude Code / Codex CLI `/login` flows and writes the resulting - // OAuth bundle to Vault. The worker is fronted by no edge route, so - // this address skips forward-auth entirely. - val credentialWorkerUrl: String = "http://agents-login-worker.agents-system.svc.cluster.local:8081", - // Shared internal token presented on every credential-worker call - // as the `x-internal-token` header. Sourced from the INTERNAL_TOKEN - // env var. Empty => the worker rejects every proxied request with a - // 401, so an unconfigured deployment never reaches the login flow. - val credentialWorkerToken: String = "", + // The `agent` user's home, where the Claude and Codex CLIs keep their own + // Agent Login (ADR 0002). Not the JVM's $HOME: the JVM runs as `api`, whose + // home is /app, while Agent Sessions run as `agent` through run-as-agent. + // A path that is not mounted means "not signed in yet", never a bad start. + val agentHome: String = "/home/agent", val setups: List = listOf( AgentSetupProperties( diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/config/SecurityConfig.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/config/SecurityConfig.kt index d7d7499..efb1c2e 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/config/SecurityConfig.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/config/SecurityConfig.kt @@ -27,15 +27,6 @@ class SecurityConfig { addUrlPatterns("/api/v1/internal/github/*") order = 0 } - - @Bean - fun credentialInternalBearerFilterRegistration( - props: AgentRuntimeProperties, - ): FilterRegistrationBean = - FilterRegistrationBean(InternalBearerAuthFilter(props.credentialIngestBearer)).apply { - addUrlPatterns("/api/v1/internal/credentials") - order = 0 - } } class XUserIdFilter : OncePerRequestFilter() { diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/domain/model/AgentOauthCredential.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/domain/model/AgentOauthCredential.kt deleted file mode 100644 index b40f9cf..0000000 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/domain/model/AgentOauthCredential.kt +++ /dev/null @@ -1,38 +0,0 @@ -package com.jorisjonkers.personalstack.agents.domain.model - -import java.time.Instant - -/** Which agent CLI a stored login credential is for. */ -enum class AgentCredentialProvider { - CLAUDE, - CODEX, - ; - - companion object { - fun fromRaw(value: String): AgentCredentialProvider? = - when (value.trim().lowercase()) { - "claude" -> CLAUDE - "codex" -> CODEX - else -> null - } - } -} - -/** - * A user's captured login credential for one provider. `payload` carries the - * provider-specific fields (Claude: `oauth_token`; Codex: `auth_json` + - * `config_toml`). `valid` reflects the most recent provider probe, or null - * when a fresh or inconclusive credential has not been verified. - * - * The payload is secret: it is never logged or returned to the browser. Only - * the per-workspace runner injection reads it. - */ -data class AgentOauthCredential( - val userId: String, - val provider: AgentCredentialProvider, - val payload: Map, - val valid: Boolean?, - val validatedAt: Instant?, - val updatedAt: Instant, - val updatedBy: String, -) diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/domain/port/AgentCredentialRepository.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/domain/port/AgentCredentialRepository.kt deleted file mode 100644 index 4b271a5..0000000 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/domain/port/AgentCredentialRepository.kt +++ /dev/null @@ -1,39 +0,0 @@ -package com.jorisjonkers.personalstack.agents.domain.port - -import com.jorisjonkers.personalstack.agents.domain.model.AgentCredentialProvider -import com.jorisjonkers.personalstack.agents.domain.model.AgentOauthCredential - -/** - * Persistence for per-user agent login credentials, keyed by - * `(userId, provider)`. Replaces the Vault `agents/{claude,codex}-oauth` - * paths. Implementations store the payload as-is (it is secret) and never log - * it. - */ -interface AgentCredentialRepository { - /** Upsert the captured credential, resetting validity until the next probe. */ - fun upsert(credential: AgentOauthCredential): AgentOauthCredential - - /** The user's credential for a provider, or null when none is stored. */ - fun find( - userId: String, - provider: AgentCredentialProvider, - ): AgentOauthCredential? - - /** Mark the most recent probe result for a stored credential. */ - fun markValidity( - userId: String, - provider: AgentCredentialProvider, - valid: Boolean, - ) - - /** Per-provider presence/validity summary for a user (no payload). */ - fun statusFor(userId: String): List - - data class CredentialStatus( - val provider: AgentCredentialProvider, - val stored: Boolean, - val valid: Boolean?, - val validatedAt: java.time.Instant?, - val updatedAt: java.time.Instant?, - ) -} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/domain/port/AgentLoginStore.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/domain/port/AgentLoginStore.kt new file mode 100644 index 0000000..25f4887 --- /dev/null +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/domain/port/AgentLoginStore.kt @@ -0,0 +1,27 @@ +package com.jorisjonkers.personalstack.agents.domain.port + +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentKind + +/** + * Whether a provider has an Agent Login available to Agent Sessions. + * + * ADR 0002: an Agent Login is a property of the user, not of a Workspace. + * The user signs in once from a terminal in an Agent Session, the CLI writes + * its own login files under `$HOME`, and the home volume keeps them across + * restarts. Nothing here captures, stores, validates or injects a + * credential — the CLI owns its own login end to end, and this only reports + * whether one is there, so agents-ui can show the sign-in hint instead of an + * error. + */ +interface AgentLoginStore { + /** Whether [kind]'s CLI has a login on the home volume. */ + fun isPresent(kind: WorkspaceAgentKind): Boolean + + /** Presence for every Agent Kind that has a provider login, in a stable order. */ + fun statuses(): List + + data class AgentLoginStatus( + val kind: WorkspaceAgentKind, + val present: Boolean, + ) +} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/credentials/CredentialValidator.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/credentials/CredentialValidator.kt deleted file mode 100644 index e60d7be..0000000 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/credentials/CredentialValidator.kt +++ /dev/null @@ -1,49 +0,0 @@ -package com.jorisjonkers.personalstack.agents.infrastructure.credentials - -import com.jorisjonkers.personalstack.agents.domain.model.AgentCredentialProvider -import org.springframework.stereotype.Component - -enum class CredentialValidationResult { - VALID, - EXPLICIT_INVALID, - UNKNOWN, -} - -@Component -class CredentialValidator { - fun validate( - provider: AgentCredentialProvider, - payload: Map, - ): CredentialValidationResult = - if (payloadMatches(provider, payload)) { - CredentialValidationResult.UNKNOWN - } else { - CredentialValidationResult.EXPLICIT_INVALID - } - - fun fromHttpStatus(statusCode: Int): CredentialValidationResult = - when (statusCode) { - in HTTP_OK_MIN..HTTP_OK_MAX -> CredentialValidationResult.VALID - HTTP_UNAUTHORIZED, HTTP_FORBIDDEN -> CredentialValidationResult.EXPLICIT_INVALID - else -> CredentialValidationResult.UNKNOWN - } - - private fun payloadMatches( - provider: AgentCredentialProvider, - payload: Map, - ): Boolean = - when (provider) { - AgentCredentialProvider.CLAUDE -> - payload["credentials_json"].isPresent() || payload["oauth_token"].isPresent() - AgentCredentialProvider.CODEX -> payload["auth_json"].isPresent() - } - - private fun String?.isPresent(): Boolean = !isNullOrBlank() - - companion object { - private const val HTTP_OK_MIN = 200 - private const val HTTP_OK_MAX = 299 - private const val HTTP_UNAUTHORIZED = 401 - private const val HTTP_FORBIDDEN = 403 - } -} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/AgentGatewayClientRouter.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/AgentGatewayClientRouter.kt index 4016b3a..9af5158 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/AgentGatewayClientRouter.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/AgentGatewayClientRouter.kt @@ -89,23 +89,29 @@ class AgentGatewayClientRouter( headlessJobId: String, ): AgentGatewayClient.HeadlessJob = target(workspace).pollHeadlessJob(workspace, headlessJobId) - /** Spawn has no session yet, so it routes on the Agent Kind it is about to start. */ + /** + * Spawn has no session yet. Since #64 the Agent Kind no longer selects a + * gateway -- Claude and Codex run in this container too -- so this is the + * same test RunnerSessionBindingRouter applies, and the two must not drift. + */ private fun target( workspace: Workspace, - kind: WorkspaceAgentKind, - ): AgentGatewayClient = - if (workspace.kind == WorkspaceKind.SCRATCH && kind == WorkspaceAgentKind.SHELL) { - inContainerGateway - } else { - podGateway - } + @Suppress("UNUSED_PARAMETER") kind: WorkspaceAgentKind, + ): AgentGatewayClient = target(workspace) private fun target( workspace: Workspace, gatewayAgentId: String, ): AgentGatewayClient = if (registry.find(workspace.id, gatewayAgentId) != null) inContainerGateway else podGateway - /** Workspace-wide, no session named: only a Scratch Workspace runs in this container. */ + /** + * Workspace-wide, no session named. A Scratch Workspace runs in this + * container, unless it is an older one that still has a runner Pod bound. + */ private fun target(workspace: Workspace): AgentGatewayClient = - if (workspace.kind == WorkspaceKind.SCRATCH) inContainerGateway else podGateway + if (workspace.kind == WorkspaceKind.SCRATCH && workspace.podName == null) { + inContainerGateway + } else { + podGateway + } } diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/HttpCredentialWorkerClient.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/HttpCredentialWorkerClient.kt deleted file mode 100644 index 1b9522d..0000000 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/HttpCredentialWorkerClient.kt +++ /dev/null @@ -1,125 +0,0 @@ -package com.jorisjonkers.personalstack.agents.infrastructure.integration - -import com.jorisjonkers.personalstack.agents.config.AgentRuntimeProperties -import org.springframework.stereotype.Component -import org.springframework.web.client.RestClient - -/** - * Thin REST adapter for the internal credential-worker - * ([AgentRuntimeProperties.credentialWorkerUrl]). The worker owns the - * Claude Code / Codex CLI `/login` lifecycle and the Vault write; this - * client only relays one HTTP call per verb and presents the shared - * `x-internal-token` header on each. - * - * Worker 4xx/5xx responses surface as [RestClientResponseException] so - * the controller can map the worker's status (409 busy, 400 bad input, - * 404 unknown session) onto its own response. The token is never logged. - */ -@Component -class HttpCredentialWorkerClient( - private val restClient: RestClient, - private val props: AgentRuntimeProperties, -) { - /** Worker session-status payload. Mirrors src/worker/session.ts `SessionStatus`. */ - data class SessionStatus( - val id: String, - val provider: String, - val phase: String, - val authorizeUrl: String? = null, - val deviceCode: String? = null, - val verificationUrl: String? = null, - val needsRedirectUrl: Boolean = false, - val message: String? = null, - val error: String? = null, - val updatedAt: String? = null, - ) - - private data class StartBody( - val provider: String, - val updatedBy: String, - ) - - private data class RedirectBody( - val url: String, - ) - - /** Worker ack for redirect/cancel. */ - data class OkResult( - val ok: Boolean = false, - val error: String? = null, - ) - - /** Non-secret summary of one provider's stored credential. */ - data class CredentialStatus( - val exists: Boolean = false, - val version: Int = 0, - val updatedAt: String? = null, - val updatedBy: String? = null, - val schemaVersion: String? = null, - ) - - /** Stored-credential status for both providers (worker GET /status). */ - data class StoredStatus( - val claude: CredentialStatus = CredentialStatus(), - val codex: CredentialStatus = CredentialStatus(), - ) - - fun start( - provider: String, - updatedBy: String, - ): SessionStatus = - restClient - .post() - .uri("${base()}/sessions") - .header(INTERNAL_TOKEN_HEADER, props.credentialWorkerToken) - .body(StartBody(provider = provider, updatedBy = updatedBy)) - .retrieve() - .body(SessionStatus::class.java) - ?: error("empty response from credential worker /sessions") - - fun storedStatus(): StoredStatus = - restClient - .get() - .uri("${base()}/status") - .header(INTERNAL_TOKEN_HEADER, props.credentialWorkerToken) - .retrieve() - .body(StoredStatus::class.java) - ?: error("empty response from credential worker /status") - - fun status(sessionId: String): SessionStatus = - restClient - .get() - .uri("${base()}/sessions/$sessionId") - .header(INTERNAL_TOKEN_HEADER, props.credentialWorkerToken) - .retrieve() - .body(SessionStatus::class.java) - ?: error("empty response from credential worker /sessions/$sessionId") - - fun submitRedirect( - sessionId: String, - url: String, - ): OkResult = - restClient - .post() - .uri("${base()}/sessions/$sessionId/redirect") - .header(INTERNAL_TOKEN_HEADER, props.credentialWorkerToken) - .body(RedirectBody(url = url)) - .retrieve() - .body(OkResult::class.java) - ?: error("empty response from credential worker redirect") - - fun cancel(sessionId: String): OkResult = - restClient - .post() - .uri("${base()}/sessions/$sessionId/cancel") - .header(INTERNAL_TOKEN_HEADER, props.credentialWorkerToken) - .retrieve() - .body(OkResult::class.java) - ?: error("empty response from credential worker cancel") - - private fun base(): String = props.credentialWorkerUrl.trim().trimEnd('/') - - private companion object { - const val INTERNAL_TOKEN_HEADER = "x-internal-token" - } -} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/InContainerAgentGatewayClient.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/InContainerAgentGatewayClient.kt index 914a247..a89d40d 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/InContainerAgentGatewayClient.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/InContainerAgentGatewayClient.kt @@ -47,15 +47,13 @@ class InContainerAgentGatewayClient( private val log = LoggerFactory.getLogger(InContainerAgentGatewayClient::class.java) override fun spawnAgent(request: AgentGatewayClient.SpawnAgentRequest): AgentGatewayClient.GatewayAgent { - require(request.kind == WorkspaceAgentKind.SHELL) { - "the in-container gateway only runs Shell Agent Sessions; requested kind=${request.kind}" - } + val command = commandFor(request.kind) val workspace = request.workspace val cwd = request.workspacePath ?: directories.ensureCreated(workspace.id).toString() val id = UUID.randomUUID().toString().substring(0, ID_PREVIEW_CHARS) val tmuxSessionName = "agent-${workspace.id.short()}-$id" val logFile = sessionLogFile(cwd, id) - tmux.newSession(tmuxSessionName, SHELL_COMMAND, cwd) + tmux.newSession(tmuxSessionName, command, cwd) tmux.startPipeToFile(tmuxSessionName, logFile) registry.put( ShellSession( @@ -67,8 +65,8 @@ class InContainerAgentGatewayClient( createdAt = Instant.now(), ), ) - log.info("spawned shell agent {} ({}) in {}", id, tmuxSessionName, cwd) - return AgentGatewayClient.GatewayAgent(id = id, kind = WorkspaceAgentKind.SHELL, cwd = cwd) + log.info("spawned {} agent {} ({}) in {}", request.kind, id, tmuxSessionName, cwd) + return AgentGatewayClient.GatewayAgent(id = id, kind = request.kind, cwd = cwd) } override fun stopAgent( @@ -200,7 +198,6 @@ class InContainerAgentGatewayClient( private companion object { const val ID_PREVIEW_CHARS = 8 const val SESSIONS_SUBDIR = ".agent-sessions" - val SHELL_COMMAND = listOf("/bin/bash", "-l") // A clone runs over the network, unlike every other command this // client shells out through; the 30s default in RunAsAgentCommandRunner @@ -211,3 +208,18 @@ class InContainerAgentGatewayClient( const val CREDENTIAL_USE_HTTP_PATH_CONFIG = "credential.useHttpPath=true" } } + +// What tmux runs for each Agent Kind. Bare `claude` and `codex` are the +// interactive TUIs: when the home volume holds no Agent Login yet, the CLI's +// own sign-in prompt is what the user completes from this terminal, which is +// the only way a login is ever created (ADR 0002). The headless forms -- +// `claude -p`, `codex exec` -- belong to #66. +// +// `when` over the enum rather than a map: a new Agent Kind then fails the +// build here instead of failing a request at runtime. +private fun commandFor(kind: WorkspaceAgentKind): List = + when (kind) { + WorkspaceAgentKind.SHELL -> listOf("/bin/bash", "-l") + WorkspaceAgentKind.CLAUDE -> listOf("claude") + WorkspaceAgentKind.CODEX -> listOf("codex") + } diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/Fabric8AgentRunnerOrchestrator.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/Fabric8AgentRunnerOrchestrator.kt index f892d98..e5dbad0 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/Fabric8AgentRunnerOrchestrator.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/Fabric8AgentRunnerOrchestrator.kt @@ -6,7 +6,6 @@ import com.jorisjonkers.personalstack.agents.domain.model.AgentSetupVersion import com.jorisjonkers.personalstack.agents.domain.model.RunnerSetupProvisioningSpec import com.jorisjonkers.personalstack.agents.domain.model.RunnerState import com.jorisjonkers.personalstack.agents.domain.model.Workspace -import com.jorisjonkers.personalstack.agents.domain.port.AgentCredentialRepository import com.jorisjonkers.personalstack.agents.domain.port.AgentRunnerOrchestrator import com.jorisjonkers.personalstack.agents.domain.port.RepositoryRepository import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceRepositoryRepository @@ -32,20 +31,20 @@ import java.util.concurrent.TimeUnit * HTTPS via a git credential helper, so repository access stays * scoped to repos the App is installed on. * - * Pod construction, credential management, and state-reading are delegated to - * RunnerPodSpecBuilder, RunnerCredentialSecretManager, and RunnerStateReader. + * Pod construction and state-reading are delegated to RunnerPodSpecBuilder + * and RunnerStateReader. There is no credential management left: an Agent + * Login lives on the home volume (ADR 0002), so a runner Pod is never handed + * one. */ @Component @Profile("!system-test") class Fabric8AgentRunnerOrchestrator( private val client: KubernetesClient, private val props: AgentRuntimeProperties, - credentialsProvider: ObjectProvider, workspaceRepos: ObjectProvider, repositories: ObjectProvider, ) : AgentRunnerOrchestrator { private val log = LoggerFactory.getLogger(Fabric8AgentRunnerOrchestrator::class.java) - private val credentials = RunnerCredentialSecretManager(client, props, credentialsProvider) private val podSpec = RunnerPodSpecBuilder(props, workspaceRepos, repositories, ::ownReleaseVersion) private val stateReader = RunnerStateReader() @@ -64,8 +63,7 @@ class Fabric8AgentRunnerOrchestrator( pvc = "workspace-$short", service = "agent-runner-$short", ) - val credentialSecret = credentials.ensureCredentialSecret(workspace, short) - applyResources(workspace, setup, runnerGeneration, names, credentialSecret) + applyResources(workspace, setup, runnerGeneration, names) val endpoint = "http://${names.service}.${props.namespace}.svc.cluster.local:${setup.gatewayPort}" log.info( "provisioned runner pod {} for workspace {} using setup {}@{} generation {}", @@ -87,7 +85,6 @@ class Fabric8AgentRunnerOrchestrator( setup: RunnerSetupProvisioningSpec, runnerGeneration: Long, names: RunnerResourceNames, - credentialSecret: RunnerCredentialSecretManager.CredentialSecret?, ) { client .persistentVolumeClaims() @@ -97,7 +94,7 @@ class Fabric8AgentRunnerOrchestrator( client .pods() .inNamespace(props.namespace) - .resource(podSpec.pod(workspace, setup, runnerGeneration, names, credentialSecret)) + .resource(podSpec.pod(workspace, setup, runnerGeneration, names)) .serverSideApply() client .services() @@ -154,10 +151,15 @@ class Fabric8AgentRunnerOrchestrator( .inNamespace(props.namespace) .withName("workspace-$short") .delete() + // #64 stopped creating this Secret, but destroy() still deletes it: the + // ones an earlier release already wrote hold OAuth tokens, and nothing + // else reaps them. The name is inlined because the manager that owned + // it is gone; this is the last reader of that naming scheme, and it + // goes with the Pod path in #67. client .secrets() .inNamespace(props.namespace) - .withName(credentials.credentialSecretName(short)) + .withName("agent-runner-credentials-$short") .delete() log.info("destroyed runner pod and PVC for workspace {}", workspace.id) } diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerCredentialSecretManager.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerCredentialSecretManager.kt deleted file mode 100644 index 819b559..0000000 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerCredentialSecretManager.kt +++ /dev/null @@ -1,115 +0,0 @@ -package com.jorisjonkers.personalstack.agents.infrastructure.k8s - -import com.jorisjonkers.personalstack.agents.config.AgentRuntimeProperties -import com.jorisjonkers.personalstack.agents.domain.model.AgentCredentialProvider -import com.jorisjonkers.personalstack.agents.domain.model.Workspace -import com.jorisjonkers.personalstack.agents.domain.port.AgentCredentialRepository -import io.fabric8.kubernetes.api.model.SecretBuilder -import io.fabric8.kubernetes.client.KubernetesClient -import org.slf4j.LoggerFactory -import org.springframework.beans.factory.ObjectProvider -import java.util.Base64 - -/** - * Manages the per-workspace credential Secret in Kubernetes. Extracted from - * Fabric8AgentRunnerOrchestrator to keep that class below the TooManyFunctions - * and LargeClass thresholds. - */ -internal class RunnerCredentialSecretManager( - private val client: KubernetesClient, - private val props: AgentRuntimeProperties, - private val credentialsProvider: ObjectProvider, -) { - private val log = LoggerFactory.getLogger(RunnerCredentialSecretManager::class.java) - - internal data class CredentialSecret( - val name: String, - val hasClaude: Boolean, - val hasClaudeCredentialsJson: Boolean, - val hasClaudeAccountJson: Boolean, - val hasCodex: Boolean, - val hasCodexConfig: Boolean, - ) - - fun credentialSecretName(short: String): String = "agent-runner-credentials-$short" - - fun ensureCredentialSecret( - workspace: Workspace, - short: String, - ): CredentialSecret? { - val name = credentialSecretName(short) - val data = - credentialSecretData(workspace) - ?: run { - client - .secrets() - .inNamespace(props.namespace) - .withName(name) - .delete() - return null - } - client - .secrets() - .inNamespace(props.namespace) - .resource( - SecretBuilder() - .withNewMetadata() - .withName(name) - .withNamespace(props.namespace) - .withLabels( - mapOf( - "app.kubernetes.io/part-of" to "agent-runner", - "agent-runner/workspace-id" to short, - ), - ).endMetadata() - .withType("Opaque") - .withData(data) - .build(), - ).serverSideApply() - return CredentialSecret( - name = name, - hasClaude = data.containsKey("claude_oauth_token"), - hasClaudeCredentialsJson = data.containsKey("claude_credentials_json"), - hasClaudeAccountJson = data.containsKey("claude_account_json"), - hasCodex = data.containsKey("codex_auth_json"), - hasCodexConfig = data.containsKey("codex_config_toml"), - ) - } - - private fun credentialSecretData(workspace: Workspace): Map? { - val owner = workspace.ownerUserId?.takeIf { it.isNotBlank() } ?: return null - val store = credentialsProvider.ifAvailable ?: return null - val data = - buildMap { - val claude = loadCredential(store, owner, AgentCredentialProvider.CLAUDE) - claude?.payload?.get("oauth_token")?.takeIf { it.isNotBlank() }?.let { - put("claude_oauth_token", b64(it)) - } - claude?.payload?.get("credentials_json")?.takeIf { it.isNotBlank() }?.let { - put("claude_credentials_json", b64(it)) - } - claude?.payload?.get("account_json")?.takeIf { it.isNotBlank() }?.let { - put("claude_account_json", b64(it)) - } - val codex = loadCredential(store, owner, AgentCredentialProvider.CODEX) - val codexAuth = codex?.payload?.get("auth_json")?.takeIf { it.isNotBlank() } - val codexConfig = codex?.payload?.get("config_toml")?.takeIf { it.isNotBlank() } - if (codexAuth != null) { - put("codex_auth_json", b64(codexAuth)) - codexConfig?.let { put("codex_config_toml", b64(it)) } - } - } - return data.takeIf { it.isNotEmpty() } - } - - private fun loadCredential( - store: AgentCredentialRepository, - owner: String, - provider: AgentCredentialProvider, - ) = runCatching { store.find(owner, provider) } - .onFailure { log.warn("could not load {} credential for workspace owner", provider) } - .getOrNull() - ?.takeUnless { it.valid == false } - - private fun b64(s: String): String = Base64.getEncoder().encodeToString(s.toByteArray()) -} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerPodSpecBuilder.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerPodSpecBuilder.kt index 2428582..2e54934 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerPodSpecBuilder.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerPodSpecBuilder.kt @@ -6,7 +6,6 @@ import com.jorisjonkers.personalstack.agents.domain.model.RunnerState import com.jorisjonkers.personalstack.agents.domain.model.Workspace import com.jorisjonkers.personalstack.agents.domain.port.RepositoryRepository import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceRepositoryRepository -import com.jorisjonkers.personalstack.agents.infrastructure.k8s.RunnerCredentialSecretManager.CredentialSecret import io.fabric8.kubernetes.api.model.Container import io.fabric8.kubernetes.api.model.ContainerBuilder import io.fabric8.kubernetes.api.model.ContainerPortBuilder @@ -31,8 +30,6 @@ import io.fabric8.kubernetes.api.model.VolumeMountBuilder import org.springframework.beans.factory.ObjectProvider private const val DOCKER_SOCKET_VOLUME = "docker-socket" -private const val AGENT_CREDENTIALS_VOLUME = "agent-credentials" -private const val AGENT_CREDENTIALS_MOUNT = "/var/run/secrets/agents/credentials" /** * Builds Kubernetes resource specs (Pod, PVC, Service) for a workspace runner. @@ -75,7 +72,6 @@ internal class RunnerPodSpecBuilder( setup: RunnerSetupProvisioningSpec, runnerGeneration: Long, names: RunnerResourceNames, - credentialSecret: CredentialSecret?, ): Pod = PodBuilder() .withMetadata(podMetadata(workspace, setup, runnerGeneration, names)) @@ -92,8 +88,8 @@ internal class RunnerPodSpecBuilder( .withRestartPolicy("Always") .withSecurityContext(podSecurityContext(setup)) .withInitContainers(agentStateInitContainer(setup)) - .withContainers(agentRunnerContainer(workspace, setup, runnerGeneration, credentialSecret)) - .withVolumes(volumes.podVolumes(names.pvc, credentialSecret, setup)) + .withContainers(agentRunnerContainer(workspace, setup, runnerGeneration)) + .withVolumes(volumes.podVolumes(names.pvc, setup)) .endSpec() .build() @@ -166,7 +162,6 @@ internal class RunnerPodSpecBuilder( workspace: Workspace, setup: RunnerSetupProvisioningSpec, runnerGeneration: Long, - credentialSecret: CredentialSecret?, ): Container = ContainerBuilder() .withName("agent-runner") @@ -179,8 +174,8 @@ internal class RunnerPodSpecBuilder( .withName("gateway") .withContainerPort(setup.gatewayPort) .build(), - ).withEnv(env.podEnv(workspace, setup, runnerGeneration, credentialSecret)) - .withVolumeMounts(volumes.podVolumeMounts(setup, credentialSecret)) + ).withEnv(env.podEnv(workspace, setup, runnerGeneration)) + .withVolumeMounts(volumes.podVolumeMounts(setup)) // Startup probe gates liveness + readiness until the gateway's // JVM has finished its cold start. Without it the liveness probe // (failureThreshold 3 x 10s ~= 30s, no initial delay) killed the @@ -273,14 +268,12 @@ internal class RunnerContainerEnvBuilder( workspace: Workspace, setup: RunnerSetupProvisioningSpec, runnerGeneration: Long, - credentialSecret: CredentialSecret?, ) = buildList { addAll(baseEnv()) addAll(setupEnv(setup, runnerGeneration)) addAll(dockerEnv(setup)) addAll(knowledgeEnv(setup)) addAll(githubAppTokenEnv()) - addAll(agentCredentialEnv(credentialSecret)) addAll(repoEnv(workspace)) } @@ -426,59 +419,6 @@ internal class RunnerContainerEnvBuilder( .endValueFrom() .build(), ) - - private fun agentCredentialEnv(credentialSecret: CredentialSecret?) = - if (credentialSecret == null) { - emptyList() - } else { - buildList { - if (credentialSecret.hasClaudeCredentialsJson) { - add( - EnvVarBuilder() - .withName("AGENT_CLAUDE_CREDENTIALS_FILE") - .withValue("$AGENT_CREDENTIALS_MOUNT/claude_credentials_json") - .build(), - ) - } - if (credentialSecret.hasClaudeAccountJson) { - add( - EnvVarBuilder() - .withName("AGENT_CLAUDE_ACCOUNT_FILE") - .withValue("$AGENT_CREDENTIALS_MOUNT/claude_account_json") - .build(), - ) - } - if (credentialSecret.hasClaude && !credentialSecret.hasClaudeCredentialsJson) { - add( - EnvVarBuilder() - .withName("CLAUDE_CODE_OAUTH_TOKEN") - .withNewValueFrom() - .withNewSecretKeyRef() - .withName(credentialSecret.name) - .withKey("claude_oauth_token") - .endSecretKeyRef() - .endValueFrom() - .build(), - ) - } - if (credentialSecret.hasCodex) { - add( - EnvVarBuilder() - .withName("AGENT_CODEX_AUTH_JSON_FILE") - .withValue("$AGENT_CREDENTIALS_MOUNT/codex_auth_json") - .build(), - ) - } - if (credentialSecret.hasCodexConfig) { - add( - EnvVarBuilder() - .withName("AGENT_CODEX_CONFIG_TOML_FILE") - .withValue("$AGENT_CREDENTIALS_MOUNT/codex_config_toml") - .build(), - ) - } - } - } } /** @@ -487,48 +427,35 @@ internal class RunnerContainerEnvBuilder( * TooManyFunctions threshold. */ internal class RunnerVolumeSpecBuilder { - fun podVolumeMounts( - setup: RunnerSetupProvisioningSpec, - credentialSecret: CredentialSecret?, - ) = buildList { - add(VolumeMountBuilder().withName("workspace").withMountPath("/workspace").build()) - addAll(agentStateVolumeMounts()) - if (credentialSecret != null) { + fun podVolumeMounts(setup: RunnerSetupProvisioningSpec) = + buildList { + add(VolumeMountBuilder().withName("workspace").withMountPath("/workspace").build()) + addAll(agentStateVolumeMounts()) + if (setup.dockerSocketEnabled) { + add( + VolumeMountBuilder() + .withName(DOCKER_SOCKET_VOLUME) + .withMountPath(setup.dockerSocketPath) + .build(), + ) + } + // Declarative MCP server set; the entrypoint seeds it into + // ~/.claude.json. Optional volume, so an absent ConfigMap + // leaves the runner with no managed MCP servers. add( VolumeMountBuilder() - .withName(AGENT_CREDENTIALS_VOLUME) - .withMountPath(AGENT_CREDENTIALS_MOUNT) + .withName("mcp-config") + .withMountPath(setup.mcpDir) .withReadOnly(true) .build(), ) } - if (setup.dockerSocketEnabled) { - add( - VolumeMountBuilder() - .withName(DOCKER_SOCKET_VOLUME) - .withMountPath(setup.dockerSocketPath) - .build(), - ) - } - // Declarative MCP server set; the entrypoint seeds it into - // ~/.claude.json. Optional volume, so an absent ConfigMap - // leaves the runner with no managed MCP servers. - add( - VolumeMountBuilder() - .withName("mcp-config") - .withMountPath(setup.mcpDir) - .withReadOnly(true) - .build(), - ) - } fun podVolumes( workspacePvc: String, - credentialSecret: CredentialSecret?, setup: RunnerSetupProvisioningSpec, ) = buildList { add(pvcVolume("workspace", workspacePvc)) - credentialSecret?.let { add(agentCredentialsVolume(it.name)) } dockerSocketVolume(setup)?.let(::add) add(mcpConfigVolume(setup)) } @@ -589,14 +516,6 @@ internal class RunnerVolumeSpecBuilder { .build() } - private fun agentCredentialsVolume(credentialSecret: String): Volume = - VolumeBuilder() - .withName(AGENT_CREDENTIALS_VOLUME) - .withNewSecret() - .withSecretName(credentialSecret) - .endSecret() - .build() - private fun mcpConfigVolume(setup: RunnerSetupProvisioningSpec): Volume = VolumeBuilder() .withName("mcp-config") diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/login/HomeVolumeAgentLoginStore.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/login/HomeVolumeAgentLoginStore.kt new file mode 100644 index 0000000..7c3f1bb --- /dev/null +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/login/HomeVolumeAgentLoginStore.kt @@ -0,0 +1,55 @@ +package com.jorisjonkers.personalstack.agents.infrastructure.login + +import com.jorisjonkers.personalstack.agents.config.AgentRuntimeProperties +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentKind +import com.jorisjonkers.personalstack.agents.domain.port.AgentLoginStore +import org.springframework.beans.factory.annotation.Autowired +import org.springframework.stereotype.Component +import java.nio.file.Files +import java.nio.file.Path + +/** + * Reads Agent Login presence straight off the `agent` user's home volume. + * + * The paths are the CLIs' own, not ours: Claude writes + * `~/.claude/.credentials.json` and Codex writes `~/.codex/auth.json`, and + * both rewrite them on every token refresh. Nothing in agents-api opens + * them — presence is the whole answer, and reading the contents would put a + * live OAuth token somewhere it does not need to be. + * + * The home directory is a parameter rather than `System.getenv("HOME")`: + * the JVM runs as `api`, whose home is `/app`, while Agent Sessions run as + * `agent` through `run-as-agent`, which sets `HOME=/home/agent`. Reading the + * API's own `HOME` would answer about the wrong user's home every time. + */ +@Component +class HomeVolumeAgentLoginStore( + private val agentHome: Path, +) : AgentLoginStore { + // The Path constructor is what the tests use; Spring takes this one. + @Autowired + constructor(props: AgentRuntimeProperties) : this(Path.of(props.agentHome)) + + override fun isPresent(kind: WorkspaceAgentKind): Boolean { + val relative = LOGIN_FILES[kind] ?: return false + val file = agentHome.resolve(relative) + // An absent home volume is "not signed in yet", never a failure: a + // developer laptop and the integration tests have no such mount + // (fleet-infra#326). Size, not existence, because a truncated write + // leaves an empty file that no CLI can authenticate with. + return runCatching { Files.isRegularFile(file) && Files.size(file) > 0 }.getOrDefault(false) + } + + override fun statuses(): List = + LOGIN_FILES.keys.map { AgentLoginStore.AgentLoginStatus(kind = it, present = isPresent(it)) } + + private companion object { + // Ordered: this is what agents-ui renders, and a stable order keeps + // the list from reshuffling between polls. + val LOGIN_FILES = + linkedMapOf( + WorkspaceAgentKind.CLAUDE to ".claude/.credentials.json", + WorkspaceAgentKind.CODEX to ".codex/auth.json", + ) + } +} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/persistence/JooqAgentCredentialRepository.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/persistence/JooqAgentCredentialRepository.kt deleted file mode 100644 index ff0db5e..0000000 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/persistence/JooqAgentCredentialRepository.kt +++ /dev/null @@ -1,123 +0,0 @@ -package com.jorisjonkers.personalstack.agents.infrastructure.persistence - -import com.fasterxml.jackson.databind.ObjectMapper -import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper -import com.fasterxml.jackson.module.kotlin.readValue -import com.jorisjonkers.personalstack.agents.domain.model.AgentCredentialProvider -import com.jorisjonkers.personalstack.agents.domain.model.AgentOauthCredential -import com.jorisjonkers.personalstack.agents.domain.port.AgentCredentialRepository -import org.jooq.DSLContext -import org.jooq.Record -import org.jooq.impl.DSL -import org.jooq.impl.SQLDataType -import org.springframework.stereotype.Repository -import java.time.OffsetDateTime -import java.time.ZoneOffset - -@Repository -class JooqAgentCredentialRepository( - private val dsl: DSLContext, -) : AgentCredentialRepository { - private val json: ObjectMapper = jacksonObjectMapper() - - override fun upsert(credential: AgentOauthCredential): AgentOauthCredential { - val payload = json.writeValueAsString(credential.payload) - val updatedAt = credential.updatedAt.atOffset(ZoneOffset.UTC) - dsl - .insertInto(TABLE) - .set(USER_ID, credential.userId) - .set(PROVIDER, credential.provider.name) - .set(PAYLOAD, payload) - .set(VALID, null as Boolean?) - .set(VALIDATED_AT, null as OffsetDateTime?) - .set(UPDATED_AT, updatedAt) - .set(UPDATED_BY, credential.updatedBy) - .onConflict(USER_ID, PROVIDER) - .doUpdate() - .set(PAYLOAD, payload) - .set(VALID, null as Boolean?) - .set(VALIDATED_AT, null as OffsetDateTime?) - .set(UPDATED_AT, updatedAt) - .set(UPDATED_BY, credential.updatedBy) - .execute() - return credential.copy(valid = null, validatedAt = null) - } - - override fun find( - userId: String, - provider: AgentCredentialProvider, - ): AgentOauthCredential? = - dsl - .selectFrom(TABLE) - .where(USER_ID.eq(userId)) - .and(PROVIDER.eq(provider.name)) - .fetchOne() - ?.toCredential() - - override fun markValidity( - userId: String, - provider: AgentCredentialProvider, - valid: Boolean, - ) { - dsl - .update(TABLE) - .set(VALID, valid) - .set(VALIDATED_AT, OffsetDateTime.now(ZoneOffset.UTC)) - .where(USER_ID.eq(userId)) - .and(PROVIDER.eq(provider.name)) - .execute() - } - - override fun statusFor(userId: String): List = - AgentCredentialProvider.entries.map { provider -> - val rec = - dsl - .select(PROVIDER, VALID, VALIDATED_AT, UPDATED_AT) - .from(TABLE) - .where(USER_ID.eq(userId)) - .and(PROVIDER.eq(provider.name)) - .fetchOne() - if (rec == null) { - AgentCredentialRepository.CredentialStatus( - provider = provider, - stored = false, - valid = null, - validatedAt = null, - updatedAt = null, - ) - } else { - AgentCredentialRepository.CredentialStatus( - provider = provider, - stored = true, - valid = rec.get(VALID), - validatedAt = rec.get(VALIDATED_AT)?.toInstant(), - updatedAt = rec.get(UPDATED_AT)?.toInstant(), - ) - } - } - - private fun Record.toCredential(): AgentOauthCredential { - val payload: Map = - this.get(PAYLOAD)?.let { json.readValue>(it) }.orEmpty() - return AgentOauthCredential( - userId = this.get(USER_ID), - provider = AgentCredentialProvider.valueOf(this.get(PROVIDER)), - payload = payload, - valid = this.get(VALID), - validatedAt = this.get(VALIDATED_AT)?.toInstant(), - updatedAt = this.get(UPDATED_AT).toInstant(), - updatedBy = this.get(UPDATED_BY), - ) - } - - private companion object { - val TABLE = DSL.table(DSL.name("agent_oauth_credentials")) - val USER_ID = DSL.field(DSL.name("user_id"), SQLDataType.VARCHAR) - val PROVIDER = DSL.field(DSL.name("provider"), SQLDataType.VARCHAR) - val PAYLOAD = DSL.field(DSL.name("payload"), SQLDataType.CLOB) - val VALID = DSL.field(DSL.name("token_valid"), SQLDataType.BOOLEAN) - val VALIDATED_AT = DSL.field(DSL.name("validated_at"), SQLDataType.TIMESTAMPWITHTIMEZONE) - val UPDATED_AT = DSL.field(DSL.name("updated_at"), SQLDataType.TIMESTAMPWITHTIMEZONE) - val UPDATED_BY = DSL.field(DSL.name("updated_by"), SQLDataType.VARCHAR) - } -} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/AgentLoginController.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/AgentLoginController.kt new file mode 100644 index 0000000..8a4fafd --- /dev/null +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/AgentLoginController.kt @@ -0,0 +1,44 @@ +package com.jorisjonkers.personalstack.agents.infrastructure.web + +import com.jorisjonkers.personalstack.agents.domain.port.AgentLoginStore +import com.jorisjonkers.personalstack.agents.infrastructure.web.dto.AgentLoginStatusResponse +import io.swagger.v3.oas.annotations.Operation +import org.springframework.http.ResponseEntity +import org.springframework.web.bind.annotation.GetMapping +import org.springframework.web.bind.annotation.RequestMapping +import org.springframework.web.bind.annotation.RestController + +/** + * Reports which providers have an Agent Login on the home volume. + * + * This replaces `/api/v1/credentials/status`, which reported what agents-api + * had *captured and stored* for a user. Under ADR 0002 it stores nothing: the + * CLI writes its own login when the user signs in from a terminal in an Agent + * Session, and this only says whether that has happened yet. + * + * No `X-User-Id`. The old endpoint keyed on the forward-auth identity because + * credentials were per-user rows; an Agent Login is a property of the home + * volume, and there is one of those per container. Taking a user header would + * imply a per-user answer this cannot give. + * + * A missing Agent Login is not an error and does not block a session: the CLI + * prompts for sign-in itself, and this is what lets agents-ui say so first. + * + * SCOPE, and agents-ui must respect it: this answers for Agent Sessions that + * run in *this container*, which today means a Scratch Workspace. A Repo-backed + * Workspace still runs its sessions in a runner Pod, which has no access to + * this volume and, since #64, no injected credential either -- so `present = + * true` here says nothing about one. agents-ui must show the hint for every + * Repo-backed Workspace regardless of what this reports, until #67 moves that + * path in-container and the two agree. + */ +@RestController +@RequestMapping("/api/v1/agent-logins") +class AgentLoginController( + private val logins: AgentLoginStore, +) { + @GetMapping + @Operation(summary = "Report which providers have an Agent Login on the home volume") + fun status(): ResponseEntity = + ResponseEntity.ok(AgentLoginStatusResponse.of(logins.statuses())) +} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/CredentialController.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/CredentialController.kt deleted file mode 100644 index c202c36..0000000 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/CredentialController.kt +++ /dev/null @@ -1,134 +0,0 @@ -package com.jorisjonkers.personalstack.agents.infrastructure.web - -import com.fasterxml.jackson.databind.ObjectMapper -import com.jorisjonkers.personalstack.agents.domain.port.AgentCredentialRepository -import com.jorisjonkers.personalstack.agents.infrastructure.integration.HttpCredentialWorkerClient -import com.jorisjonkers.personalstack.agents.infrastructure.web.dto.CredentialActionResponse -import com.jorisjonkers.personalstack.agents.infrastructure.web.dto.CredentialSessionResponse -import com.jorisjonkers.personalstack.agents.infrastructure.web.dto.StartCredentialSessionRequest -import com.jorisjonkers.personalstack.agents.infrastructure.web.dto.StoredCredentialStatusResponse -import com.jorisjonkers.personalstack.agents.infrastructure.web.dto.SubmitRedirectUrlRequest -import com.jorisjonkers.personalstack.common.web.ProblemDetail -import io.swagger.v3.oas.annotations.Operation -import jakarta.validation.Valid -import org.springframework.http.HttpStatus -import org.springframework.http.ResponseEntity -import org.springframework.web.bind.annotation.GetMapping -import org.springframework.web.bind.annotation.PathVariable -import org.springframework.web.bind.annotation.PostMapping -import org.springframework.web.bind.annotation.RequestBody -import org.springframework.web.bind.annotation.RequestHeader -import org.springframework.web.bind.annotation.RequestMapping -import org.springframework.web.bind.annotation.RestController -import org.springframework.web.client.RestClientResponseException -import java.net.URI - -/** - * Browser-facing proxy onto the internal credential-worker. The whole - * agents surface already sits behind forward-auth (the AGENTS - * permission), so this controller adds no auth of its own beyond the - * standard `X-User-Id` identity the edge injects. - * - * Worker 4xx responses (409 busy, 400 bad input, 404 unknown session) - * are relayed back with the worker's own status as an RFC 7807 - * ProblemDetail whose `detail` carries the worker's message; any other - * failure falls through to the global handler. The worker emits a bare - * `{"error":"…"}` body — relaying that verbatim left the browser client - * (which reads `detail`/`title`/`status`) rendering an opaque - * "HTTP undefined", hiding the real reason a login could not start. - */ -@RestController -@RequestMapping("/api/v1/credentials") -class CredentialController( - private val worker: HttpCredentialWorkerClient, - private val credentials: AgentCredentialRepository, -) { - // Not injected: the OpenAPI web-mvc slice that exports the spec does not - // expose an ObjectMapper bean, so a constructor dependency would break it. - private val objectMapper = ObjectMapper() - - @Deprecated( - "Agent Login moves to the home volume (#64). Removed once agents-ui no longer calls it.", - ) - @GetMapping("/status") - @Operation(summary = "Report what credentials are currently stored for each provider") - fun storedStatus( - @RequestHeader("X-User-Id") userId: String, - ): ResponseEntity = - ResponseEntity.ok(StoredCredentialStatusResponse.of(credentials.statusFor(userId))) - - @Deprecated( - "Agent Login moves to the home volume (#64). Removed once agents-ui no longer calls it.", - ) - @PostMapping("/sessions") - @Operation(summary = "Start a CLI re-authentication session for Claude or Codex") - fun start( - @RequestHeader("X-User-Id") userId: String, - @Valid @RequestBody req: StartCredentialSessionRequest, - ): ResponseEntity<*> = - relay { - // updatedBy is the forward-auth identity, never client-chosen. - val status = worker.start(provider = req.provider, updatedBy = userId) - ResponseEntity.status(HttpStatus.CREATED).body(CredentialSessionResponse.of(status)) - } - - @Deprecated( - "Agent Login moves to the home volume (#64). Removed once agents-ui no longer calls it.", - ) - @GetMapping("/sessions/{id}") - @Operation(summary = "Get the current status of a re-authentication session") - fun status( - @PathVariable id: String, - ): ResponseEntity<*> = - relay { - ResponseEntity.ok(CredentialSessionResponse.of(worker.status(id))) - } - - @Deprecated( - "Agent Login moves to the home volume (#64). Removed once agents-ui no longer calls it.", - ) - @PostMapping("/sessions/{id}/redirect") - @Operation(summary = "Submit the Claude post-approval redirect URL back to a session") - fun redirect( - @PathVariable id: String, - @Valid @RequestBody req: SubmitRedirectUrlRequest, - ): ResponseEntity<*> = - relay { - ResponseEntity.ok(CredentialActionResponse.of(worker.submitRedirect(id, req.url))) - } - - @Deprecated( - "Agent Login moves to the home volume (#64). Removed once agents-ui no longer calls it.", - ) - @PostMapping("/sessions/{id}/cancel") - @Operation(summary = "Cancel an in-flight re-authentication session") - fun cancel( - @PathVariable id: String, - ): ResponseEntity<*> = - relay { - ResponseEntity.ok(CredentialActionResponse.of(worker.cancel(id))) - } - - private inline fun relay(block: () -> ResponseEntity<*>): ResponseEntity<*> = - try { - block() - } catch (ex: RestClientResponseException) { - ResponseEntity - .status(ex.statusCode) - .body( - ProblemDetail( - type = URI.create("https://jorisjonkers.dev/errors/credential-worker"), - title = "Credential worker request failed", - status = ex.statusCode.value(), - detail = workerDetail(ex.responseBodyAsString), - instance = null, - ), - ) - } - - /** Pull the worker's `{"error":"…"}` message out for the ProblemDetail `detail`. */ - private fun workerDetail(body: String): String = - runCatching { objectMapper.readTree(body).path("error").asText("") } - .getOrDefault("") - .ifBlank { "credential worker request failed" } -} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/InternalCredentialController.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/InternalCredentialController.kt deleted file mode 100644 index fb2ae84..0000000 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/InternalCredentialController.kt +++ /dev/null @@ -1,81 +0,0 @@ -package com.jorisjonkers.personalstack.agents.infrastructure.web - -import com.jorisjonkers.personalstack.agents.domain.model.AgentCredentialProvider -import com.jorisjonkers.personalstack.agents.domain.model.AgentOauthCredential -import com.jorisjonkers.personalstack.agents.domain.port.AgentCredentialRepository -import com.jorisjonkers.personalstack.agents.infrastructure.credentials.CredentialValidationResult -import com.jorisjonkers.personalstack.agents.infrastructure.credentials.CredentialValidator -import com.jorisjonkers.personalstack.agents.infrastructure.web.dto.InternalCredentialIngestRequest -import com.jorisjonkers.personalstack.agents.infrastructure.web.dto.InternalCredentialIngestResponse -import io.swagger.v3.oas.annotations.Hidden -import jakarta.validation.Valid -import org.springframework.http.HttpStatus -import org.springframework.http.ResponseEntity -import org.springframework.web.bind.annotation.PostMapping -import org.springframework.web.bind.annotation.RequestBody -import org.springframework.web.bind.annotation.RequestMapping -import org.springframework.web.bind.annotation.RestController -import java.time.Instant - -@Hidden -@RestController -@RequestMapping("/api/v1/internal/credentials") -class InternalCredentialController( - private val credentials: AgentCredentialRepository, - private val validator: CredentialValidator, -) { - @PostMapping - fun ingest( - @Valid @RequestBody request: InternalCredentialIngestRequest, - ): ResponseEntity { - val provider = - runCatching { AgentCredentialProvider.valueOf(request.provider) } - .getOrNull() - ?: return ResponseEntity.badRequest().build() - if (!payloadMatches(provider, request.payload)) { - return ResponseEntity.badRequest().build() - } - - credentials.upsert( - AgentOauthCredential( - userId = request.userId, - provider = provider, - payload = request.payload, - valid = null, - validatedAt = null, - updatedAt = Instant.now(), - // The credential owner is the only legitimate updater on this - // internal ingest path; a client-supplied updatedBy would let a - // caller forge authorship, so derive it from the owner identity. - updatedBy = request.userId, - ), - ) - - val validation = - runCatching { validator.validate(provider, request.payload) } - .getOrDefault(CredentialValidationResult.UNKNOWN) - when (validation) { - CredentialValidationResult.VALID -> credentials.markValidity(request.userId, provider, true) - CredentialValidationResult.EXPLICIT_INVALID -> credentials.markValidity(request.userId, provider, false) - CredentialValidationResult.UNKNOWN -> Unit - } - - return ResponseEntity - .status(HttpStatus.ACCEPTED) - .body(InternalCredentialIngestResponse(provider = provider.name, status = validation.name)) - } - - private fun payloadMatches( - provider: AgentCredentialProvider, - payload: Map, - ): Boolean = - when (provider) { - AgentCredentialProvider.CLAUDE -> - payload["credentials_json"].isPresent() || payload["oauth_token"].isPresent() - // config_toml is optional: `codex login` only writes auth.json, and the - // runner self-provisions a config.toml when none is injected. - AgentCredentialProvider.CODEX -> payload["auth_json"].isPresent() - } - - private fun String?.isPresent(): Boolean = !isNullOrBlank() -} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/dto/AgentLoginDtos.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/dto/AgentLoginDtos.kt new file mode 100644 index 0000000..e216814 --- /dev/null +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/dto/AgentLoginDtos.kt @@ -0,0 +1,30 @@ +package com.jorisjonkers.personalstack.agents.infrastructure.web.dto + +import com.jorisjonkers.personalstack.agents.domain.port.AgentLoginStore + +/** + * Whether a provider has an Agent Login available, so agents-ui can show the + * sign-in hint next to an Agent Session rather than an error. + * + * Presence only. There is deliberately no field carrying, or derived from, + * the login itself: under ADR 0002 the CLI owns its own credential end to + * end and agents-api never reads it. + */ +data class AgentLoginResponse( + val kind: String, + val present: Boolean, +) { + companion object { + fun of(status: AgentLoginStore.AgentLoginStatus) = + AgentLoginResponse(kind = status.kind.name.lowercase(), present = status.present) + } +} + +data class AgentLoginStatusResponse( + val logins: List, +) { + companion object { + fun of(statuses: List) = + AgentLoginStatusResponse(logins = statuses.map(AgentLoginResponse::of)) + } +} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/dto/CredentialDtos.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/dto/CredentialDtos.kt deleted file mode 100644 index 1edf1b4..0000000 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/dto/CredentialDtos.kt +++ /dev/null @@ -1,125 +0,0 @@ -package com.jorisjonkers.personalstack.agents.infrastructure.web.dto - -import com.jorisjonkers.personalstack.agents.domain.model.AgentCredentialProvider -import com.jorisjonkers.personalstack.agents.domain.port.AgentCredentialRepository -import com.jorisjonkers.personalstack.agents.infrastructure.integration.HttpCredentialWorkerClient -import jakarta.validation.constraints.NotBlank -import jakarta.validation.constraints.Pattern -import java.time.Instant - -/** Start a CLI re-authentication session for one provider. */ -data class StartCredentialSessionRequest( - @field:NotBlank - @field:Pattern(regexp = "claude|codex", message = "provider must be \"claude\" or \"codex\"") - val provider: String, -) - -/** Submit the Claude post-approval redirect URL back to a session. */ -data class SubmitRedirectUrlRequest( - @field:NotBlank val url: String, -) - -/** - * Credential-worker session status, mirroring the worker's - * `SessionStatus`. `phase` is one of starting, awaiting_url, - * awaiting_device, finalizing, succeeded, failed, cancelled. Secret- - * shaped material is already redacted by the worker. - */ -data class CredentialSessionResponse( - val id: String, - val provider: String, - val phase: String, - val authorizeUrl: String?, - val deviceCode: String?, - val verificationUrl: String?, - val needsRedirectUrl: Boolean, - val message: String?, - val error: String?, - val updatedAt: String?, -) { - companion object { - fun of(s: HttpCredentialWorkerClient.SessionStatus) = - CredentialSessionResponse( - id = s.id, - provider = s.provider, - phase = s.phase, - authorizeUrl = s.authorizeUrl, - deviceCode = s.deviceCode, - verificationUrl = s.verificationUrl, - needsRedirectUrl = s.needsRedirectUrl, - message = s.message, - error = s.error, - updatedAt = s.updatedAt, - ) - } -} - -/** Acknowledgement for redirect / cancel relays. */ -data class CredentialActionResponse( - val ok: Boolean, - val error: String?, -) { - companion object { - fun of(r: HttpCredentialWorkerClient.OkResult) = CredentialActionResponse(ok = r.ok, error = r.error) - } -} - -data class InternalCredentialIngestRequest( - @field:NotBlank val userId: String, - @field:NotBlank val provider: String, - val payload: Map = emptyMap(), -) - -data class InternalCredentialIngestResponse( - val provider: String, - val status: String, -) - -data class StoredCredentialStatusResponse( - val claude: StoredProviderCredentialStatus, - val codex: StoredProviderCredentialStatus, -) { - companion object { - fun of(statuses: List): StoredCredentialStatusResponse { - val byProvider = statuses.associateBy { it.provider } - return StoredCredentialStatusResponse( - claude = StoredProviderCredentialStatus.of(byProvider[AgentCredentialProvider.CLAUDE]), - codex = StoredProviderCredentialStatus.of(byProvider[AgentCredentialProvider.CODEX]), - ) - } - } -} - -data class StoredProviderCredentialStatus( - val exists: Boolean, - val state: String, - val valid: Boolean?, - val validatedAt: Instant?, - val updatedAt: Instant?, -) { - companion object { - fun of(status: AgentCredentialRepository.CredentialStatus?): StoredProviderCredentialStatus { - if (status == null || !status.stored) { - return StoredProviderCredentialStatus( - exists = false, - state = "absent", - valid = null, - validatedAt = null, - updatedAt = null, - ) - } - return StoredProviderCredentialStatus( - exists = true, - state = - when (status.valid) { - true -> "usable" - false -> "invalid" - null -> "unvalidated" - }, - valid = status.valid, - validatedAt = status.validatedAt, - updatedAt = status.updatedAt, - ) - } - } -} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionChecker.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionChecker.kt index bd4b01e..297c72a 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionChecker.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionChecker.kt @@ -17,7 +17,6 @@ import com.jorisjonkers.personalstack.agents.domain.model.AgentSessionId import com.jorisjonkers.personalstack.agents.domain.model.AgentSessionStatus import com.jorisjonkers.personalstack.agents.domain.model.RunnerSetupOperation import com.jorisjonkers.personalstack.agents.domain.model.Workspace -import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentKind import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceKind import com.jorisjonkers.personalstack.agents.domain.port.AgentSessionRepository import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceRepository @@ -179,7 +178,7 @@ internal class AttachPreconditionChecker( ): AttachOutcome { val gatewayAgentId = session.gatewayAgentId val endpoint = workspace.gatewayEndpoint - val local = isInContainerShellSession(workspace, session) + val local = isInContainerSession(workspace) return when { isSetupTransitionInProgress(session, workspace) -> AttachOutcome.Rejected( @@ -208,10 +207,15 @@ internal class AttachPreconditionChecker( } } - private fun isInContainerShellSession( - workspace: Workspace, - session: AgentSession, - ): Boolean = workspace.kind == WorkspaceKind.SCRATCH && session.kind == WorkspaceAgentKind.SHELL + // Must mirror RunnerSessionBindingRouter.targetFor exactly. A session this + // container binds has no gateway endpoint -- a Scratch Workspace never + // provisions a Pod -- so a checker that disagrees rejects the attach with + // "workspace has no gateway endpoint". For a Claude or Codex session that + // is fatal rather than cosmetic: the terminal is the only place an Agent + // Login can be created, so an unopenable one leaves the user with no way to + // sign in at all (ADR 0002). + private fun isInContainerSession(workspace: Workspace): Boolean = + workspace.kind == WorkspaceKind.SCRATCH && workspace.podName == null private fun isSetupTransitionInProgress( agentSession: AgentSession, diff --git a/api/src/main/resources/application.yml b/api/src/main/resources/application.yml index 1881121..43dcae0 100644 --- a/api/src/main/resources/application.yml +++ b/api/src/main/resources/application.yml @@ -220,14 +220,12 @@ agent-runtime: github-app-token-url: ${GITHUB_APP_TOKEN_URL:http://agents-api.agents-system.svc.cluster.local:8082/api/v1/internal/github/installation-token} github-app-bearer-secret: ${GITHUB_APP_BEARER_SECRET:github-app} github-app-bearer-secret-key: ${GITHUB_APP_BEARER_SECRET_KEY:token-bearer} - credential-ingest-bearer: ${CREDENTIAL_INGEST_BEARER:} - # Credential-worker proxy. The worker drives the Claude Code / Codex - # CLI `/login` flows and writes the captured OAuth bundle to Vault; - # agents-api only relays the operator's browser interactions to it. - # The token is the shared `x-internal-token` (sourced from - # INTERNAL_TOKEN); empty => the worker rejects every proxied call. - credential-worker-url: ${CREDENTIAL_WORKER_URL:http://agents-login-worker.agents-system.svc.cluster.local:8081} - credential-worker-token: ${INTERNAL_TOKEN:} + # The `agent` user's home, where the Claude and Codex CLIs keep their own + # Agent Login (ADR 0002). Read-only to agents-api: it reports whether a login + # is there and never opens one. Not $HOME -- the JVM runs as `api`, whose home + # is /app, while Agent Sessions run as `agent` through run-as-agent. A path + # that is not mounted means "not signed in yet", never a failed start. + agent-home: ${AGENT_RUNTIME_AGENT_HOME:/home/agent} # RAG retrieval / capture configuration. # rag.enabled is a deprecated master toggle (false disables both retrieval and diff --git a/api/src/main/resources/db/migration/V28__drop_agent_oauth_credentials.sql b/api/src/main/resources/db/migration/V28__drop_agent_oauth_credentials.sql new file mode 100644 index 0000000..d415ac7 --- /dev/null +++ b/api/src/main/resources/db/migration/V28__drop_agent_oauth_credentials.sql @@ -0,0 +1,14 @@ +-- Agent Login moves to the home volume (ADR 0002, #64). The user signs in once +-- from a terminal in an Agent Session; the CLI writes its own login files under +-- $HOME and the home volume keeps them across restarts. agents-api no longer +-- captures, stores, validates or injects a credential, so the table V18 created +-- has no reader and no writer left. +-- +-- Forward-only, and deliberately so: the rows are OAuth tokens that nothing +-- reads any more, and keeping them would leave live credentials in the database +-- purely so a rollback could use a mechanism this release removes. Rolling +-- agents-api back past this migration therefore does not restore the old login +-- path -- the agents-login worker and its endpoints are gone too. The recovery +-- for a bad release is to sign in again from an Agent Session, which is the +-- whole point of putting the login on a volume. +DROP TABLE IF EXISTS agent_oauth_credentials; diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/InContainerSessionBindingServiceTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/InContainerSessionBindingServiceTest.kt index cda226f..1ae4d6e 100644 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/InContainerSessionBindingServiceTest.kt +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/InContainerSessionBindingServiceTest.kt @@ -58,9 +58,38 @@ class InContainerSessionBindingServiceTest { verify { sessionStatus.publishStatus(saved.captured) } } + // #64: Claude and Codex bind here too. The kind is carried straight through + // to the gateway, which picks the CLI to run; the binder itself is + // kind-agnostic, so a wrong kind reaching the gateway is the gateway's + // error to raise, not a second guard here. @Test - fun `start rejects a non-Shell kind`() { + fun `start binds a Claude Agent Session and passes the kind to the gateway`() { every { workspaces.findById(workspaceId) } returns workspace + every { gateway.spawnAgent(any()) } returns + AgentGatewayClient.GatewayAgent(id = "abc12345", kind = WorkspaceAgentKind.CLAUDE, cwd = "/workspaces/x") + val saved = slot() + every { sessions.save(capture(saved)) } answers { saved.captured } + + val result = + service.start( + StartRunnerSessionBindingInput( + workspaceId = workspaceId, + sessionId = sessionId, + kind = WorkspaceAgentKind.CLAUDE, + ), + ) + + assertThat(result).isInstanceOf(RunnerSessionBindingResult.Bound::class.java) + assertThat(saved.captured.kind).isEqualTo(WorkspaceAgentKind.CLAUDE) + verify { + gateway.spawnAgent(match { it.kind == WorkspaceAgentKind.CLAUDE }) + } + } + + @Test + fun `start still refuses a Repo-backed Workspace`() { + every { workspaces.findById(workspaceId) } returns + workspace.copy(kind = WorkspaceKind.REPO_BACKED) org.junit.jupiter.api.assertThrows { service.start( diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSessionBindingRouterTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSessionBindingRouterTest.kt index e996899..cc5af73 100644 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSessionBindingRouterTest.kt +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSessionBindingRouterTest.kt @@ -40,8 +40,11 @@ class RunnerSessionBindingRouterTest { verify(exactly = 0) { podBinding.start(any()) } } + // #64: Claude and Codex join Shell in-container, so the CLI reads its Agent + // Login off the home volume. Before this they went to a runner Pod, which + // got its login injected as a Secret -- the mechanism ADR 0002 replaces. @Test - fun `start for a Claude session in a Scratch workspace still goes to the pod binder`() { + fun `start for a Claude session in a Scratch workspace binds in-container`() { val workspaceId = WorkspaceId.random() every { workspaces.findById(workspaceId) } returns workspace(workspaceId, WorkspaceKind.SCRATCH) val request = @@ -53,6 +56,47 @@ class RunnerSessionBindingRouterTest { router.start(request) + verify { inContainerBinding.start(request) } + verify(exactly = 0) { podBinding.start(any()) } + } + + @Test + fun `start for a Codex session in a Scratch workspace binds in-container`() { + val workspaceId = WorkspaceId.random() + every { workspaces.findById(workspaceId) } returns workspace(workspaceId, WorkspaceKind.SCRATCH) + val request = + StartRunnerSessionBindingInput( + workspaceId = workspaceId, + sessionId = AgentSessionId.random(), + kind = WorkspaceAgentKind.CODEX, + ) + + router.start(request) + + verify { inContainerBinding.start(request) } + verify(exactly = 0) { podBinding.start(any()) } + } + + // A Repo-backed Workspace keeps going to the Pod binder, for every Agent + // Kind. The in-container binder does not clone a repo -- that is the Pod + // entrypoint's job until #67 moves the whole path across -- so routing one + // here would start an Agent Session in an empty directory. The cost is that + // a Claude session in a Repo-backed Workspace has no Agent Login during the + // transition, because #64 removes the Secret injection that used to supply + // one. #67 closes it. + @Test + fun `start for a Claude session in a repo-backed workspace still goes to the pod binder`() { + val workspaceId = WorkspaceId.random() + every { workspaces.findById(workspaceId) } returns workspace(workspaceId, WorkspaceKind.REPO_BACKED) + val request = + StartRunnerSessionBindingInput( + workspaceId = workspaceId, + sessionId = AgentSessionId.random(), + kind = WorkspaceAgentKind.CLAUDE, + ) + + router.start(request) + verify { podBinding.start(request) } verify(exactly = 0) { inContainerBinding.start(any()) } } @@ -74,6 +118,29 @@ class RunnerSessionBindingRouterTest { verify(exactly = 0) { inContainerBinding.start(any()) } } + // A Scratch Workspace created before #62 can still hold a Claude or Codex + // session bound to a runner Pod, and those rows survive this deploy. Sending + // one to the in-container binder strands it: restart() throws + // UnsupportedOperationException, and ensureBound() answers with this + // container's workspace directory instead of the Pod's /workspace. + @Test + fun `start for a Scratch workspace that still has a runner Pod goes to the pod binder`() { + val workspaceId = WorkspaceId.random() + every { workspaces.findById(workspaceId) } returns + workspace(workspaceId, WorkspaceKind.SCRATCH).copy(podName = "agent-runner-legacy") + val request = + StartRunnerSessionBindingInput( + workspaceId = workspaceId, + sessionId = AgentSessionId.random(), + kind = WorkspaceAgentKind.CLAUDE, + ) + + router.start(request) + + verify { podBinding.start(request) } + verify(exactly = 0) { inContainerBinding.start(any()) } + } + @Test fun `ensureBound resolves the workspace and session kind to pick the binder`() { val workspaceId = WorkspaceId.random() diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/config/AgentRuntimePropertiesBindingTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/config/AgentRuntimePropertiesBindingTest.kt index 620fa34..f0f928d 100644 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/config/AgentRuntimePropertiesBindingTest.kt +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/config/AgentRuntimePropertiesBindingTest.kt @@ -131,13 +131,13 @@ class AgentRuntimePropertiesBindingTest { mapOf( "agent-runtime.durable-session-retention-seconds" to "3600", "agent-runtime.durable-session-cleanup-batch-size" to "7", - "agent-runtime.credential-ingest-bearer" to "credential-secret", + "agent-runtime.agent-home" to "/tmp/agent-home", ), ) assertThat(props.durableSessionRetentionSeconds).isEqualTo(3600) assertThat(props.durableSessionCleanupBatchSize).isEqualTo(7) - assertThat(props.credentialIngestBearer).isEqualTo("credential-secret") + assertThat(props.agentHome).isEqualTo("/tmp/agent-home") } @Test diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/config/InternalBearerAuthFilterTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/config/InternalBearerAuthFilterTest.kt index 0ab4760..bdef4a8 100644 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/config/InternalBearerAuthFilterTest.kt +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/config/InternalBearerAuthFilterTest.kt @@ -59,15 +59,13 @@ class InternalBearerAuthFilterTest { codexCredentialsPvc = "codex-credentials", githubDeployKeySecret = "github-deploy-key", githubAppTokenBearer = "github-bearer", - credentialIngestBearer = "credential-bearer", ) val config = SecurityConfig() val github = config.githubInternalBearerFilterRegistration(props) - val credentials = config.credentialInternalBearerFilterRegistration(props) + // The credential-ingest filter went with the endpoint it guarded (#64); + // the GitHub token filter is the only internal bearer left. assertThat(github.urlPatterns).containsExactly("/api/v1/internal/github/*") - assertThat(credentials.urlPatterns).containsExactly("/api/v1/internal/credentials") - assertThat(github.filter).isNotSameAs(credentials.filter) } } diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/credentials/CredentialValidatorTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/credentials/CredentialValidatorTest.kt deleted file mode 100644 index 5440b90..0000000 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/credentials/CredentialValidatorTest.kt +++ /dev/null @@ -1,19 +0,0 @@ -package com.jorisjonkers.personalstack.agents.infrastructure.credentials - -import org.assertj.core.api.Assertions.assertThat -import org.junit.jupiter.api.Test - -class CredentialValidatorTest { - private val validator = CredentialValidator() - - @Test - fun `http status mapping only treats success and explicit auth failures as conclusive`() { - assertThat(validator.fromHttpStatus(200)).isEqualTo(CredentialValidationResult.VALID) - assertThat(validator.fromHttpStatus(204)).isEqualTo(CredentialValidationResult.VALID) - assertThat(validator.fromHttpStatus(401)).isEqualTo(CredentialValidationResult.EXPLICIT_INVALID) - assertThat(validator.fromHttpStatus(403)).isEqualTo(CredentialValidationResult.EXPLICIT_INVALID) - assertThat(validator.fromHttpStatus(400)).isEqualTo(CredentialValidationResult.UNKNOWN) - assertThat(validator.fromHttpStatus(429)).isEqualTo(CredentialValidationResult.UNKNOWN) - assertThat(validator.fromHttpStatus(500)).isEqualTo(CredentialValidationResult.UNKNOWN) - } -} diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/InContainerAgentGatewayClientTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/InContainerAgentGatewayClientTest.kt index 50f5cea..0d651ac 100644 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/InContainerAgentGatewayClientTest.kt +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/InContainerAgentGatewayClientTest.kt @@ -55,15 +55,65 @@ class InContainerAgentGatewayClientTest { } } + // #64: Claude and Codex run in this container too, as `agent`, so the CLI + // reads the Agent Login off the home volume. Bare `claude` / `codex` is the + // interactive TUI — the one the user can sign in from when there is no + // login yet. `claude -p` and `codex exec` are the headless forms (#66). @Test - fun `spawnAgent rejects a non-Shell kind`() { - assertThrows { + fun `spawnAgent runs the Claude CLI in tmux for a Claude Agent Session`() { + every { directories.ensureCreated(workspaceId) } returns Path.of("/workspaces/$workspaceId") + + val agent = + client.spawnAgent( + AgentGatewayClient.SpawnAgentRequest(workspace = workspace(), kind = WorkspaceAgentKind.CLAUDE), + ) + + assertThat(agent.kind).isEqualTo(WorkspaceAgentKind.CLAUDE) + verify { + tmux.newSession( + match { it.startsWith("agent-${workspaceId.short()}-") }, + listOf("claude"), + "/workspaces/$workspaceId", + ) + } + } + + @Test + fun `spawnAgent runs the Codex CLI in tmux for a Codex Agent Session`() { + every { directories.ensureCreated(workspaceId) } returns Path.of("/workspaces/$workspaceId") + + val agent = client.spawnAgent( AgentGatewayClient.SpawnAgentRequest(workspace = workspace(), kind = WorkspaceAgentKind.CODEX), ) + + assertThat(agent.kind).isEqualTo(WorkspaceAgentKind.CODEX) + verify { + tmux.newSession( + match { it.startsWith("agent-${workspaceId.short()}-") }, + listOf("codex"), + "/workspaces/$workspaceId", + ) } } + // A missing Agent Login must not stop the session: the CLI's own sign-in + // prompt is where the user signs in, and agents-ui only adds a hint + // alongside it. Refusing to spawn would remove the one place a login can + // be created (ADR 0002). + @Test + fun `spawnAgent starts a Claude Agent Session even with no Agent Login on the home volume`() { + every { directories.ensureCreated(workspaceId) } returns Path.of("/workspaces/$workspaceId") + + val agent = + client.spawnAgent( + AgentGatewayClient.SpawnAgentRequest(workspace = workspace(), kind = WorkspaceAgentKind.CLAUDE), + ) + + assertThat(agent.id).isNotBlank() + verify { tmux.newSession(any(), listOf("claude"), any()) } + } + @Test fun `stopAgent kills the tmux session and drops it from the registry`() { every { directories.ensureCreated(workspaceId) } returns Path.of("/workspaces/$workspaceId") diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/login/HomeVolumeAgentLoginStoreTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/login/HomeVolumeAgentLoginStoreTest.kt new file mode 100644 index 0000000..521ac24 --- /dev/null +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/login/HomeVolumeAgentLoginStoreTest.kt @@ -0,0 +1,92 @@ +package com.jorisjonkers.personalstack.agents.infrastructure.login + +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentKind +import com.jorisjonkers.personalstack.agents.domain.port.AgentLoginStore +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir +import java.nio.file.Files +import java.nio.file.Path + +class HomeVolumeAgentLoginStoreTest { + @Test + fun `reports an Agent Login for each provider whose CLI has written its own login file`( + @TempDir home: Path, + ) { + writeLogin(home, ".claude/.credentials.json") + val store = HomeVolumeAgentLoginStore(home) + + assertThat(store.isPresent(WorkspaceAgentKind.CLAUDE)).isTrue() + assertThat(store.isPresent(WorkspaceAgentKind.CODEX)).isFalse() + } + + @Test + fun `reports a Codex Agent Login from its own auth file`( + @TempDir home: Path, + ) { + writeLogin(home, ".codex/auth.json") + val store = HomeVolumeAgentLoginStore(home) + + assertThat(store.isPresent(WorkspaceAgentKind.CODEX)).isTrue() + } + + // An empty file is what a half-written or truncated login leaves behind. + // Reporting it as present sends the user to a CLI that will fail rather + // than to the sign-in hint, which is the worse of the two wrong answers. + @Test + fun `does not report an Agent Login for an empty login file`( + @TempDir home: Path, + ) { + val file = home.resolve(".claude/.credentials.json") + Files.createDirectories(file.parent) + Files.writeString(file, "") + + assertThat(HomeVolumeAgentLoginStore(home).isPresent(WorkspaceAgentKind.CLAUDE)).isFalse() + } + + // Criterion 9 of fleet-infra#326: a developer laptop and the integration + // tests have no home volume mounted. An absent directory is "no login + // yet", never a failed start. + @Test + fun `reports no Agent Login when the home volume is not mounted at all`( + @TempDir dir: Path, + ) { + val store = HomeVolumeAgentLoginStore(dir.resolve("absent")) + + assertThat(store.isPresent(WorkspaceAgentKind.CLAUDE)).isFalse() + assertThat(store.isPresent(WorkspaceAgentKind.CODEX)).isFalse() + } + + // SHELL is an Agent Kind with no provider login at all. Asking is not an + // error, and the answer is never "sign in". + @Test + fun `reports no Agent Login requirement for a Shell Agent Session`( + @TempDir home: Path, + ) { + writeLogin(home, ".claude/.credentials.json") + + assertThat(HomeVolumeAgentLoginStore(home).isPresent(WorkspaceAgentKind.SHELL)).isFalse() + } + + @Test + fun `summarises every provider that can hold an Agent Login`( + @TempDir home: Path, + ) { + writeLogin(home, ".codex/auth.json") + + assertThat(HomeVolumeAgentLoginStore(home).statuses()) + .containsExactly( + AgentLoginStore.AgentLoginStatus(WorkspaceAgentKind.CLAUDE, present = false), + AgentLoginStore.AgentLoginStatus(WorkspaceAgentKind.CODEX, present = true), + ) + } + + private fun writeLogin( + home: Path, + relative: String, + ) { + val file = home.resolve(relative) + Files.createDirectories(file.parent) + Files.writeString(file, """{"token":"x"}""") + } +} diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/AgentLoginControllerTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/AgentLoginControllerTest.kt new file mode 100644 index 0000000..a7210aa --- /dev/null +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/AgentLoginControllerTest.kt @@ -0,0 +1,73 @@ +package com.jorisjonkers.personalstack.agents.infrastructure.web + +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentKind +import com.jorisjonkers.personalstack.agents.domain.port.AgentLoginStore +import com.jorisjonkers.personalstack.common.web.GlobalExceptionHandler +import io.mockk.every +import io.mockk.mockk +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.springframework.test.web.servlet.MockMvc +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get +import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath +import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status +import org.springframework.test.web.servlet.setup.MockMvcBuilders + +class AgentLoginControllerTest { + private val logins = mockk() + private lateinit var mockMvc: MockMvc + + @BeforeEach + fun setUp() { + mockMvc = + MockMvcBuilders + .standaloneSetup(AgentLoginController(logins)) + .setControllerAdvice(GlobalExceptionHandler()) + .build() + } + + @Test + fun `reports presence per provider`() { + every { logins.statuses() } returns + listOf( + AgentLoginStore.AgentLoginStatus(WorkspaceAgentKind.CLAUDE, present = true), + AgentLoginStore.AgentLoginStatus(WorkspaceAgentKind.CODEX, present = false), + ) + + mockMvc + .perform(get("/api/v1/agent-logins")) + .andExpect(status().isOk) + .andExpect(jsonPath("$.logins[0].kind").value("claude")) + .andExpect(jsonPath("$.logins[0].present").value(true)) + .andExpect(jsonPath("$.logins[1].kind").value("codex")) + .andExpect(jsonPath("$.logins[1].present").value(false)) + } + + // A response carrying anything derived from the login itself would put an + // OAuth token on the wire; presence is the entire contract. + @Test + fun `reports nothing but the provider and whether a login is present`() { + every { logins.statuses() } returns + listOf(AgentLoginStore.AgentLoginStatus(WorkspaceAgentKind.CLAUDE, present = true)) + + mockMvc + .perform(get("/api/v1/agent-logins")) + .andExpect(status().isOk) + .andExpect(jsonPath("$.logins[0].length()").value(2)) + } + + @Test + fun `reports an empty answer rather than failing when no provider has a login`() { + every { logins.statuses() } returns + listOf( + AgentLoginStore.AgentLoginStatus(WorkspaceAgentKind.CLAUDE, present = false), + AgentLoginStore.AgentLoginStatus(WorkspaceAgentKind.CODEX, present = false), + ) + + mockMvc + .perform(get("/api/v1/agent-logins")) + .andExpect(status().isOk) + .andExpect(jsonPath("$.logins[0].present").value(false)) + .andExpect(jsonPath("$.logins[1].present").value(false)) + } +} diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/CredentialControllerTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/CredentialControllerTest.kt deleted file mode 100644 index 81cdd49..0000000 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/CredentialControllerTest.kt +++ /dev/null @@ -1,280 +0,0 @@ -package com.jorisjonkers.personalstack.agents.infrastructure.web - -import com.fasterxml.jackson.databind.ObjectMapper -import com.jorisjonkers.personalstack.agents.domain.model.AgentCredentialProvider -import com.jorisjonkers.personalstack.agents.domain.port.AgentCredentialRepository -import com.jorisjonkers.personalstack.agents.infrastructure.integration.HttpCredentialWorkerClient -import com.jorisjonkers.personalstack.common.web.GlobalExceptionHandler -import io.mockk.every -import io.mockk.mockk -import io.mockk.slot -import io.mockk.verify -import org.junit.jupiter.api.BeforeEach -import org.junit.jupiter.api.Test -import org.springframework.http.HttpStatus -import org.springframework.http.MediaType -import org.springframework.test.web.servlet.MockMvc -import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get -import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post -import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath -import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status -import org.springframework.test.web.servlet.setup.MockMvcBuilders -import org.springframework.web.client.HttpClientErrorException -import java.time.Instant - -class CredentialControllerTest { - private val worker = mockk() - private val credentials = mockk() - private val objectMapper = ObjectMapper() - private lateinit var mockMvc: MockMvc - - @BeforeEach - fun setUp() { - mockMvc = - MockMvcBuilders - .standaloneSetup(CredentialController(worker, credentials)) - .setControllerAdvice(GlobalExceptionHandler()) - .build() - } - - private class WorkerStatusOptions { - var id: String = "sess-1" - var provider: String = "claude" - var phase: String = "starting" - var authorizeUrl: String? = null - var deviceCode: String? = null - var verificationUrl: String? = null - var needsRedirectUrl: Boolean = false - } - - private fun workerStatus(configure: WorkerStatusOptions.() -> Unit = {}) = - WorkerStatusOptions().apply(configure).let { options -> - HttpCredentialWorkerClient.SessionStatus( - id = options.id, - provider = options.provider, - phase = options.phase, - authorizeUrl = options.authorizeUrl, - deviceCode = options.deviceCode, - verificationUrl = options.verificationUrl, - needsRedirectUrl = options.needsRedirectUrl, - message = null, - error = null, - updatedAt = "2026-06-20T10:00:00Z", - ) - } - - @Test - fun `POST sessions starts a claude session and returns 201`() { - every { worker.start(provider = "claude", updatedBy = any()) } returns - workerStatus { - provider = "claude" - phase = "awaiting_url" - authorizeUrl = "https://claude.ai/authorize?x=1" - } - - mockMvc - .perform( - post("/api/v1/credentials/sessions") - .header("X-User-Id", "operator@example.com") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(mapOf("provider" to "claude"))), - ).andExpect(status().isCreated) - .andExpect(jsonPath("$.id").value("sess-1")) - .andExpect(jsonPath("$.provider").value("claude")) - .andExpect(jsonPath("$.phase").value("awaiting_url")) - .andExpect(jsonPath("$.authorizeUrl").value("https://claude.ai/authorize?x=1")) - } - - @Test - fun `POST sessions resolves updatedBy from the principal not the request body`() { - val updatedBy = slot() - every { worker.start(provider = "codex", updatedBy = capture(updatedBy)) } returns - workerStatus { provider = "codex" } - - mockMvc - .perform( - post("/api/v1/credentials/sessions") - .header("X-User-Id", "real-operator") - .contentType(MediaType.APPLICATION_JSON) - // A malicious client tries to spoof updatedBy via the body. - .content( - objectMapper.writeValueAsString( - mapOf("provider" to "codex", "updatedBy" to "attacker"), - ), - ), - ).andExpect(status().isCreated) - - // The worker must be told the forward-auth identity, never the body value. - verify { worker.start(provider = "codex", updatedBy = "real-operator") } - assert(updatedBy.captured == "real-operator") - } - - @Test - fun `POST sessions with an invalid provider returns 422 without calling the worker`() { - mockMvc - .perform( - post("/api/v1/credentials/sessions") - .header("X-User-Id", "operator") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(mapOf("provider" to "gemini"))), - ).andExpect(status().isUnprocessableContent) - - verify(exactly = 0) { worker.start(any(), any()) } - } - - @Test - fun `POST sessions relays a worker 409 as problem+json carrying the worker message`() { - every { worker.start(any(), any()) } throws - HttpClientErrorException.create( - HttpStatus.CONFLICT, - "Conflict", - org.springframework.http.HttpHeaders.EMPTY, - """{"error":"a login session is already in progress"}""".toByteArray(), - null, - ) - - mockMvc - .perform( - post("/api/v1/credentials/sessions") - .header("X-User-Id", "operator") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(mapOf("provider" to "claude"))), - ).andExpect(status().isConflict) - // The browser client reads detail/title/status; a bare {"error":…} - // body rendered "HTTP undefined" and hid the real reason. - .andExpect(jsonPath("$.status").value(409)) - .andExpect(jsonPath("$.title").value("Credential worker request failed")) - .andExpect(jsonPath("$.detail").value("a login session is already in progress")) - } - - @Test - fun `POST sessions maps a blank worker error body to a generic detail`() { - every { worker.start(any(), any()) } throws - HttpClientErrorException.create( - HttpStatus.BAD_GATEWAY, - "Bad Gateway", - org.springframework.http.HttpHeaders.EMPTY, - ByteArray(0), - null, - ) - - mockMvc - .perform( - post("/api/v1/credentials/sessions") - .header("X-User-Id", "operator") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(mapOf("provider" to "claude"))), - ).andExpect(status().isBadGateway) - .andExpect(jsonPath("$.detail").value("credential worker request failed")) - } - - @Test - fun `GET session returns the worker status`() { - every { worker.status("sess-9") } returns - workerStatus { - id = "sess-9" - provider = "codex" - phase = "awaiting_device" - deviceCode = "ABCD-1234" - verificationUrl = "https://auth.openai.com/device" - } - - mockMvc - .perform(get("/api/v1/credentials/sessions/sess-9").header("X-User-Id", "operator")) - .andExpect(status().isOk) - .andExpect(jsonPath("$.phase").value("awaiting_device")) - .andExpect(jsonPath("$.deviceCode").value("ABCD-1234")) - .andExpect(jsonPath("$.verificationUrl").value("https://auth.openai.com/device")) - } - - @Test - fun `GET unknown session relays the worker 404`() { - every { worker.status("nope") } throws - HttpClientErrorException.create( - HttpStatus.NOT_FOUND, - "Not Found", - org.springframework.http.HttpHeaders.EMPTY, - """{"error":"no matching session"}""".toByteArray(), - null, - ) - - mockMvc - .perform(get("/api/v1/credentials/sessions/nope").header("X-User-Id", "operator")) - .andExpect(status().isNotFound) - .andExpect(jsonPath("$.status").value(404)) - .andExpect(jsonPath("$.detail").value("no matching session")) - } - - @Test - fun `POST redirect relays the url and returns ok`() { - every { worker.submitRedirect("sess-1", "https://claude.ai/redirect?code=x") } returns - HttpCredentialWorkerClient.OkResult(ok = true) - - mockMvc - .perform( - post("/api/v1/credentials/sessions/sess-1/redirect") - .header("X-User-Id", "operator") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(mapOf("url" to "https://claude.ai/redirect?code=x"))), - ).andExpect(status().isOk) - .andExpect(jsonPath("$.ok").value(true)) - } - - @Test - fun `POST redirect with a blank url returns 422`() { - mockMvc - .perform( - post("/api/v1/credentials/sessions/sess-1/redirect") - .header("X-User-Id", "operator") - .contentType(MediaType.APPLICATION_JSON) - .content(objectMapper.writeValueAsString(mapOf("url" to ""))), - ).andExpect(status().isUnprocessableContent) - - verify(exactly = 0) { worker.submitRedirect(any(), any()) } - } - - @Test - fun `GET status returns browser-safe stored-credential summary scoped to the user`() { - every { credentials.statusFor("operator") } returns - listOf( - AgentCredentialRepository.CredentialStatus( - provider = AgentCredentialProvider.CLAUDE, - stored = true, - valid = null, - validatedAt = null, - updatedAt = Instant.parse("2026-06-23T10:00:00Z"), - ), - AgentCredentialRepository.CredentialStatus( - provider = AgentCredentialProvider.CODEX, - stored = true, - valid = false, - validatedAt = Instant.parse("2026-06-23T11:00:00Z"), - updatedAt = Instant.parse("2026-06-23T09:00:00Z"), - ), - ) - - mockMvc - .perform(get("/api/v1/credentials/status").header("X-User-Id", "operator")) - .andExpect(status().isOk) - .andExpect(jsonPath("$.claude.exists").value(true)) - .andExpect(jsonPath("$.claude.state").value("unvalidated")) - .andExpect(jsonPath("$.claude.valid").doesNotExist()) - .andExpect(jsonPath("$.claude.updatedAt").value("2026-06-23T10:00:00Z")) - .andExpect(jsonPath("$.codex.exists").value(true)) - .andExpect(jsonPath("$.codex.state").value("invalid")) - .andExpect(jsonPath("$.codex.valid").value(false)) - - verify { credentials.statusFor("operator") } - verify(exactly = 0) { worker.storedStatus() } - } - - @Test - fun `POST cancel returns the worker ack`() { - every { worker.cancel("sess-1") } returns HttpCredentialWorkerClient.OkResult(ok = true) - - mockMvc - .perform(post("/api/v1/credentials/sessions/sess-1/cancel").header("X-User-Id", "operator")) - .andExpect(status().isOk) - .andExpect(jsonPath("$.ok").value(true)) - } -} diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/InternalCredentialControllerTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/InternalCredentialControllerTest.kt deleted file mode 100644 index d7112a0..0000000 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/web/InternalCredentialControllerTest.kt +++ /dev/null @@ -1,234 +0,0 @@ -package com.jorisjonkers.personalstack.agents.infrastructure.web - -import com.fasterxml.jackson.databind.ObjectMapper -import com.jorisjonkers.personalstack.agents.domain.model.AgentCredentialProvider -import com.jorisjonkers.personalstack.agents.domain.model.AgentOauthCredential -import com.jorisjonkers.personalstack.agents.domain.port.AgentCredentialRepository -import com.jorisjonkers.personalstack.agents.infrastructure.credentials.CredentialValidationResult -import com.jorisjonkers.personalstack.agents.infrastructure.credentials.CredentialValidator -import com.jorisjonkers.personalstack.common.web.GlobalExceptionHandler -import io.mockk.every -import io.mockk.just -import io.mockk.mockk -import io.mockk.runs -import io.mockk.slot -import io.mockk.verify -import org.assertj.core.api.Assertions.assertThat -import org.hamcrest.Matchers.containsString -import org.hamcrest.Matchers.not -import org.junit.jupiter.api.BeforeEach -import org.junit.jupiter.api.Test -import org.springframework.http.MediaType -import org.springframework.test.web.servlet.MockMvc -import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post -import org.springframework.test.web.servlet.result.MockMvcResultMatchers.content -import org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath -import org.springframework.test.web.servlet.result.MockMvcResultMatchers.status -import org.springframework.test.web.servlet.setup.MockMvcBuilders - -class InternalCredentialControllerTest { - private val store = mockk() - private val validator = mockk() - private val objectMapper = ObjectMapper() - private lateinit var mockMvc: MockMvc - - @BeforeEach - fun setUp() { - mockMvc = - MockMvcBuilders - .standaloneSetup(InternalCredentialController(store, validator)) - .setControllerAdvice(GlobalExceptionHandler()) - .build() - } - - @Test - fun `POST credentials derives updatedBy from the owner userId, never the request body`() { - val captured = slot() - every { store.upsert(capture(captured)) } answers { captured.captured } - every { validator.validate(any(), any()) } returns CredentialValidationResult.UNKNOWN - - val result = - mockMvc - .perform( - post("/api/v1/internal/credentials") - .contentType(MediaType.APPLICATION_JSON) - .content( - objectMapper.writeValueAsString( - mapOf( - "userId" to "u-1", - "provider" to "CLAUDE", - "payload" to mapOf("oauth_token" to "sk-ant-secret"), - // A caller cannot forge authorship: any updatedBy in the - // body is ignored and the stored value tracks the owner. - "updatedBy" to "attacker", - ), - ), - ), - ).andExpect(status().isAccepted) - .andExpect(jsonPath("$.provider").value("CLAUDE")) - .andExpect(jsonPath("$.status").value("UNKNOWN")) - .andReturn() - - assertThat(captured.captured.userId).isEqualTo("u-1") - assertThat(captured.captured.updatedBy).isEqualTo("u-1") - assertThat(captured.captured.provider).isEqualTo(AgentCredentialProvider.CLAUDE) - assertThat(captured.captured.valid).isNull() - assertThat(captured.captured.validatedAt).isNull() - assertThat(result.response.contentAsString).doesNotContain("sk-ant-secret") - verify(exactly = 0) { store.markValidity(any(), any(), any()) } - } - - @Test - fun `POST credentials accepts Claude credentials json without legacy oauth token`() { - every { store.upsert(any()) } answers { arg(0) } - every { validator.validate(AgentCredentialProvider.CLAUDE, any()) } returns CredentialValidationResult.UNKNOWN - - mockMvc - .perform( - post("/api/v1/internal/credentials") - .contentType(MediaType.APPLICATION_JSON) - .content( - objectMapper.writeValueAsString( - mapOf( - "userId" to "u-credentials-json", - "provider" to "CLAUDE", - "payload" to - mapOf( - "credentials_json" to """{"claudeAiOauth":{"accessToken":"secret"}}""", - ), - "updatedBy" to "worker", - ), - ), - ), - ).andExpect(status().isAccepted) - .andExpect(jsonPath("$.provider").value("CLAUDE")) - .andExpect(jsonPath("$.status").value("UNKNOWN")) - .andExpect(content().string(not(containsString("accessToken")))) - - verify(exactly = 0) { store.markValidity(any(), any(), any()) } - } - - @Test - fun `POST credentials ingests codex payload and marks verified success only when validator succeeds`() { - every { store.upsert(any()) } answers { arg(0) } - every { validator.validate(AgentCredentialProvider.CODEX, any()) } returns CredentialValidationResult.VALID - every { store.markValidity("u-2", AgentCredentialProvider.CODEX, true) } just runs - - mockMvc - .perform( - post("/api/v1/internal/credentials") - .contentType(MediaType.APPLICATION_JSON) - .content( - objectMapper.writeValueAsString( - mapOf( - "userId" to "u-2", - "provider" to "CODEX", - "payload" to - mapOf( - "auth_json" to """{"access_token":"secret"}""", - "config_toml" to "profile = \"default\"", - ), - "updatedBy" to "worker", - ), - ), - ), - ).andExpect(status().isAccepted) - .andExpect(content().string(not(containsString("access_token")))) - - verify { store.markValidity("u-2", AgentCredentialProvider.CODEX, true) } - } - - @Test - fun `POST credentials marks explicit invalid only for validator explicit invalid`() { - every { store.upsert(any()) } answers { arg(0) } - every { validator.validate(any(), any()) } returns CredentialValidationResult.EXPLICIT_INVALID - every { store.markValidity("u-3", AgentCredentialProvider.CLAUDE, false) } just runs - - mockMvc - .perform( - post("/api/v1/internal/credentials") - .contentType(MediaType.APPLICATION_JSON) - .content( - objectMapper.writeValueAsString( - mapOf( - "userId" to "u-3", - "provider" to "CLAUDE", - "payload" to mapOf("oauth_token" to "bad"), - "updatedBy" to "worker", - ), - ), - ), - ).andExpect(status().isAccepted) - - verify { store.markValidity("u-3", AgentCredentialProvider.CLAUDE, false) } - } - - @Test - fun `POST credentials ignores validator failures and keeps request path successful`() { - every { store.upsert(any()) } answers { arg(0) } - every { validator.validate(any(), any()) } throws IllegalStateException("probe failed with secret") - - mockMvc - .perform( - post("/api/v1/internal/credentials") - .contentType(MediaType.APPLICATION_JSON) - .content( - objectMapper.writeValueAsString( - mapOf( - "userId" to "u-4", - "provider" to "CLAUDE", - "payload" to mapOf("oauth_token" to "secret"), - "updatedBy" to "worker", - ), - ), - ), - ).andExpect(status().isAccepted) - .andExpect(jsonPath("$.status").value("UNKNOWN")) - - verify(exactly = 0) { store.markValidity(any(), any(), any()) } - } - - @Test - fun `POST credentials rejects missing provider-specific payload fields`() { - mockMvc - .perform( - post("/api/v1/internal/credentials") - .contentType(MediaType.APPLICATION_JSON) - .content( - objectMapper.writeValueAsString( - mapOf( - "userId" to "u-5", - "provider" to "CODEX", - // codex requires auth_json; config_toml alone is incomplete. - "payload" to mapOf("config_toml" to "model=\"x\""), - "updatedBy" to "worker", - ), - ), - ), - ).andExpect(status().isBadRequest) - - verify(exactly = 0) { store.upsert(any()) } - verify(exactly = 0) { validator.validate(any(), any()) } - } - - @Test - fun `POST credentials requires uppercase provider enum values`() { - mockMvc - .perform( - post("/api/v1/internal/credentials") - .contentType(MediaType.APPLICATION_JSON) - .content( - objectMapper.writeValueAsString( - mapOf( - "userId" to "u-6", - "provider" to "claude", - "payload" to mapOf("oauth_token" to "secret"), - "updatedBy" to "worker", - ), - ), - ), - ).andExpect(status().isBadRequest) - - verify(exactly = 0) { store.upsert(any()) } - } -} diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionCheckerTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionCheckerTest.kt index 9b6e35c..0b7da1c 100644 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionCheckerTest.kt +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionCheckerTest.kt @@ -18,6 +18,7 @@ import com.jorisjonkers.personalstack.agents.domain.model.AgentSetupVersion import com.jorisjonkers.personalstack.agents.domain.model.Workspace import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentKind import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceId +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceKind import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceStatus import com.jorisjonkers.personalstack.agents.domain.port.AgentGatewayClient import com.jorisjonkers.personalstack.agents.domain.port.AgentSessionRepository @@ -56,6 +57,39 @@ class AttachPreconditionCheckerTest { assertThat(result?.gatewayEndpoint).isEqualTo("http://runner:8090") } + // #64. A Scratch Workspace never provisions a Pod, so it has no gateway + // endpoint; the attach must be resolved as local instead of rejected with + // "workspace has no gateway endpoint". For Claude and Codex that rejection + // is fatal rather than cosmetic: the terminal is the only place an Agent + // Login can be created, so an unopenable one leaves no way to sign in at + // all. The gate used to require kind == SHELL and silently did exactly that. + @Test + fun `resolveAttach returns Ready and local for a Claude session in a Scratch Workspace`() { + every { sessions.findById(sessionId) } returns agentSession(gatewayAgentId = "gw-1") + every { workspaces.findById(workspaceId) } returns + workspace().copy(kind = WorkspaceKind.SCRATCH, gatewayEndpoint = null, podName = null) + + val result = checker.resolveAttach(clientSession()) + + assertThat(result).isNotNull + assertThat(result?.local).isTrue() + assertThat(result?.gatewayEndpoint).isNull() + } + + // A Scratch Workspace created before #62 can still hold a session bound to a + // runner Pod. Those attach through the Pod's gateway, not this container. + @Test + fun `resolveAttach stays remote for a Scratch Workspace that still has a runner Pod`() { + every { sessions.findById(sessionId) } returns agentSession(gatewayAgentId = "gw-1") + every { workspaces.findById(workspaceId) } returns + workspace().copy(kind = WorkspaceKind.SCRATCH, podName = "agent-runner-legacy") + + val result = checker.resolveAttach(clientSession()) + + assertThat(result?.local).isFalse() + assertThat(result?.gatewayEndpoint).isEqualTo("http://runner:8090") + } + @Test fun `resolveAttach rejects and closes when session id is malformed`() { val client = mockk(relaxed = true) diff --git a/client-spec/openapi/agents-api-client.json b/client-spec/openapi/agents-api-client.json index 6413744..5491b3c 100644 --- a/client-spec/openapi/agents-api-client.json +++ b/client-spec/openapi/agents-api-client.json @@ -588,110 +588,6 @@ "deprecated" : true } }, - "/api/v1/credentials/sessions" : { - "post" : { - "tags" : [ "credential-controller" ], - "summary" : "Start a CLI re-authentication session for Claude or Codex", - "operationId" : "start_1", - "parameters" : [ { - "name" : "X-User-Id", - "in" : "header", - "required" : true, - "schema" : { - "type" : "string" - } - } ], - "requestBody" : { - "content" : { - "application/json" : { - "schema" : { - "$ref" : "#/components/schemas/StartCredentialSessionRequest" - } - } - }, - "required" : true - }, - "responses" : { - "200" : { - "description" : "OK", - "content" : { - "*/*" : { - "schema" : { - "type" : "object" - } - } - } - } - }, - "deprecated" : true - } - }, - "/api/v1/credentials/sessions/{id}/redirect" : { - "post" : { - "tags" : [ "credential-controller" ], - "summary" : "Submit the Claude post-approval redirect URL back to a session", - "operationId" : "redirect", - "parameters" : [ { - "name" : "id", - "in" : "path", - "required" : true, - "schema" : { - "type" : "string" - } - } ], - "requestBody" : { - "content" : { - "application/json" : { - "schema" : { - "$ref" : "#/components/schemas/SubmitRedirectUrlRequest" - } - } - }, - "required" : true - }, - "responses" : { - "200" : { - "description" : "OK", - "content" : { - "*/*" : { - "schema" : { - "type" : "object" - } - } - } - } - }, - "deprecated" : true - } - }, - "/api/v1/credentials/sessions/{id}/cancel" : { - "post" : { - "tags" : [ "credential-controller" ], - "summary" : "Cancel an in-flight re-authentication session", - "operationId" : "cancel", - "parameters" : [ { - "name" : "id", - "in" : "path", - "required" : true, - "schema" : { - "type" : "string" - } - } ], - "responses" : { - "200" : { - "description" : "OK", - "content" : { - "*/*" : { - "schema" : { - "type" : "object" - } - } - } - } - }, - "deprecated" : true - } - }, "/api/v1/conversations" : { "get" : { "tags" : [ "conversation-controller" ], @@ -1376,62 +1272,6 @@ } } }, - "/api/v1/credentials/status" : { - "get" : { - "tags" : [ "credential-controller" ], - "summary" : "Report what credentials are currently stored for each provider", - "operationId" : "storedStatus", - "parameters" : [ { - "name" : "X-User-Id", - "in" : "header", - "required" : true, - "schema" : { - "type" : "string" - } - } ], - "responses" : { - "200" : { - "description" : "OK", - "content" : { - "*/*" : { - "schema" : { - "$ref" : "#/components/schemas/StoredCredentialStatusResponse" - } - } - } - } - }, - "deprecated" : true - } - }, - "/api/v1/credentials/sessions/{id}" : { - "get" : { - "tags" : [ "credential-controller" ], - "summary" : "Get the current status of a re-authentication session", - "operationId" : "status", - "parameters" : [ { - "name" : "id", - "in" : "path", - "required" : true, - "schema" : { - "type" : "string" - } - } ], - "responses" : { - "200" : { - "description" : "OK", - "content" : { - "*/*" : { - "schema" : { - "type" : "object" - } - } - } - } - }, - "deprecated" : true - } - }, "/api/v1/conversations/{id}" : { "get" : { "tags" : [ "conversation-controller" ], @@ -1609,6 +1449,25 @@ } } }, + "/api/v1/agent-logins" : { + "get" : { + "tags" : [ "agent-login-controller" ], + "summary" : "Report which providers have an Agent Login on the home volume", + "operationId" : "status", + "responses" : { + "200" : { + "description" : "OK", + "content" : { + "*/*" : { + "schema" : { + "$ref" : "#/components/schemas/AgentLoginStatusResponse" + } + } + } + } + } + } + }, "/api/v1/workspaces/{workspaceId}/sessions/{sessionId}" : { "delete" : { "tags" : [ "agent-session-controller" ], @@ -2298,27 +2157,6 @@ }, "required" : [ "createdAt", "defaultBranch", "id", "name", "projectId", "repoUrl", "updatedAt" ] }, - "StartCredentialSessionRequest" : { - "type" : "object", - "properties" : { - "provider" : { - "type" : "string", - "minLength" : 1, - "pattern" : "claude|codex" - } - }, - "required" : [ "provider" ] - }, - "SubmitRedirectUrlRequest" : { - "type" : "object", - "properties" : { - "url" : { - "type" : "string", - "minLength" : 1 - } - }, - "required" : [ "url" ] - }, "StartConversationRequest" : { "type" : "object", "properties" : { @@ -3040,41 +2878,6 @@ }, "required" : [ "checkedAt", "state" ] }, - "StoredCredentialStatusResponse" : { - "type" : "object", - "properties" : { - "claude" : { - "$ref" : "#/components/schemas/StoredProviderCredentialStatus" - }, - "codex" : { - "$ref" : "#/components/schemas/StoredProviderCredentialStatus" - } - }, - "required" : [ "claude", "codex" ] - }, - "StoredProviderCredentialStatus" : { - "type" : "object", - "properties" : { - "exists" : { - "type" : "boolean" - }, - "state" : { - "type" : "string" - }, - "valid" : { - "type" : [ "boolean", "null" ] - }, - "validatedAt" : { - "type" : [ "string", "null" ], - "format" : "date-time" - }, - "updatedAt" : { - "type" : [ "string", "null" ], - "format" : "date-time" - } - }, - "required" : [ "exists", "state" ] - }, "ConversationDetailResponse" : { "type" : "object", "properties" : { @@ -3238,6 +3041,30 @@ } }, "required" : [ "claudeCredentialsPvc", "cliTools", "codexCredentialsPvc", "connectorConfig", "createdAt", "defaultMcpProfile", "defaultSelectable", "displayName", "dockerSocketEnabled", "dockerSocketPath", "dockerSocketSupplementalGroups", "gatewayPort", "githubDeployKeySecret", "image", "imagePullPolicy", "knowledgeBaseUrl", "knowledgeBearerSecret", "knowledgeBearerSecretKey", "mcpServersConfigMap", "namespace", "nodeSelector", "selectable", "serviceAccount", "setup", "toolAllowlist", "toolProfiles", "updatedAt" ] + }, + "AgentLoginResponse" : { + "type" : "object", + "properties" : { + "kind" : { + "type" : "string" + }, + "present" : { + "type" : "boolean" + } + }, + "required" : [ "kind", "present" ] + }, + "AgentLoginStatusResponse" : { + "type" : "object", + "properties" : { + "logins" : { + "type" : "array", + "items" : { + "$ref" : "#/components/schemas/AgentLoginResponse" + } + } + }, + "required" : [ "logins" ] } }, "securitySchemes" : { diff --git a/client-spec/openapi/agents-api.json b/client-spec/openapi/agents-api.json index 89376ee..e5768b3 100644 --- a/client-spec/openapi/agents-api.json +++ b/client-spec/openapi/agents-api.json @@ -588,110 +588,6 @@ "deprecated" : true } }, - "/api/v1/credentials/sessions" : { - "post" : { - "tags" : [ "credential-controller" ], - "summary" : "Start a CLI re-authentication session for Claude or Codex", - "operationId" : "start_1", - "parameters" : [ { - "name" : "X-User-Id", - "in" : "header", - "required" : true, - "schema" : { - "type" : "string" - } - } ], - "requestBody" : { - "content" : { - "application/json" : { - "schema" : { - "$ref" : "#/components/schemas/StartCredentialSessionRequest" - } - } - }, - "required" : true - }, - "responses" : { - "200" : { - "description" : "OK", - "content" : { - "*/*" : { - "schema" : { - "type" : "object" - } - } - } - } - }, - "deprecated" : true - } - }, - "/api/v1/credentials/sessions/{id}/redirect" : { - "post" : { - "tags" : [ "credential-controller" ], - "summary" : "Submit the Claude post-approval redirect URL back to a session", - "operationId" : "redirect", - "parameters" : [ { - "name" : "id", - "in" : "path", - "required" : true, - "schema" : { - "type" : "string" - } - } ], - "requestBody" : { - "content" : { - "application/json" : { - "schema" : { - "$ref" : "#/components/schemas/SubmitRedirectUrlRequest" - } - } - }, - "required" : true - }, - "responses" : { - "200" : { - "description" : "OK", - "content" : { - "*/*" : { - "schema" : { - "type" : "object" - } - } - } - } - }, - "deprecated" : true - } - }, - "/api/v1/credentials/sessions/{id}/cancel" : { - "post" : { - "tags" : [ "credential-controller" ], - "summary" : "Cancel an in-flight re-authentication session", - "operationId" : "cancel", - "parameters" : [ { - "name" : "id", - "in" : "path", - "required" : true, - "schema" : { - "type" : "string" - } - } ], - "responses" : { - "200" : { - "description" : "OK", - "content" : { - "*/*" : { - "schema" : { - "type" : "object" - } - } - } - } - }, - "deprecated" : true - } - }, "/api/v1/conversations" : { "get" : { "tags" : [ "conversation-controller" ], @@ -1376,62 +1272,6 @@ } } }, - "/api/v1/credentials/status" : { - "get" : { - "tags" : [ "credential-controller" ], - "summary" : "Report what credentials are currently stored for each provider", - "operationId" : "storedStatus", - "parameters" : [ { - "name" : "X-User-Id", - "in" : "header", - "required" : true, - "schema" : { - "type" : "string" - } - } ], - "responses" : { - "200" : { - "description" : "OK", - "content" : { - "*/*" : { - "schema" : { - "$ref" : "#/components/schemas/StoredCredentialStatusResponse" - } - } - } - } - }, - "deprecated" : true - } - }, - "/api/v1/credentials/sessions/{id}" : { - "get" : { - "tags" : [ "credential-controller" ], - "summary" : "Get the current status of a re-authentication session", - "operationId" : "status", - "parameters" : [ { - "name" : "id", - "in" : "path", - "required" : true, - "schema" : { - "type" : "string" - } - } ], - "responses" : { - "200" : { - "description" : "OK", - "content" : { - "*/*" : { - "schema" : { - "type" : "object" - } - } - } - } - }, - "deprecated" : true - } - }, "/api/v1/conversations/{id}" : { "get" : { "tags" : [ "conversation-controller" ], @@ -1609,6 +1449,25 @@ } } }, + "/api/v1/agent-logins" : { + "get" : { + "tags" : [ "agent-login-controller" ], + "summary" : "Report which providers have an Agent Login on the home volume", + "operationId" : "status", + "responses" : { + "200" : { + "description" : "OK", + "content" : { + "*/*" : { + "schema" : { + "$ref" : "#/components/schemas/AgentLoginStatusResponse" + } + } + } + } + } + } + }, "/api/v1/workspaces/{workspaceId}/sessions/{sessionId}" : { "delete" : { "tags" : [ "agent-session-controller" ], @@ -2298,27 +2157,6 @@ }, "required" : [ "createdAt", "defaultBranch", "id", "name", "projectId", "repoUrl", "updatedAt" ] }, - "StartCredentialSessionRequest" : { - "type" : "object", - "properties" : { - "provider" : { - "type" : "string", - "minLength" : 1, - "pattern" : "claude|codex" - } - }, - "required" : [ "provider" ] - }, - "SubmitRedirectUrlRequest" : { - "type" : "object", - "properties" : { - "url" : { - "type" : "string", - "minLength" : 1 - } - }, - "required" : [ "url" ] - }, "StartConversationRequest" : { "type" : "object", "properties" : { @@ -3040,41 +2878,6 @@ }, "required" : [ "checkedAt", "state" ] }, - "StoredCredentialStatusResponse" : { - "type" : "object", - "properties" : { - "claude" : { - "$ref" : "#/components/schemas/StoredProviderCredentialStatus" - }, - "codex" : { - "$ref" : "#/components/schemas/StoredProviderCredentialStatus" - } - }, - "required" : [ "claude", "codex" ] - }, - "StoredProviderCredentialStatus" : { - "type" : "object", - "properties" : { - "exists" : { - "type" : "boolean" - }, - "state" : { - "type" : "string" - }, - "valid" : { - "type" : [ "boolean", "null" ] - }, - "validatedAt" : { - "type" : [ "string", "null" ], - "format" : "date-time" - }, - "updatedAt" : { - "type" : [ "string", "null" ], - "format" : "date-time" - } - }, - "required" : [ "exists", "state" ] - }, "ConversationDetailResponse" : { "type" : "object", "properties" : { @@ -3238,6 +3041,30 @@ } }, "required" : [ "claudeCredentialsPvc", "cliTools", "codexCredentialsPvc", "connectorConfig", "createdAt", "defaultMcpProfile", "defaultSelectable", "displayName", "dockerSocketEnabled", "dockerSocketPath", "dockerSocketSupplementalGroups", "gatewayPort", "githubDeployKeySecret", "image", "imagePullPolicy", "knowledgeBaseUrl", "knowledgeBearerSecret", "knowledgeBearerSecretKey", "mcpServersConfigMap", "namespace", "nodeSelector", "selectable", "serviceAccount", "setup", "toolAllowlist", "toolProfiles", "updatedAt" ] + }, + "AgentLoginResponse" : { + "type" : "object", + "properties" : { + "kind" : { + "type" : "string" + }, + "present" : { + "type" : "boolean" + } + }, + "required" : [ "kind", "present" ] + }, + "AgentLoginStatusResponse" : { + "type" : "object", + "properties" : { + "logins" : { + "type" : "array", + "items" : { + "$ref" : "#/components/schemas/AgentLoginResponse" + } + } + }, + "required" : [ "logins" ] } }, "securitySchemes" : {