Merge branch 'fix/approval-cascade-and-minrole' into 'main'

fix(approval): close 2 more bypass paths from security audit

See merge request 2-6/2-6-4/terravault/ordinis!182

ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/DictionaryDefinitionService.java

@@ -141,13 +141,24 @@ public class DictionaryDefinitionService {
     boolean schemaChanged = !java.util.Objects.equals(prevSchema, req.schemaJson());
     boolean tryingToDisableApproval =
         req.approvalRequired() != null && !req.approvalRequired();
-    if (existing.isApprovalRequired() && (schemaChanged || tryingToDisableApproval)) {
+    // Approval-min-role flip is a third backdoor: a maker with PUT permission
+    // could clear approvalMinRole (send "" → stored as null) and then approve
+    // their own restricted-dict drafts using a generic RECORD_REVIEWER role.
+    // Treat any change to approvalMinRole the same as a schema rewrite —
+    // route through the draft workflow (or eventually a dedicated admin
+    // endpoint that requires a higher role than `approvalMinRole` itself).
+    boolean tryingToChangeMinRole =
+        req.approvalMinRole() != null
+            && !java.util.Objects.equals(
+                req.approvalMinRole().isBlank() ? null : req.approvalMinRole(),
+                existing.getApprovalMinRole());
+    if (existing.isApprovalRequired()
+        && (schemaChanged || tryingToDisableApproval || tryingToChangeMinRole)) {
       throw OrdinisException.unprocessableEntity(
           "approval_required",
-          "Schema changes require the draft workflow on this dictionary. "
+          "Schema changes, approvalRequired toggle-off, and approvalMinRole "
-              + "Use POST /api/v1/admin/dictionaries/" + name
+              + "edits all require the draft workflow on this dictionary. "
-              + "/schema-drafts. Toggling approvalRequired off is also "
+              + "Use POST /api/v1/admin/dictionaries/" + name + "/schema-drafts.");
-              + "blocked here — needs an explicit admin endpoint.");
     }
 
     existing.setDisplayName(req.displayName());

ordinis-rest-api/src/main/java/cloud/nstart/terravault/ordinis/restapi/service/reference/CascadeCloseService.java

@@ -64,6 +64,10 @@ public class CascadeCloseService {
   private final LineageIndexService lineage;
   private final RecordCloseSink closeSink;
   private final ScopeContext scopeContext;
+  /** Used to enforce the approvalRequired gate on the target + every cascaded
+   *  dependent dict. Optional: null in legacy test ctors that don't exercise
+   *  the gate. Production wiring always provides it. */
+  private final DictionaryDefinitionService defService;
   /** Phase 4 SLO. Optional — null в test ctor'е. */
   private final Optional<MeterRegistry> meterRegistry;
 
@@ -79,6 +83,7 @@ public class CascadeCloseService {
     this(lineage,
         new DefaultRecordCloseSink(recordRepo, defService, auditLogger, outbox),
         scopeContext,
+        defService,
         meterRegistry);
   }
 
@@ -87,17 +92,19 @@ public class CascadeCloseService {
       LineageIndexService lineage,
       RecordCloseSink closeSink,
       ScopeContext scopeContext) {
-    this(lineage, closeSink, scopeContext, Optional.empty());
+    this(lineage, closeSink, scopeContext, null, Optional.empty());
   }
 
   CascadeCloseService(
       LineageIndexService lineage,
       RecordCloseSink closeSink,
       ScopeContext scopeContext,
+      DictionaryDefinitionService defService,
       Optional<MeterRegistry> meterRegistry) {
     this.lineage = lineage;
     this.closeSink = closeSink;
     this.scopeContext = scopeContext;
+    this.defService = defService;
     this.meterRegistry = meterRegistry;
   }
 
@@ -150,6 +157,22 @@ public class CascadeCloseService {
     OffsetDateTime when = closeAt == null ? OffsetDateTime.now() : closeAt;
     CascadePlan plan = evaluatePlan(targetDict, targetKey, when);
 
+    // Approval gate: cascade-close goes through closeSink which writes
+    // straight to the repo + outbox + audit, completely skipping
+    // DictionaryRecordService.close → requireApprovalGate. So we have to
+    // enforce here, against BOTH the target dict and every cascaded
+    // dependent. If any of them is approval-required, refuse the whole
+    // cascade — it cannot atomically transition to drafts mid-cascade,
+    // and partial cascade would leave dangling references.
+    // (Future work: auto-create one parent draft + N dependent drafts in
+    // a single submit-for-review bundle. Out of scope for the gap fix.)
+    if (defService != null) {
+      assertNotApprovalRequired(targetDict);
+      for (var dep : plan.cascade()) {
+        assertNotApprovalRequired(dep.sourceDict());
+      }
+    }
+
     if (!plan.blockers().isEmpty()) {
       stopTimerAndCount(sample, targetDict, "blocked");
       throw OrdinisException.conflict(
@@ -202,6 +225,31 @@ public class CascadeCloseService {
     }
   }
 
+  /**
+   * Block cascade-close from bypassing the maker-checker draft workflow on
+   * approval-required dictionaries. Throws 422 {@code approval_required}
+   * pointing the caller at the drafts API.
+   */
+  private void assertNotApprovalRequired(String dict) {
+    DictionaryDefinition d;
+    try {
+      d = defService.findByName(dict);
+    } catch (RuntimeException e) {
+      // If we can't resolve the dict we definitely shouldn't proceed.
+      throw OrdinisException.badRequest(
+          "dictionary_not_found",
+          "Cascade close: cannot resolve dictionary " + dict);
+    }
+    if (d.isApprovalRequired()) {
+      throw OrdinisException.unprocessableEntity(
+          "approval_required",
+          "Cascade close cannot proceed: dictionary " + dict
+              + " requires the draft workflow. Open a record draft via "
+              + "POST /api/v1/dictionaries/" + dict + "/records/{businessKey}/drafts "
+              + "with operation=CLOSE for each affected record.");
+    }
+  }
+
   // === Metrics helpers ===
 
   private Timer.Sample startTimer() {