diff --git a/contracts/agents-api/sandbox-deployment.md b/contracts/agents-api/sandbox-deployment.md index 01442b076..3b138d52d 100644 --- a/contracts/agents-api/sandbox-deployment.md +++ b/contracts/agents-api/sandbox-deployment.md @@ -276,9 +276,16 @@ Malformed selections return 400; validated configuration diagnostics use `invalid_sandbox_configuration`. Stale generations, retained resources or an incompatible deployment return 409 `sandbox_deployment_conflict`. A node configuration mismatch returns 409 `sandbox_specification_mismatch`; rejected -node credentials return 401 `invalid_node_credential`. Unavailable provider -preparation returns 503 `execution_unavailable`. Storage and credential failures -remain errors; an empty or failed read is not evidence of cleanup. +node credentials return 401 `invalid_node_credential`. Node machine routes check +the credential before any deployment state, so a missing or rejected credential, +including one issued for another installation, gets that 401 even before +initialization or under E2B. Until the deployment is initialized, +`GET /api/v1/sandbox-node/configuration` and `POST /api/v1/sandbox-node/enroll` +answer an otherwise accepted credential with 503 `runtime_node_unavailable`; +`GET /api/v1/sandbox-node/identity` and the node connection answer 401 for any +credential. Unavailable provider preparation returns 503 `execution_unavailable`. +Storage and credential failures remain errors; an empty or failed read is not +evidence of cleanup. The administrator node list and node detail report an unready provider with one fixed `diagnostic` code: `docker_unavailable`, `docker_limits_unsupported`, diff --git a/contracts/agents-api/v1/model_execution_test.go b/contracts/agents-api/v1/model_execution_test.go index b2fe5f398..2c6dd664c 100644 --- a/contracts/agents-api/v1/model_execution_test.go +++ b/contracts/agents-api/v1/model_execution_test.go @@ -12,9 +12,15 @@ func TestModelExecutionValidation(t *testing.T) { t.Fatalf("incorrect protocol validation: %s/%s", tc.protocol, tc.harness) } } - for _, url := range []string{"http://example.com", "https://user:pass@example.com", "https://example.com?key=secret", "https://example.com#secret", "https://"} { + for _, url := range []string{"http://example.com", "https://user:pass@example.com", "https://example.com?key=secret", "https://example.com#secret", "https://", + "https://example.com:99999/v1", "https://example.com:0/v1", "https://xn--.test", "https://xn--a.test", "https://a..b", "https://999.1.1.1"} { if (&ModelProviderInput{Protocol: "responses", BaseURL: url, APIKey: "secret"}).Validate() == nil { - t.Fatal("unsafe provider URL accepted") + t.Fatal("unsafe or unusable provider URL accepted", url) + } + } + for _, url := range []string{"https://example.com:8443/v1", "https://127.0.0.1/v1", "https://[::1]:8443/v1", "https://model_gateway.internal/v1", "https://bücher.example/v1"} { + if err := (&ModelProviderInput{Protocol: "responses", BaseURL: url, APIKey: "secret"}).Validate(); err != nil { + t.Fatal("valid provider URL rejected", url, err) } } if (&ModelProviderInput{Protocol: "anthropic", BaseURL: "https://example.com", APIKey: "secret"}).ValidateHarness("mcode") == nil { diff --git a/contracts/agents-api/v1/model_provider_admission.go b/contracts/agents-api/v1/model_provider_admission.go index c9e6141f0..b158e99db 100644 --- a/contracts/agents-api/v1/model_provider_admission.go +++ b/contracts/agents-api/v1/model_provider_admission.go @@ -1,18 +1,57 @@ package v1 import ( + "net" "net/url" + "strconv" "strings" + + "golang.org/x/net/idna" ) const providerEnvironment = "openai_hosted" const providerScheme = "https" +// providerHost converts a domain as URL host parsing does (UTS #46 without +// hyphen or STD3 restrictions), rejecting invalid labels such as bad punycode. +var providerHost = idna.New(idna.MapForLookup(), idna.BidiRule(), idna.StrictDomainName(false), idna.CheckHyphens(false)) + func ModelProviderEnvironmentSupported(environment string) bool { return environment == providerEnvironment } func validModelProviderBaseURL(base string) bool { u, err := url.Parse(base) - return err == nil && u.Scheme == providerScheme && u.Hostname() != "" && u.User == nil && u.RawQuery == "" && u.Fragment == "" && !strings.ContainsAny(base, "\x00\r\n") + return err == nil && u.Scheme == providerScheme && validModelProviderHost(u) && u.User == nil && u.RawQuery == "" && u.Fragment == "" && !strings.ContainsAny(base, "\x00\r\n") +} + +// validModelProviderHost requires a usable host: an IP address, or a domain +// whose labels are nonempty letters, digits, hyphens and underscores and whose +// final label is not numeric. Any port must be in 1-65535. +func validModelProviderHost(u *url.URL) bool { + if port := u.Port(); port != "" { + if n, err := strconv.Atoi(port); err != nil || n < 1 || n > 65535 { + return false + } + } + host := u.Hostname() + if net.ParseIP(host) != nil { + return true + } + ascii, err := providerHost.ToASCII(host) + if err != nil { + return false + } + labels := strings.Split(strings.TrimSuffix(ascii, "."), ".") + for _, label := range labels { + if label == "" || len(label) > 63 || label == "xn--" || strings.IndexFunc(label, invalidHostRune) >= 0 { + return false + } + } + // A numeric final label makes the host an IPv4 address, which ParseIP rejected. + return strings.Trim(labels[len(labels)-1], "0123456789") != "" +} + +func invalidHostRune(r rune) bool { + return !(r >= 'a' && r <= 'z' || r >= '0' && r <= '9' || r == '-' || r == '_') } diff --git a/packages/agents-client/saved-agent-defaults.md b/packages/agents-client/saved-agent-defaults.md index 33a32317f..5f8432594 100644 --- a/packages/agents-client/saved-agent-defaults.md +++ b/packages/agents-client/saved-agent-defaults.md @@ -37,6 +37,8 @@ Reads return `ModelProviderView`, containing safe endpoint/limit fields and `api_key_configured`, never `api_key`. It is distinct from `ModelProviderInput`: do not submit a read response as an update. Replacing a provider requires its full protocol, endpoint and key; MiniMax Code also requires both token limits. +Either default may be absent from a read: an Agent saved with only a provider has +no `harness`, and one saved with an empty extension reads `x_agents_core: {}`. On update, omitting the extension preserves all defaults; omitting either nested member preserves that member. A null provider clears its saved bundle, while diff --git a/packages/agents-client/src/admin-client.test.ts b/packages/agents-client/src/admin-client.test.ts index d24d763d1..3f531086e 100644 --- a/packages/agents-client/src/admin-client.test.ts +++ b/packages/agents-client/src/admin-client.test.ts @@ -177,6 +177,35 @@ describe("AdminClient response contracts", () => { expect((await clientWith(page([session])).client.listSessions(projectId)).data).toEqual([session]); }); + it("projects saved Agent Core defaults, including the safe provider view, and rejects secrets", async () => { + const saved = { id: resourceId, object: "agent", model: "model", name: null, instructions: null, metadata: {}, multi_agent: { enabled: false, max_concurrent_subagents: null }, reasoning: { effort: null, summary: null }, service_tier: "auto", text: { format: { type: "text" }, verbosity: "medium" }, tools: [], created_at: 1, updated_at: 1 }; + const provider = { protocol: "anthropic", base_url: "https://provider.test/v1", context_window: 200000, max_output_tokens: 8000, api_key_configured: true }; + // Core omits the extension, or returns each default only when saved. + for (const core of [undefined, null, {}, { harness: "mcode" }, { model_provider: provider }, { harness: "mcode", model_provider: provider }, + { model_provider: { protocol: "responses", base_url: "https://provider.test", api_key_configured: true } }]) { + const agent = core === undefined ? saved : { ...saved, x_agents_core: core }; + expect(await clientWith(agent).client.retrieveAgent(projectId, resourceId)).toEqual(agent); + expect((await clientWith(page([agent])).client.listAgents(projectId)).data).toEqual([agent]); + } + for (const core of ["mcode", { harness: "other" }, { harness: null }, { harness: "mcode", api_key: "leak" }, { extra: 1 }, + { model_provider: null }, { model_provider: { ...provider, api_key: "leak" } }, { model_provider: { protocol: "anthropic", base_url: "https://provider.test" } }]) { + const agent = { ...saved, x_agents_core: core }; + await expect(clientWith(agent).client.retrieveAgent(projectId, resourceId)).rejects.toBeInstanceOf(AgentCoreError); + await expect(clientWith(page([agent])).client.listAgents(projectId)).rejects.toBeInstanceOf(AgentCoreError); + } + }); + + it("reads any provider base URL an earlier Core stored but still rejects unsafe ones", async () => { + const saved = { id: resourceId, object: "agent", model: "model", name: null, instructions: null, metadata: {}, multi_agent: { enabled: false, max_concurrent_subagents: null }, reasoning: { effort: null, summary: null }, service_tier: "auto", text: { format: { type: "text" }, verbosity: "medium" }, tools: [], created_at: 1, updated_at: 1 }; + const withURL = (base_url: string, id = resourceId) => ({ ...saved, id, x_agents_core: { model_provider: { protocol: "responses", base_url, api_key_configured: true } } }); + // Core once accepted hosts and ports that URL parsing rejects; one must not fail the list. + const stored = ["https://p.test:99999/v1", "https://xn--.test", "https://[::1]:8443/v1", "HTTPS://p.test/v1?"].map((url, index) => withURL(url, `agent-${index}`)); + expect((await clientWith(page(stored)).client.listAgents(projectId)).data).toEqual(stored); + for (const url of ["http://p.test", "https://user:pw@p.test", "https://p.test/?key=secret", "https://p.test/#secret", "https://:443/v1"]) { + await expect(clientWith(page([withURL(url)])).client.listAgents(projectId)).rejects.toBeInstanceOf(AgentCoreError); + } + }); + it("binds Skills, versions and Artifacts to requested resources", async () => { const skill = { id: "skill", object: "skill", created_at: 1, name: "helper", description: "help", default_version: "1", latest_version: "2" }; expect(await clientWith(skill).client.retrieveSkill(projectId, "skill")).toEqual(skill); diff --git a/packages/agents-client/src/admin-projection.ts b/packages/agents-client/src/admin-projection.ts index 53a9f8326..3d2494ece 100644 --- a/packages/agents-client/src/admin-projection.ts +++ b/packages/agents-client/src/admin-projection.ts @@ -1,4 +1,4 @@ -import { AgentCoreError, projectAgentSnapshot, projectRuntimeObservation } from "./client"; +import { AgentCoreError, projectRuntimeObservation, projectSavedAgentConfiguration } from "./client"; import { projectTokenUsage } from "./usage-projection"; import { canonicalUuid, exactFields, isNonnegativeInteger, isRecord, sameResourceId } from "./response-projection"; import type { ListPage, SavedAgent } from "./types"; @@ -69,7 +69,7 @@ export function projectAdminSessionArchive(value: unknown, sessionId: string): A export function projectSavedAgent(value: unknown, expectedId?: string): SavedAgent { if (!isRecord(value)) return invalidAdminResponse(); const { object, metadata, created_at, updated_at, ...snapshot } = value; - const agent = projectAgentSnapshot(snapshot); + const agent = projectSavedAgentConfiguration(snapshot); if (object !== "agent" || !isRecord(metadata) || Object.values(metadata).some((entry) => typeof entry !== "string") || !isNonnegativeInteger(created_at) || !isNonnegativeInteger(updated_at) || updated_at < created_at || (expectedId !== undefined && !sameResourceId(agent.id, expectedId))) return invalidAdminResponse(); diff --git a/packages/agents-client/src/client.test.ts b/packages/agents-client/src/client.test.ts index 5fb94cf7f..2dab3da6f 100644 --- a/packages/agents-client/src/client.test.ts +++ b/packages/agents-client/src/client.test.ts @@ -677,6 +677,21 @@ describe("OpenAIAgentsClient", () => { }); }); + it("accepts only a harness in a Session Agent's x_agents_core", async () => { + const retrieve = (core: unknown) => new OpenAIAgentsClient({ fetch: recordingFetch(jsonResponse({ + ...sessionResource(), + agent: core === undefined ? agentSnapshot() : { ...agentSnapshot(), x_agents_core: core }, + }), []) }).retrieveSession("session"); + for (const core of [undefined, null, { harness: "codex" }]) { + expect((await retrieve(core)).agent.x_agents_core).toEqual(core); + } + // Session reads never carry saved provider defaults; they live in execution_configuration. + const provider = { protocol: "responses", base_url: "https://provider.test", api_key_configured: true }; + for (const core of [{}, { model_provider: provider }, { harness: "codex", model_provider: provider }, { harness: "codex", api_key: "leak" }]) { + await expect(retrieve(core)).rejects.toMatchObject({ status: 502, code: "invalid_session_resource" }); + } + }); + it("preserves a nullable resource error code", async () => { const client = new OpenAIAgentsClient({ fetch: recordingFetch(jsonResponse({ error: { diff --git a/packages/agents-client/src/client.ts b/packages/agents-client/src/client.ts index 8515168f7..5a79c218b 100644 --- a/packages/agents-client/src/client.ts +++ b/packages/agents-client/src/client.ts @@ -2,6 +2,7 @@ import { exactFields, onlyFields, isRecord, hasOwn, canonicalUuid, isNonnegative import { projectTokenUsage } from "./usage-projection"; import { projectAgentTurn, projectSessionItem, projectItemContent, projectHistoryPage, validateHistoryPageOptions } from "./history-projection"; import { projectOpenAIHostedSessionEnvironment } from "./session-environment-projection"; +import { safeProvider } from "./execution-configuration-projection"; import { createSSEDecoder } from "./sse"; import { projectVaultCredentialAuth, validCredentialURL } from "./vault-credential-auth"; import { @@ -28,6 +29,8 @@ import { } from "./skill-projection"; import type { AgentCore, + AgentsCoreSelection, + AgentSnapshot, AgentDeleted, AgentEnvironmentInput, AgentEnvironmentResource, @@ -58,6 +61,7 @@ import type { PageOrder, ReadOptions, SavedAgent, + SavedAgentCore, SessionDeleted, SessionEvent, SessionListOptions, @@ -256,6 +260,8 @@ const agentSnapshotFields = new Set([ "service_tier", "text", "tools", ]); const agentSnapshotAcceptedFields = new Set([...agentSnapshotFields, "x_agents_core"]); +const sessionAgentCoreFields = new Set(["harness"]); +const savedAgentCoreFields = new Set(["harness", "model_provider"]); const multiAgentFields = new Set(["enabled", "max_concurrent_subagents"]); const reasoningFields = new Set(["effort", "summary"]); const textFields = new Set(["format", "verbosity"]); @@ -595,12 +601,53 @@ function invalidSessionResource(message = "Agent Core returned an invalid Sessio throw new AgentCoreError(message, 502, "invalid_session_resource"); } +type AgentConfiguration = Omit & { x_agents_core?: Core | null }; + +/** A Session's effective Agent reports only its persisted harness. */ +function projectSessionAgentCore(value: unknown): AgentsCoreSelection | null | undefined { + if (value === undefined || value === null) return value; + if (!isRecord(value) || !exactFields(value, sessionAgentCoreFields) || !isHarnessKind(value.harness)) { + return invalidSessionResource(); + } + return { harness: value.harness }; +} + +/** + * Saved Agent defaults: an optional harness and an optional safe provider view. + * Either may be absent, so an empty object is valid. The API key is write-only. + */ +function projectSavedAgentCore(value: unknown): SavedAgentCore | null | undefined { + if (value === undefined || value === null) return value; + if ( + !isRecord(value) || !onlyFields(value, savedAgentCoreFields) || + (hasOwn(value, "harness") && !isHarnessKind(value.harness)) + ) return invalidSessionResource(); + return { + ...(hasOwn(value, "harness") ? { harness: value.harness as CoreHarnessKind } : {}), + ...(hasOwn(value, "model_provider") + ? { model_provider: safeProvider(value.model_provider, invalidSessionResource) } + : {}), + }; +} + export function projectAgentSnapshot(value: unknown): AgentSession["agent"] { + return projectAgentConfiguration(value, projectSessionAgentCore); +} + +/** Projects a saved Agent's configuration members, without its resource fields. */ +export function projectSavedAgentConfiguration(value: unknown): AgentConfiguration { + return projectAgentConfiguration(value, projectSavedAgentCore); +} + +function projectAgentConfiguration( + value: unknown, + projectCore: (value: unknown) => Core | null | undefined, +): AgentConfiguration { if ( !isRecord(value) || !onlyFields(value, agentSnapshotAcceptedFields) || [...agentSnapshotFields].some((field) => !hasOwn(value, field)) ) return invalidSessionResource(); - const agentsCore = value.x_agents_core; + const agentsCore = projectCore(value.x_agents_core); const multiAgent = value.multi_agent; const reasoning = value.reasoning; const text = value.text; @@ -609,9 +656,6 @@ export function projectAgentSnapshot(value: unknown): AgentSession["agent"] { typeof value.model !== "string" || value.model.trim() === "" || !(value.name === null || typeof value.name === "string") || !(value.instructions === null || typeof value.instructions === "string") || - !(agentsCore === undefined || agentsCore === null || ( - isRecord(agentsCore) && exactFields(agentsCore, new Set(["harness"])) && isHarnessKind(agentsCore.harness) - )) || !isRecord(multiAgent) || !exactFields(multiAgent, multiAgentFields) || typeof multiAgent.enabled !== "boolean" || !(multiAgent.max_concurrent_subagents === null || @@ -635,9 +679,7 @@ export function projectAgentSnapshot(value: unknown): AgentSession["agent"] { return { id: value.id, - ...(agentsCore === undefined - ? {} - : { x_agents_core: agentsCore === null ? null : { harness: agentsCore.harness as CoreHarnessKind } }), + ...(agentsCore === undefined ? {} : { x_agents_core: agentsCore }), model: value.model, name: value.name, instructions: value.instructions, diff --git a/packages/agents-client/src/execution-configuration-projection.ts b/packages/agents-client/src/execution-configuration-projection.ts index 8d577b7a5..b288e9728 100644 --- a/packages/agents-client/src/execution-configuration-projection.ts +++ b/packages/agents-client/src/execution-configuration-projection.ts @@ -13,17 +13,31 @@ function selection(value: unknown, invalid: Invalid): SessionExecutionConfigurat return { value: value.value as string | null, source: value.source as ExecutionConfigurationSource }; } -function safeProvider(value: unknown, invalid: Invalid): ModelProviderView { +// Splits an absolute URL as RFC 3986 appendix B does, without parsing its host. +const baseURLPattern = /^https:\/\/([^/?#]*)[^?#]*(?:\?([^#]*))?(?:#([\s\S]*))?$/iu; + +/** + * Checks a stored base URL no more strictly than Core's write rule: HTTPS with a + * host and no credentials, query or fragment. Host syntax is Core's to enforce; + * a value an earlier Core accepted must not fail a whole list. + */ +function safeBaseURL(value: string): boolean { + const match = baseURLPattern.exec(value); + if (match === null || /[\r\n\0]/u.test(value)) return false; + const [, authority = "", query = "", fragment = ""] = match; + const host = authority.replace(/:[0-9]*$/u, "").replace(/^\[(.*)\]$/u, "$1"); + return !authority.includes("@") && host !== "" && query === "" && fragment === ""; +} + +/** The safe provider view shared by frozen Session configuration and saved Agent reads. */ +export function safeProvider(value: unknown, invalid: Invalid): ModelProviderView { if (!isRecord(value) || !onlyFields(value, providerFields) || (value.protocol !== "responses" && value.protocol !== "anthropic") || typeof value.base_url !== "string" || typeof value.api_key_configured !== "boolean" || (value.context_window !== undefined && !isNonnegativeInteger(value.context_window)) || (value.max_output_tokens !== undefined && !isNonnegativeInteger(value.max_output_tokens)) || Number(value.max_output_tokens ?? 0) > Number(value.context_window ?? 0)) return invalid(); - try { - const url = new URL(value.base_url); - if (url.protocol !== "https:" || !url.hostname || url.username || url.password || url.search || url.hash || /[\r\n\0]/u.test(value.base_url)) return invalid(); - } catch { return invalid(); } + if (!safeBaseURL(value.base_url)) return invalid(); return { protocol: value.protocol, base_url: value.base_url, api_key_configured: value.api_key_configured, ...(value.context_window === undefined ? {} : { context_window: value.context_window as number }), diff --git a/services/agents-api/internal/store/runtime_node_configuration.go b/services/agents-api/internal/store/runtime_node_configuration.go index 3626ce589..a57d91c55 100644 --- a/services/agents-api/internal/store/runtime_node_configuration.go +++ b/services/agents-api/internal/store/runtime_node_configuration.go @@ -6,6 +6,7 @@ import ( "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/db/sqlc" "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/sandbox" + "github.com/jackc/pgx/v5/pgtype" ) type RuntimeNodeConfiguration struct { @@ -21,39 +22,50 @@ type RuntimeNodeConfiguration struct { // Read-only bootstrap never consumes enrollment tokens or reveals cloud credentials. // The deployment lock keeps authentication and the returned generation consistent. +// The credential is authenticated before any deployment state is reported. func (s *Store) RuntimeNodeConfiguration(ctx context.Context, nodeID, token string) (RuntimeNodeConfiguration, error) { var result RuntimeNodeConfiguration - err := s.runtimeManagerTransaction(ctx, func(q *sqlc.Queries, d sqlc.RuntimeDeployment) error { - if d.Mode != "nodes" { - return ErrSandboxDeploymentConflict - } - spec, err := deploymentSpecification(d) - if err != nil { - return err - } + err := s.runtimeDeploymentTransaction(ctx, func(q *sqlc.Queries, d sqlc.RuntimeDeployment) error { + var node *sqlc.RuntimeNode + var installation pgtype.UUID var active, retained int32 if nodeID == "" { r, err := q.GetRuntimeEnrollment(ctx, runtimeTokenDigest(token)) - if err != nil || r.ConsumedAt.Valid || !r.ExpiresAt.Time.After(time.Now()) || r.InstallationID != d.InstallationID { + if err != nil || r.ConsumedAt.Valid || !r.ExpiresAt.Time.After(time.Now()) { return ErrRuntimeNodeCredential } - active, retained = r.MaxActive, r.MaxRetained - if d.Maintenance { - return ErrSandboxDeploymentConflict - } + installation, active, retained = r.InstallationID, r.MaxActive, r.MaxRetained } else { id, err := parseConnectionGeneration(nodeID) if err != nil { return ErrRuntimeNodeCredential } n, err := q.GetRuntimeNode(ctx, id) - if err != nil || n.InstallationID != d.InstallationID || n.CredentialSha256 != runtimeTokenDigest(token) { + if err != nil || n.CredentialSha256 != runtimeTokenDigest(token) { return ErrRuntimeNodeCredential } - active, retained = n.MaxActive, n.MaxRetained - if n.DeploymentGeneration != d.Generation || n.SpecificationDigest != spec.Digest(d.ProviderKind) { - return ErrRuntimeSpecificationMismatch - } + node, installation, active, retained = &n, n.InstallationID, n.MaxActive, n.MaxRetained + } + // A claimed installation rejects foreign credentials identically before + // and after initialization. + if d.InstallationID.Valid && installation != d.InstallationID { + return ErrRuntimeNodeCredential + } + if !runtimeDeploymentInitialized(d) { + return ErrRuntimeNodeUnavailable + } + if d.Mode != "nodes" { + return ErrSandboxDeploymentConflict + } + spec, err := deploymentSpecification(d) + if err != nil { + return err + } + if node == nil && d.Maintenance { + return ErrSandboxDeploymentConflict + } + if node != nil && (node.DeploymentGeneration != d.Generation || node.SpecificationDigest != spec.Digest(d.ProviderKind)) { + return ErrRuntimeSpecificationMismatch } result = RuntimeNodeConfiguration{MaxActive: int(active), MaxRetained: retainedLimit(d.ProviderKind, int(active), int(retained)), InstallationID: runtimeUUID(d.InstallationID), Provider: d.ProviderKind, CoreURL: d.CoreUrl, Generation: uint64(d.Generation), Specification: spec, SpecificationDigest: spec.Digest(d.ProviderKind)} return nil diff --git a/services/agents-api/internal/store/runtime_nodes.go b/services/agents-api/internal/store/runtime_nodes.go index 56a3b34bf..a4fdcc4b6 100644 --- a/services/agents-api/internal/store/runtime_nodes.go +++ b/services/agents-api/internal/store/runtime_nodes.go @@ -40,18 +40,31 @@ func validateRuntimeNode(name string, active, retained int) error { return nil } func (s *Store) runtimeManagerTransaction(ctx context.Context, apply func(*sqlc.Queries, sqlc.RuntimeDeployment) error) error { + return s.runtimeDeploymentTransaction(ctx, func(q *sqlc.Queries, d sqlc.RuntimeDeployment) error { + if !runtimeDeploymentInitialized(d) { + return ErrRuntimeNodeUnavailable + } + return apply(q, d) + }) +} + +// runtimeDeploymentTransaction locks the deployment whether or not it is +// initialized. Node machine routes use it to authenticate their credential +// before reporting any deployment state, including an uninitialized one. +func (s *Store) runtimeDeploymentTransaction(ctx context.Context, apply func(*sqlc.Queries, sqlc.RuntimeDeployment) error) error { return pgx.BeginFunc(ctx, s.pool, func(tx pgx.Tx) error { q := s.queries.WithTx(tx) deployment, err := q.LockRuntimeDeployment(ctx) if err != nil { return err } - if !deployment.InstallationID.Valid || deployment.ProviderKind == "" { - return ErrRuntimeNodeUnavailable - } return apply(q, deployment) }) } + +func runtimeDeploymentInitialized(d sqlc.RuntimeDeployment) bool { + return d.InstallationID.Valid && d.ProviderKind != "" +} func (s *Store) GetRuntimeDeployment(ctx context.Context) (RuntimeDeploymentView, error) { return getRuntimeDeploymentView(ctx, s.queries) } @@ -140,7 +153,7 @@ func (s *Store) EnrollRuntimeNode(ctx context.Context, token string, input Runti return RuntimeNodeIdentity{}, err } var result RuntimeNodeIdentity - err = s.runtimeManagerTransaction(ctx, func(q *sqlc.Queries, d sqlc.RuntimeDeployment) error { + err = s.runtimeDeploymentTransaction(ctx, func(q *sqlc.Queries, d sqlc.RuntimeDeployment) error { receipt, err := q.GetRuntimeEnrollment(ctx, runtimeTokenDigest(token)) if errors.Is(err, pgx.ErrNoRows) { return ErrRuntimeNodeCredential @@ -148,9 +161,15 @@ func (s *Store) EnrollRuntimeNode(ctx context.Context, token string, input Runti if err != nil { return ErrRuntimeNodeUnavailable } - if receipt.ConsumedAt.Valid || !receipt.ExpiresAt.Time.After(time.Now()) || receipt.InstallationID != d.InstallationID { + if receipt.ConsumedAt.Valid || !receipt.ExpiresAt.Time.After(time.Now()) { + return ErrRuntimeNodeCredential + } + if d.InstallationID.Valid && receipt.InstallationID != d.InstallationID { return ErrRuntimeNodeCredential } + if !runtimeDeploymentInitialized(d) { + return ErrRuntimeNodeUnavailable + } if d.Mode != "nodes" || d.Maintenance || input.Provider != d.ProviderKind { return ErrInvalidInput } diff --git a/services/agents-api/internal/store/sandbox_node_auth_order_http_test.go b/services/agents-api/internal/store/sandbox_node_auth_order_http_test.go new file mode 100644 index 000000000..5a44574e9 --- /dev/null +++ b/services/agents-api/internal/store/sandbox_node_auth_order_http_test.go @@ -0,0 +1,91 @@ +package store_test + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/MiniMax-AI-Dev/parsar/internal/agentdaemon/device" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/api" + "github.com/MiniMax-AI-Dev/parsar/services/agents-api/internal/store" + "github.com/google/uuid" +) + +// Before the sandbox deployment is initialized, node machine routes still +// authenticate first: a missing or invalid credential gets 401, and only a +// recognized credential learns that the deployment is unavailable. +func TestSandboxNodeRoutesAuthenticateBeforeDeploymentState(t *testing.T) { + s, pool := store.NewManagedTestStore(t) + admin, err := api.NewDeploymentAuthenticator([]string{device.HashCredential(uuid.NewString())}) + if err != nil { + t.Fatal(err) + } + auth, err := api.NewDatabaseAuthenticator(s) + if err != nil { + t.Fatal(err) + } + handler, err := api.NewHandler(s, auth, "codex", api.WithSandboxManager(s, admin)) + if err != nil { + t.Fatal(err) + } + // Recognized, unconsumed enrollment tokens; no deployment has been initialized. + enrollment := func(token, installation string) { + t.Helper() + if _, err := pool.Exec(t.Context(), "INSERT INTO runtime_node_enrollments(token_sha256,installation_id,expires_at) VALUES(encode(sha256($1::bytea),'hex'),$2,clock_timestamp()+interval '10 minutes')", token, installation); err != nil { + t.Fatal(err) + } + } + token, claimedToken, claimed := strings.Repeat("e", 64), strings.Repeat("c", 64), uuid.NewString() + enrollment(token, uuid.NewString()) + enrollment(claimedToken, claimed) + enroll, _ := json.Marshal(store.RuntimeNodeEnrollment{NodeID: uuid.NewString(), Credential: strings.Repeat("n", 64), Name: "Early node", Provider: "docker", + BackendFingerprint: strings.Repeat("b", 64), DeploymentGeneration: 1, SpecificationDigest: strings.Repeat("d", 64)}) + nodeID := uuid.NewString() + type check struct { + name, method, path, authorization, nodeHeader, body string + want int + } + run := func(checks []check) { + t.Helper() + for _, test := range checks { + r := httptest.NewRequest(test.method, test.path, strings.NewReader(test.body)) + if test.authorization != "" { + r.Header.Set("Authorization", test.authorization) + } + if test.nodeHeader != "" { + r.Header.Set("X-Parsar-Node-ID", test.nodeHeader) + } + r.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + handler.ServeHTTP(w, r) + if w.Code != test.want { + t.Errorf("%s = %d, want %d: %s", test.name, w.Code, test.want, w.Body) + } + } + } + run([]check{ + {"configuration without credential", "GET", "/api/v1/sandbox-node/configuration", "", "", "", http.StatusUnauthorized}, + {"configuration with invalid token", "GET", "/api/v1/sandbox-node/configuration", "Bearer invalid", "", "", http.StatusUnauthorized}, + {"configuration with invalid node credential", "GET", "/api/v1/sandbox-node/configuration", "Bearer invalid", nodeID, "", http.StatusUnauthorized}, + {"configuration with recognized token", "GET", "/api/v1/sandbox-node/configuration", "Bearer " + token, "", "", http.StatusServiceUnavailable}, + {"enroll without credential", "POST", "/api/v1/sandbox-node/enroll", "", "", string(enroll), http.StatusUnauthorized}, + {"enroll with invalid token", "POST", "/api/v1/sandbox-node/enroll", "Bearer invalid", "", string(enroll), http.StatusUnauthorized}, + {"enroll with recognized token", "POST", "/api/v1/sandbox-node/enroll", "Bearer " + token, "", string(enroll), http.StatusServiceUnavailable}, + {"identity without credential", "GET", "/api/v1/sandbox-node/identity?node_id=" + nodeID, "", "", "", http.StatusUnauthorized}, + {"identity with invalid credential", "GET", "/api/v1/sandbox-node/identity?node_id=" + nodeID, "Bearer invalid", "", "", http.StatusUnauthorized}, + }) + + // Once Web claims an installation, still before initialization, another + // installation's token gets the same 401 it gets after initialization. + if _, err := pool.Exec(t.Context(), "UPDATE runtime_deployment SET installation_id=$1, web_managed=true WHERE singleton=true", claimed); err != nil { + t.Fatal(err) + } + run([]check{ + {"configuration with foreign token", "GET", "/api/v1/sandbox-node/configuration", "Bearer " + token, "", "", http.StatusUnauthorized}, + {"enroll with foreign token", "POST", "/api/v1/sandbox-node/enroll", "Bearer " + token, "", string(enroll), http.StatusUnauthorized}, + {"configuration with claimed token", "GET", "/api/v1/sandbox-node/configuration", "Bearer " + claimedToken, "", "", http.StatusServiceUnavailable}, + {"enroll with claimed token", "POST", "/api/v1/sandbox-node/enroll", "Bearer " + claimedToken, "", string(enroll), http.StatusServiceUnavailable}, + }) +} diff --git a/services/agents-api/internal/store/sandbox_specification_store_test.go b/services/agents-api/internal/store/sandbox_specification_store_test.go index 12577adaf..a772fc5c1 100644 --- a/services/agents-api/internal/store/sandbox_specification_store_test.go +++ b/services/agents-api/internal/store/sandbox_specification_store_test.go @@ -273,8 +273,9 @@ func TestSandboxSpecificationInitialCredentialRemainsPrivate(t *testing.T) { if err := s.pool.QueryRow(t.Context(), "SELECT e2b_credential FROM runtime_deployment").Scan(&stored); err != nil || len(stored) == 0 || bytes.Contains(stored, []byte(input.E2B.APIKey)) { t.Fatal("private credential was not encrypted", err) } - if _, err := s.RuntimeNodeConfiguration(t.Context(), "", input.E2B.APIKey); err == nil { - t.Fatal("cloud key authorized node bootstrap") + // The credential is rejected before the cloud deployment mode is reported. + if _, err := s.RuntimeNodeConfiguration(t.Context(), "", input.E2B.APIKey); !errors.Is(err, ErrRuntimeNodeCredential) { + t.Fatal("cloud key authorized node bootstrap", err) } }