From 9f633159ab1724f870b502c1dff16f8769b44fae Mon Sep 17 00:00:00 2001 From: Albert Mavashev Date: Mon, 20 Jul 2026 19:59:36 -0400 Subject: [PATCH] fix: enforce non-negative policy priority Reject negative policy priorities at the validated HTTP boundary, pin contract tests to governance spec 0.1.25.42, and document the compatible 0.1.25.55 server correction. --- AUDIT.md | 29 +++++++++++-- CHANGELOG.md | 13 ++++++ README.md | 2 +- .../api/contract/ContractSpecLoader.java | 2 +- .../api/controller/PolicyControllerTest.java | 28 +++++++++++++ .../model/policy/PolicyCreateRequest.java | 2 +- .../model/policy/PolicyUpdateRequest.java | 3 +- .../admin/model/PolicyModelTest.java | 41 +++++++++++++++++++ docs/contract-testing.md | 2 +- 9 files changed, 114 insertions(+), 8 deletions(-) diff --git a/AUDIT.md b/AUDIT.md index fce33b07..0c6d2874 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -1,8 +1,8 @@ # Complete Budget Governance v0.1.25.55 — Admin Server Audit **Spec:** -[`cycles-governance-admin-v0.1.25.yaml`](https://github.com/runcycles/cycles-protocol/blob/469840bb2f41ce35650c89405ea12fc56e847c76/cycles-governance-admin-v0.1.25.yaml) -(OpenAPI 3.1.0, info.version `0.1.25.41`; adds CASCADE SEMANTICS — Rule 1 `POST +[`cycles-governance-admin-v0.1.25.yaml`](https://github.com/runcycles/cycles-protocol/blob/402307a88906e9fd090159e5ccf2d0036e6aec83/cycles-governance-admin-v0.1.25.yaml) +(OpenAPI 3.1.0, info.version `0.1.25.42`; adds CASCADE SEMANTICS — Rule 1 `POST /admin/tenants/{id}` PATCH→CLOSED cascades owned budgets (→CLOSED), webhook subscriptions (→DISABLED), and API keys (→REVOKED) under a shared correlation_id — Rule 1 permits **Mode A (atomic)** or **Mode B @@ -36,7 +36,9 @@ not carry admin-only event types/categories); v0.1.25.41 (merged, cycles-protocol#130) narrows `replayEvents` success to selection-completeness + best-effort enqueue (all-or-narrow) and documents the `/test` synthetic-ping exception — .40 and .41 both implemented here in -0.1.25.51) in +0.1.25.51; v0.1.25.42 aligns policy write requests with the existing +non-negative `Policy.priority` response invariant — implemented here in +0.1.25.55) in [cycles-protocol](https://github.com/runcycles/cycles-protocol) **Server:** Spring Boot 3.5.16 / Java 21 / Jedis 7.5.2 · commons-lang3 3.18.0 @@ -44,6 +46,27 @@ pin · tomcat-embed-core 10.1.55 pin (re-introduced 2026-05-25 for Apache Tomcat CVE-2026-43512 / -43513 / -43514 / -43515 / -42498 / -41284 / -41293) +### 2026-07-20 — v0.1.25.55: enforce the policy priority contract + +Governance spec revision 0.1.25.42 resolves a request/response contradiction: +`Policy.priority` already had `minimum: 0`, while the create and update request +schemas admitted negative integers. The server mirrored that gap and could +persist a policy that its own successful response schema could not represent. + +- `PolicyCreateRequest.priority` and `PolicyUpdateRequest.priority` now enforce + `@Min(0)`. Negative writes fail at the HTTP boundary with `400 + INVALID_REQUEST` before the repository is called. +- Omitted, zero, and positive priorities are unchanged. Existing legacy rows + are not rewritten, and a PATCH that omits priority can still update another + field on a legacy row. +- Contract tests now pin cycles-protocol commit `402307a` so CI validates the + exact 0.1.25.42 schema rather than a moving branch. + +Focused model and controller verification passes 43 tests. A clean full +non-integration Maven verification passes 1,855 tests with zero failures or +errors. The 95% JaCoCo line and branch gates pass in every module: model 97.85% +line / 98.44% branch, data 97.48% / 95.35%, and API 97.47% / 95.05%. + ### 2026-07-18 — v0.1.25.54 published; production pins advanced; v0.1.25.55 opened GitHub release `v0.1.25.54` was published from merge commit `b88953b`. The diff --git a/CHANGELOG.md b/CHANGELOG.md index 73e88b67..80e5b56a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,19 @@ changes to request/response bodies or Lua-script semantics would require a minor bump. Additive fields (new optional response fields, new enum values, new optional request fields) are **not** considered breaking. +## [0.1.25.55] — 2026-07-20 + +### Fixed + +- Policy create and update requests now reject negative `priority` values with + `400 INVALID_REQUEST`, matching the existing non-negative `Policy.priority` + response contract. Omitted, zero, and positive values are unchanged. + +### Changed + +- Contract validation advances to governance spec revision 0.1.25.42 at the + reviewed cycles-protocol commit that defines the request-side minimum. + ## [0.1.25.54] — 2026-07-18 ### Security diff --git a/README.md b/README.md index 620fb8da..64fb7c40 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ **Administrative API for managing tenants, budgets, API keys, and policies in a Cycles deployment.** Configures the AI agent budget and action enforcement that the [Cycles Server](https://github.com/runcycles/cycles-server) applies at runtime. -Multi-tenant by default, with four integrated planes: tenant lifecycle and budget ledgers, API key authentication and permission enforcement, runtime reservation control, and event/webhook delivery for observability. Aligned with [Cycles Protocol v0.1.25.41](https://github.com/runcycles/cycles-protocol/blob/main/cycles-governance-admin-v0.1.25.yaml) (v0.1.25.35–.37 added the four cascade EventTypes + `EventDataTenantCascade` payload schema, the `admin_on_behalf_of` actor type, and `TENANT_CLOSED` in the reservation-denied reason-code documentation; v0.1.25.38 added the tenant self-service webhook `event_categories` boundary; v0.1.25.39 relaxed the webhook schema to permit **category-only** subscriptions, ratifying the behavior this server already shipped in 0.1.25.50; v0.1.25.40 added the **admin-plane webhook category boundary** (INVARIANT 2 — a tenant-owned subscription may not carry admin-only event types/categories) and v0.1.25.41 the **replay all-or-narrow semantics** (selection-completeness + best-effort enqueue) plus the **`/test` synthetic-ping exception**, both implemented here in 0.1.25.51 — all implemented or documentation-only here). Empty-both subscriptions remain rejected (create requires `event_types`; update requires at least one of `event_types`/`event_categories`). +Multi-tenant by default, with four integrated planes: tenant lifecycle and budget ledgers, API key authentication and permission enforcement, runtime reservation control, and event/webhook delivery for observability. Aligned with [Cycles Protocol v0.1.25.42](https://github.com/runcycles/cycles-protocol/blob/402307a88906e9fd090159e5ccf2d0036e6aec83/cycles-governance-admin-v0.1.25.yaml) (v0.1.25.35–.37 added the four cascade EventTypes + `EventDataTenantCascade` payload schema, the `admin_on_behalf_of` actor type, and `TENANT_CLOSED` in the reservation-denied reason-code documentation; v0.1.25.38 added the tenant self-service webhook `event_categories` boundary; v0.1.25.39 relaxed the webhook schema to permit **category-only** subscriptions, ratifying the behavior this server already shipped in 0.1.25.50; v0.1.25.40 added the **admin-plane webhook category boundary** (INVARIANT 2 — a tenant-owned subscription may not carry admin-only event types/categories); v0.1.25.41 added **replay all-or-narrow semantics** plus the **`/test` synthetic-ping exception**; and v0.1.25.42 aligns policy write requests with the existing non-negative priority invariant — all implemented or documentation-only here). Empty-both subscriptions remain rejected (create requires `event_types`; update requires at least one of `event_types`/`event_categories`). ## Documentation diff --git a/cycles-admin-service/cycles-admin-service-api/src/test/java/io/runcycles/admin/api/contract/ContractSpecLoader.java b/cycles-admin-service/cycles-admin-service-api/src/test/java/io/runcycles/admin/api/contract/ContractSpecLoader.java index b8f77ab7..a416fed4 100644 --- a/cycles-admin-service/cycles-admin-service-api/src/test/java/io/runcycles/admin/api/contract/ContractSpecLoader.java +++ b/cycles-admin-service/cycles-admin-service-api/src/test/java/io/runcycles/admin/api/contract/ContractSpecLoader.java @@ -27,7 +27,7 @@ */ public final class ContractSpecLoader { - public static final String SPEC_REVISION = "469840bb2f41ce35650c89405ea12fc56e847c76"; + public static final String SPEC_REVISION = "402307a88906e9fd090159e5ccf2d0036e6aec83"; public static final String DEFAULT_SPEC_URL = "https://raw.githubusercontent.com/runcycles/cycles-protocol/" + SPEC_REVISION + "/cycles-governance-admin-v0.1.25.yaml"; diff --git a/cycles-admin-service/cycles-admin-service-api/src/test/java/io/runcycles/admin/api/controller/PolicyControllerTest.java b/cycles-admin-service/cycles-admin-service-api/src/test/java/io/runcycles/admin/api/controller/PolicyControllerTest.java index 774d88a0..74b27351 100644 --- a/cycles-admin-service/cycles-admin-service-api/src/test/java/io/runcycles/admin/api/controller/PolicyControllerTest.java +++ b/cycles-admin-service/cycles-admin-service-api/src/test/java/io/runcycles/admin/api/controller/PolicyControllerTest.java @@ -76,6 +76,20 @@ void createPolicy_missingName_returns400() throws Exception { .andExpect(status().isBadRequest()); } + @Test + void createPolicy_negativePriority_returns400WithoutPersistence() throws Exception { + setupApiKeyAuth(); + + mockMvc.perform(post("/v1/admin/policies") + .header("X-Cycles-API-Key", "valid-api-key") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"name\":\"P\",\"scope_pattern\":\"*\",\"priority\":-1}")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("INVALID_REQUEST")); + + verify(policyRepository, never()).create(any(), any()); + } + @Test void createPolicy_noApiKey_returns401() throws Exception { mockMvc.perform(post("/v1/admin/policies") @@ -293,6 +307,20 @@ void updatePolicy_unknownFieldIsRejectedAtHttpBoundary() throws Exception { verify(policyRepository, never()).update(anyString(), anyString(), any()); } + @Test + void updatePolicy_negativePriority_returns400WithoutPersistence() throws Exception { + setupApiKeyAuth(); + + mockMvc.perform(patch("/v1/admin/policies/pol_1") + .header("X-Cycles-API-Key", "valid-api-key") + .contentType(MediaType.APPLICATION_JSON) + .content("{\"priority\":-1}")) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.error").value("INVALID_REQUEST")); + + verify(policyRepository, never()).update(anyString(), anyString(), any()); + } + // ========== PATCH /v1/admin/policies/{policy_id} ========== @Test diff --git a/cycles-admin-service/cycles-admin-service-model/src/main/java/io/runcycles/admin/model/policy/PolicyCreateRequest.java b/cycles-admin-service/cycles-admin-service-model/src/main/java/io/runcycles/admin/model/policy/PolicyCreateRequest.java index 86a6d4af..4d215ae1 100644 --- a/cycles-admin-service/cycles-admin-service-model/src/main/java/io/runcycles/admin/model/policy/PolicyCreateRequest.java +++ b/cycles-admin-service/cycles-admin-service-model/src/main/java/io/runcycles/admin/model/policy/PolicyCreateRequest.java @@ -16,7 +16,7 @@ public class PolicyCreateRequest { @NotBlank @Size(max = 256) @JsonProperty("name") private String name; @Size(max = 1024) @JsonProperty("description") private String description; @NotBlank @JsonProperty("scope_pattern") private String scopePattern; - @JsonProperty("priority") private Integer priority; + @Min(0) @JsonProperty("priority") private Integer priority; @Valid @JsonProperty("caps") private Caps caps; @JsonProperty("commit_overage_policy") private CommitOveragePolicy commitOveragePolicy; @Valid @JsonProperty("reservation_ttl_override") private ReservationTtlOverride reservationTtlOverride; diff --git a/cycles-admin-service/cycles-admin-service-model/src/main/java/io/runcycles/admin/model/policy/PolicyUpdateRequest.java b/cycles-admin-service/cycles-admin-service-model/src/main/java/io/runcycles/admin/model/policy/PolicyUpdateRequest.java index fcf8adb9..bd1ac53b 100644 --- a/cycles-admin-service/cycles-admin-service-model/src/main/java/io/runcycles/admin/model/policy/PolicyUpdateRequest.java +++ b/cycles-admin-service/cycles-admin-service-model/src/main/java/io/runcycles/admin/model/policy/PolicyUpdateRequest.java @@ -2,13 +2,14 @@ import io.runcycles.admin.model.shared.*; import com.fasterxml.jackson.annotation.JsonProperty; import jakarta.validation.Valid; +import jakarta.validation.constraints.Min; import lombok.*; import java.time.Instant; @Data @NoArgsConstructor @AllArgsConstructor public class PolicyUpdateRequest { @JsonProperty("name") private String name; @JsonProperty("description") private String description; - @JsonProperty("priority") private Integer priority; + @Min(0) @JsonProperty("priority") private Integer priority; @Valid @JsonProperty("caps") private Caps caps; @JsonProperty("commit_overage_policy") private CommitOveragePolicy commitOveragePolicy; @Valid @JsonProperty("reservation_ttl_override") private ReservationTtlOverride reservationTtlOverride; diff --git a/cycles-admin-service/cycles-admin-service-model/src/test/java/io/runcycles/admin/model/PolicyModelTest.java b/cycles-admin-service/cycles-admin-service-model/src/test/java/io/runcycles/admin/model/PolicyModelTest.java index f50ed21a..a24eefeb 100644 --- a/cycles-admin-service/cycles-admin-service-model/src/test/java/io/runcycles/admin/model/PolicyModelTest.java +++ b/cycles-admin-service/cycles-admin-service-model/src/test/java/io/runcycles/admin/model/PolicyModelTest.java @@ -4,6 +4,7 @@ import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException; import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import io.runcycles.admin.model.policy.PolicyCreateRequest; +import io.runcycles.admin.model.policy.PolicyUpdateRequest; import io.runcycles.admin.model.policy.RateLimits; import io.runcycles.admin.model.policy.ReservationTtlOverride; import jakarta.validation.*; @@ -52,6 +53,46 @@ void policyCreateRequest_descriptionTooLong_fails() { assertFalse(violations.isEmpty()); } + @Test + void policyCreateRequest_negativePriority_fails() { + PolicyCreateRequest req = new PolicyCreateRequest(); + req.setName("valid"); + req.setScopePattern("*"); + req.setPriority(-1); + + Set> violations = validator.validate(req); + + assertTrue(violations.stream().anyMatch(v -> "priority".equals(v.getPropertyPath().toString()))); + } + + @Test + void policyCreateRequest_zeroPriority_isValid() { + PolicyCreateRequest req = new PolicyCreateRequest(); + req.setName("valid"); + req.setScopePattern("*"); + req.setPriority(0); + + assertTrue(validator.validate(req).isEmpty()); + } + + @Test + void policyUpdateRequest_negativePriority_fails() { + PolicyUpdateRequest req = new PolicyUpdateRequest(); + req.setPriority(-1); + + Set> violations = validator.validate(req); + + assertTrue(violations.stream().anyMatch(v -> "priority".equals(v.getPropertyPath().toString()))); + } + + @Test + void policyUpdateRequest_zeroPriority_isValid() { + PolicyUpdateRequest req = new PolicyUpdateRequest(); + req.setPriority(0); + + assertTrue(validator.validate(req).isEmpty()); + } + @Test void rateLimits_rejectsUnknownFields() { String json = """ diff --git a/docs/contract-testing.md b/docs/contract-testing.md index 03d78aad..5d8dfe14 100644 --- a/docs/contract-testing.md +++ b/docs/contract-testing.md @@ -44,7 +44,7 @@ That's it. No per-test-method changes — the validator auto-applies to every `m ## Where the spec comes from -`ContractSpecLoader.loadSpec()` fetches the spec at cycles-protocol commit `469840bb2f41ce35650c89405ea12fc56e847c76` on first use and caches it under `target/contract/spec-.yaml` with a 1-hour TTL. Pinning makes PR results reproducible; use `-Dcontract.spec.url=...` while coordinating a spec upgrade, then advance the reviewed commit deliberately. +`ContractSpecLoader.loadSpec()` fetches the spec at cycles-protocol commit `402307a88906e9fd090159e5ccf2d0036e6aec83` on first use and caches it under `target/contract/spec-.yaml` with a 1-hour TTL. Pinning makes PR results reproducible; use `-Dcontract.spec.url=...` while coordinating a spec upgrade, then advance the reviewed commit deliberately. The nightly `contract-drift` job compares that reviewed revision's admin YAML with `cycles-protocol/main`. It fails on upstream drift, forcing a deliberate