From e414566ba7f914a28220ef4629fe4e1e758dce8f Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Fri, 25 Sep 2026 17:24:57 +0000 Subject: [PATCH 1/3] Accept saved Agent provider defaults in the admin Agent projection The console's Agents page failed for any Agent saved with x_agents_core.model_provider because the admin projection reused the Session Agent projection, which accepts only {harness}. Saved Agent reads return an optional harness and an optional safe provider view (protocol, base_url, optional limits, api_key_configured), and an empty object when neither is saved. Split the Core extension check: Session Agent snapshots keep the exact harness-only shape, and saved Agents reuse the execution-configuration provider validator. Unknown members, a null provider and any api_key are still rejected. --- .../agents-client/saved-agent-defaults.md | 2 + .../agents-client/src/admin-client.test.ts | 18 ++++++ .../agents-client/src/admin-projection.ts | 4 +- packages/agents-client/src/client.test.ts | 15 +++++ packages/agents-client/src/client.ts | 56 ++++++++++++++++--- .../src/execution-configuration-projection.ts | 3 +- 6 files changed, 88 insertions(+), 10 deletions(-) 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..1c4bf6408 100644 --- a/packages/agents-client/src/admin-client.test.ts +++ b/packages/agents-client/src/admin-client.test.ts @@ -177,6 +177,24 @@ 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("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..35dcf099c 100644 --- a/packages/agents-client/src/execution-configuration-projection.ts +++ b/packages/agents-client/src/execution-configuration-projection.ts @@ -13,7 +13,8 @@ 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 { +/** 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" || From 4049a4a858e338fd9927b992008db142b428dfd3 Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Fri, 25 Sep 2026 17:37:19 +0000 Subject: [PATCH 2/3] Authenticate sandbox node credentials before reporting deployment state Before the sandbox deployment was initialized, GET /api/v1/sandbox-node/configuration and POST /api/v1/sandbox-node/enroll answered any credential, invalid ones included, with 503 runtime_node_unavailable: the shared deployment lock rejected an uninitialized deployment before the route checked its credential. Configuration also reported a non-node (E2B) deployment as 409 before authenticating. Node routes now lock the deployment without that check, authenticate the enrollment token or node credential first, and only then report an uninitialized deployment (503), an installation mismatch (401) or the existing mode, maintenance and specification outcomes. Administrator routes keep the initialized-deployment check. Identity and the node connection already authenticated first and are unchanged. --- contracts/agents-api/sandbox-deployment.md | 7 +- .../store/runtime_node_configuration.go | 46 ++++++++----- .../internal/store/runtime_nodes.go | 29 ++++++-- .../sandbox_node_auth_order_http_test.go | 69 +++++++++++++++++++ .../store/sandbox_specification_store_test.go | 5 +- 5 files changed, 129 insertions(+), 27 deletions(-) create mode 100644 services/agents-api/internal/store/sandbox_node_auth_order_http_test.go diff --git a/contracts/agents-api/sandbox-deployment.md b/contracts/agents-api/sandbox-deployment.md index 01442b076..319bcd8c9 100644 --- a/contracts/agents-api/sandbox-deployment.md +++ b/contracts/agents-api/sandbox-deployment.md @@ -276,8 +276,11 @@ 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 +node credentials return 401 `invalid_node_credential`. Node machine routes check +the credential before any deployment state, so a missing or rejected credential +gets that 401 even before initialization or under E2B; a recognized credential +then gets 503 `runtime_node_unavailable` until the deployment is initialized. +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 diff --git a/services/agents-api/internal/store/runtime_node_configuration.go b/services/agents-api/internal/store/runtime_node_configuration.go index 3626ce589..3c11cda72 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,48 @@ 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 + } + if !runtimeDeploymentInitialized(d) { + return ErrRuntimeNodeUnavailable + } + if installation != d.InstallationID { + return ErrRuntimeNodeCredential + } + 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..420b9cb23 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,7 +161,13 @@ 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 !runtimeDeploymentInitialized(d) { + return ErrRuntimeNodeUnavailable + } + if receipt.InstallationID != d.InstallationID { return ErrRuntimeNodeCredential } if d.Mode != "nodes" || d.Maintenance || input.Provider != d.ProviderKind { 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..c8180fe94 --- /dev/null +++ b/services/agents-api/internal/store/sandbox_node_auth_order_http_test.go @@ -0,0 +1,69 @@ +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) + } + // A recognized, unconsumed enrollment token; no deployment has been initialized. + token := strings.Repeat("e", 64) + 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, uuid.NewString()); err != nil { + t.Fatal(err) + } + 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() + for _, test := range []struct { + name, method, path, authorization, nodeHeader, body string + want int + }{ + {"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}, + } { + 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) + } + } +} 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) } } From ca5c67634b7cedf872da41085138a5e0ac226469 Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Fri, 25 Sep 2026 18:31:59 +0000 Subject: [PATCH 3/3] Reject foreign node credentials before initialization and tolerate stored provider URLs A claimed but uninitialized installation now rejects an enrollment token or node credential issued for another installation with 401 before reporting 503, as it does after initialization, so the two deployment states are indistinguishable to such a caller. The contract states that only configuration and enroll answer an accepted credential with 503 before initialization. Core's provider base URL admission now requires a port in 1-65535 and a usable host: an IP address, or a domain with valid IDNA labels of letters, digits, hyphens and underscores and a non-numeric final label. The client's read check no longer parses the host with URL: it keeps the HTTPS, credential, query and fragment checks, so a base URL an earlier Core stored cannot fail a whole Agents list. --- contracts/agents-api/sandbox-deployment.md | 14 +++-- .../agents-api/v1/model_execution_test.go | 10 ++- .../agents-api/v1/model_provider_admission.go | 41 +++++++++++- .../agents-client/src/admin-client.test.ts | 11 ++++ .../src/execution-configuration-projection.ts | 21 +++++-- .../store/runtime_node_configuration.go | 8 ++- .../internal/store/runtime_nodes.go | 6 +- .../sandbox_node_auth_order_http_test.go | 62 +++++++++++++------ 8 files changed, 135 insertions(+), 38 deletions(-) diff --git a/contracts/agents-api/sandbox-deployment.md b/contracts/agents-api/sandbox-deployment.md index 319bcd8c9..3b138d52d 100644 --- a/contracts/agents-api/sandbox-deployment.md +++ b/contracts/agents-api/sandbox-deployment.md @@ -277,11 +277,15 @@ Malformed selections return 400; validated configuration diagnostics use incompatible deployment return 409 `sandbox_deployment_conflict`. A node configuration mismatch returns 409 `sandbox_specification_mismatch`; rejected node credentials return 401 `invalid_node_credential`. Node machine routes check -the credential before any deployment state, so a missing or rejected credential -gets that 401 even before initialization or under E2B; a recognized credential -then gets 503 `runtime_node_unavailable` until the deployment is initialized. -Unavailable provider preparation returns 503 `execution_unavailable`. Storage and credential failures -remain errors; an empty or failed read is not evidence of cleanup. +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/src/admin-client.test.ts b/packages/agents-client/src/admin-client.test.ts index 1c4bf6408..3f531086e 100644 --- a/packages/agents-client/src/admin-client.test.ts +++ b/packages/agents-client/src/admin-client.test.ts @@ -195,6 +195,17 @@ describe("AdminClient response contracts", () => { } }); + 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/execution-configuration-projection.ts b/packages/agents-client/src/execution-configuration-projection.ts index 35dcf099c..b288e9728 100644 --- a/packages/agents-client/src/execution-configuration-projection.ts +++ b/packages/agents-client/src/execution-configuration-projection.ts @@ -13,6 +13,22 @@ function selection(value: unknown, invalid: Invalid): SessionExecutionConfigurat return { value: value.value as string | null, source: value.source as ExecutionConfigurationSource }; } +// 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) || @@ -21,10 +37,7 @@ export function safeProvider(value: unknown, invalid: Invalid): ModelProviderVie (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 3c11cda72..a57d91c55 100644 --- a/services/agents-api/internal/store/runtime_node_configuration.go +++ b/services/agents-api/internal/store/runtime_node_configuration.go @@ -46,12 +46,14 @@ func (s *Store) RuntimeNodeConfiguration(ctx context.Context, nodeID, token stri } 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 installation != d.InstallationID { - return ErrRuntimeNodeCredential - } if d.Mode != "nodes" { return ErrSandboxDeploymentConflict } diff --git a/services/agents-api/internal/store/runtime_nodes.go b/services/agents-api/internal/store/runtime_nodes.go index 420b9cb23..a4fdcc4b6 100644 --- a/services/agents-api/internal/store/runtime_nodes.go +++ b/services/agents-api/internal/store/runtime_nodes.go @@ -164,12 +164,12 @@ func (s *Store) EnrollRuntimeNode(ctx context.Context, token string, input Runti 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 receipt.InstallationID != d.InstallationID { - return ErrRuntimeNodeCredential - } 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 index c8180fe94..5a44574e9 100644 --- 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 @@ -30,18 +30,42 @@ func TestSandboxNodeRoutesAuthenticateBeforeDeploymentState(t *testing.T) { if err != nil { t.Fatal(err) } - // A recognized, unconsumed enrollment token; no deployment has been initialized. - token := strings.Repeat("e", 64) - 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, uuid.NewString()); 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() - for _, test := range []struct { + 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}, @@ -51,19 +75,17 @@ func TestSandboxNodeRoutesAuthenticateBeforeDeploymentState(t *testing.T) { {"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}, - } { - 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) - } + }) + + // 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}, + }) }