From ebee0cc61388aff34ead1be23d6412e185da2f85 Mon Sep 17 00:00:00 2001 From: Ryan Min Date: Sun, 9 Aug 2026 23:12:42 +0900 Subject: [PATCH 1/7] feat: serve the delegated Basic edit route the external tool CLI calls The CLI has always posted to /api/v1/strategies/{id}/basic-edits/{preview,apply} and no such route existed. DelegatedBasicStrategyEditService and its jOOQ adapter were complete but nothing outside their own package referenced them, so an external AI tool could authenticate, create a strategy, and then fail with 404 at the only step that writes blocks. Both CLI test classes drive a stub HTTP server, which is why this survived: a stub agrees with itself about a path that is not served. DelegatedBasicEditRouteRegistrationTest pins the paths against a rename or a removed controller. The reviewed edit sequence now makes the round trip. The service requires expectedEditSequence and the CLI never sent one; having the controller re-read it would have let an owner edit landing between preview and apply be overwritten by a diff nobody reviewed against it, defeating the optimistic lock in rule 9.10. The preview reports the sequence it read, apply must return it, and a request carrying a preview hash without a sequence is refused. Scope denial and preview mismatch became subtypes of the existing rejection so the external tool contract can keep its promise of distinct exit codes (4 and 5) with machine-readable codes; every existing catch and test is unchanged. Their advice is a separate RestControllerAdvice because those codes are a promise to delegated tools only, and widening the authoring advice would extend it to owner-facing endpoints by accident. Refs #479 Co-Authored-By: Claude Opus 5 --- .../DelegatedBasicEditController.java | 162 ++++++++++++++++++ .../DelegatedBasicEditExceptionHandler.java | 57 ++++++ .../strategy/StrategyDraftConfiguration.java | 18 +- ...legatedBasicEditRouteRegistrationTest.java | 102 +++++++++++ apps/idea2strategy-cli/README.md | 5 +- .../idea2strategy/cli/Idea2StrategyCli.java | 17 +- .../idea2strategy-ai-tool-contract.json | 6 +- .../cli/ExternalAiToolE2eTest.java | 16 +- .../cli/Idea2StrategyCliTest.java | 22 ++- ...atedBasicEditPreviewMismatchException.java | 8 + .../DelegatedBasicEditRejectedException.java | 10 +- .../DelegatedBasicStrategyEditService.java | 4 +- ...DelegatedStrategyScopeDeniedException.java | 8 + ...DelegatedBasicStrategyEditJooqAdapter.java | 4 +- 14 files changed, 424 insertions(+), 15 deletions(-) create mode 100644 apps/backend-api/src/main/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditController.java create mode 100644 apps/backend-api/src/main/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditExceptionHandler.java create mode 100644 apps/backend-api/src/test/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditRouteRegistrationTest.java create mode 100644 modules/backend-application/src/main/java/com/idea2strategy/backend/application/strategy/DelegatedBasicEditPreviewMismatchException.java create mode 100644 modules/backend-application/src/main/java/com/idea2strategy/backend/application/strategy/DelegatedStrategyScopeDeniedException.java diff --git a/apps/backend-api/src/main/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditController.java b/apps/backend-api/src/main/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditController.java new file mode 100644 index 00000000..dbcdeb6f --- /dev/null +++ b/apps/backend-api/src/main/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditController.java @@ -0,0 +1,162 @@ +package com.idea2strategy.backend.api.strategy; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.idea2strategy.backend.application.common.CurrentPrincipal; +import com.idea2strategy.backend.application.strategy.BasicStrategyCatalogQueryService; +import com.idea2strategy.backend.application.strategy.DelegatedBasicEditOperation; +import com.idea2strategy.backend.application.strategy.DelegatedBasicEditPreview; +import com.idea2strategy.backend.application.strategy.DelegatedBasicStrategyEditService; +import com.idea2strategy.backend.application.strategy.DelegatedStrategyEditor; +import com.idea2strategy.backend.application.strategy.StrategyDocumentQueryService; +import com.idea2strategy.backend.domain.strategy.StrategyDocument; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +/** + * Operation-level Basic editing for delegated external tools. + * + *

The owner-facing editor writes whole documents through {@link StrategyDocumentController}. A + * delegated tool may only submit the four official operations, and only through a reviewed preview: + * {@code apply} recomputes the preview and refuses anything whose hash differs from the one the + * caller reviewed. The edit sequence travels the same round trip so a delegated apply cannot + * silently overwrite a concurrent owner edit. + */ +@RestController +@RequestMapping("/api/v1/strategies/{strategyId}/basic-edits") +@ConditionalOnProperty(name = {"spring.datasource.url", "identity.crypto.customer-jwt-signing-key"}) +public class DelegatedBasicEditController { + private final DelegatedBasicStrategyEditService editService; + private final BasicStrategyCatalogQueryService catalogService; + private final StrategyDocumentQueryService documentQueryService; + private final CurrentPrincipal principal; + private final ObjectMapper objectMapper = new ObjectMapper(); + + public DelegatedBasicEditController( + DelegatedBasicStrategyEditService editService, + BasicStrategyCatalogQueryService catalogService, + StrategyDocumentQueryService documentQueryService, + CurrentPrincipal principal) { + this.editService = editService; + this.catalogService = catalogService; + this.documentQueryService = documentQueryService; + this.principal = principal; + } + + @PostMapping("/preview") + public PreviewResponse preview( + @PathVariable UUID strategyId, + @RequestBody DelegatedBasicEditRequest request) { + long expectedEditSequence = resolveExpectedEditSequence(strategyId, request); + DelegatedBasicEditPreview preview = editService.preview( + editor(request), + strategyId, + expectedEditSequence, + catalogService.getLatestPublished(), + operations(request)); + return new PreviewResponse( + preview.beforeHash(), + preview.previewHash(), + readJson(preview.proposedSemanticDocument()), + preview.changes(), + preview.valid(), + expectedEditSequence); + } + + @PostMapping("/apply") + public AppliedResponse apply( + @PathVariable UUID strategyId, + @RequestBody DelegatedBasicEditRequest request) { + if (request.previewHash() == null || request.previewHash().isBlank()) { + throw new IllegalArgumentException("A reviewed preview hash is required to apply an edit"); + } + StrategyDocument applied = editService.apply( + editor(request), + strategyId, + resolveExpectedEditSequence(strategyId, request), + catalogService.getLatestPublished(), + operations(request), + request.previewHash()); + return new AppliedResponse( + applied.strategyId(), + applied.semanticHash(), + applied.editSequence(), + applied.updatedAt()); + } + + /** + * The owner-facing client always knows the sequence it read. The CLI learns it from the preview + * response and returns it on apply, so an omitted value is only ever the first preview of a + * round trip; reading it here would defeat the optimistic lock on apply. + */ + private long resolveExpectedEditSequence(UUID strategyId, DelegatedBasicEditRequest request) { + if (request.expectedEditSequence() != null) { + return request.expectedEditSequence(); + } + if (request.previewHash() != null && !request.previewHash().isBlank()) { + throw new IllegalArgumentException( + "An applied edit must carry the edit sequence returned by its preview"); + } + return documentQueryService.getOwned(strategyId).editSequence(); + } + + private DelegatedStrategyEditor editor(DelegatedBasicEditRequest request) { + if (request.authorizationId() == null || request.credentialId() == null) { + throw new IllegalArgumentException("Delegated authorization and credential are required"); + } + return new DelegatedStrategyEditor( + principal.accountId(), request.authorizationId(), request.credentialId()); + } + + private List operations(DelegatedBasicEditRequest request) { + if (request.operations() == null || request.operations().isEmpty()) { + throw new IllegalArgumentException("At least one edit operation is required"); + } + return request.operations().stream() + .map(operation -> new DelegatedBasicEditOperation( + operation.action(), + operation.arguments() == null ? Map.of() : operation.arguments())) + .toList(); + } + + private Map readJson(String value) { + try { + return objectMapper.readValue(value, new TypeReference<>() {}); + } catch (JsonProcessingException exception) { + throw new IllegalStateException("Proposed strategy document is invalid", exception); + } + } + + public record DelegatedBasicEditRequest( + UUID authorizationId, + UUID credentialId, + Long expectedEditSequence, + String previewHash, + List operations) { + @Override + public String toString() { + return "DelegatedBasicEditRequest[credential=REDACTED]"; + } + } + + public record OperationRequest(String action, Map arguments) {} + + public record PreviewResponse( + String beforeHash, + String previewHash, + Map diff, + List changes, + boolean valid, + long expectedEditSequence) {} + + public record AppliedResponse( + UUID strategyId, String semanticHash, long editSequence, java.time.Instant updatedAt) {} +} diff --git a/apps/backend-api/src/main/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditExceptionHandler.java b/apps/backend-api/src/main/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditExceptionHandler.java new file mode 100644 index 00000000..970e9ed0 --- /dev/null +++ b/apps/backend-api/src/main/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditExceptionHandler.java @@ -0,0 +1,57 @@ +package com.idea2strategy.backend.api.strategy; + +import com.idea2strategy.backend.application.strategy.DelegatedBasicEditPreviewMismatchException; +import com.idea2strategy.backend.application.strategy.DelegatedBasicEditRejectedException; +import com.idea2strategy.backend.application.strategy.DelegatedStrategyScopeDeniedException; +import java.util.NoSuchElementException; +import org.springframework.http.HttpStatus; +import org.springframework.http.ProblemDetail; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.RestControllerAdvice; + +/** + * Refusals for the delegated edit route only. + * + *

Kept separate from {@link StrategyAuthoringExceptionHandler} because these responses carry a + * machine-readable {@code code}: the external tool contract promises stable reasons, not only + * stable HTTP statuses, and an AI tool decides whether to stop, re-preview, or ask the user from + * that field. Owner-facing authoring endpoints make no such promise, and widening the existing + * advice would extend one to them by accident. + */ +@RestControllerAdvice(assignableTypes = DelegatedBasicEditController.class) +public class DelegatedBasicEditExceptionHandler { + @ExceptionHandler(DelegatedStrategyScopeDeniedException.class) + ProblemDetail scopeDenied(DelegatedStrategyScopeDeniedException exception) { + return problem(HttpStatus.FORBIDDEN, "SCOPE_DENIED", "Delegated scope denied", exception); + } + + @ExceptionHandler(DelegatedBasicEditPreviewMismatchException.class) + ProblemDetail previewMismatch(DelegatedBasicEditPreviewMismatchException exception) { + return problem(HttpStatus.CONFLICT, "PREVIEW_MISMATCH", "Reviewed preview mismatch", exception); + } + + @ExceptionHandler(DelegatedBasicEditRejectedException.class) + ProblemDetail editRejected(DelegatedBasicEditRejectedException exception) { + return problem( + HttpStatus.UNPROCESSABLE_ENTITY, "EDIT_REJECTED", "Delegated edit rejected", exception); + } + + @ExceptionHandler(IllegalArgumentException.class) + ProblemDetail invalidRequest(IllegalArgumentException exception) { + return problem(HttpStatus.BAD_REQUEST, "INVALID_REQUEST", "Invalid delegated edit", exception); + } + + @ExceptionHandler(NoSuchElementException.class) + ProblemDetail notFound(NoSuchElementException exception) { + return problem(HttpStatus.NOT_FOUND, "STRATEGY_NOT_FOUND", "Strategy not found", exception); + } + + private static ProblemDetail problem( + HttpStatus status, String code, String title, RuntimeException exception) { + ProblemDetail problem = ProblemDetail.forStatusAndDetail(status, exception.getMessage()); + problem.setTitle(title); + problem.setProperty("code", code); + problem.setProperty("message", exception.getMessage()); + return problem; + } +} diff --git a/apps/backend-api/src/main/java/com/idea2strategy/backend/api/strategy/StrategyDraftConfiguration.java b/apps/backend-api/src/main/java/com/idea2strategy/backend/api/strategy/StrategyDraftConfiguration.java index 23e99956..b7128438 100644 --- a/apps/backend-api/src/main/java/com/idea2strategy/backend/api/strategy/StrategyDraftConfiguration.java +++ b/apps/backend-api/src/main/java/com/idea2strategy/backend/api/strategy/StrategyDraftConfiguration.java @@ -4,6 +4,7 @@ import com.idea2strategy.backend.application.strategy.BasicStrategyDraftCommandService; import com.idea2strategy.backend.application.strategy.BasicStructureCatalogQueryService; import com.idea2strategy.backend.application.strategy.BasicStrategyValidationCommandService; +import com.idea2strategy.backend.application.strategy.DelegatedBasicStrategyEditService; import com.idea2strategy.backend.application.strategy.SecureStrategyEditLeaseTokenGenerator; import com.idea2strategy.backend.application.strategy.StrategyCopyCommandService; import com.idea2strategy.backend.application.strategy.StrategyDocumentQueryService; @@ -12,6 +13,7 @@ import com.idea2strategy.backend.application.strategy.StrategyReleaseInputCatalogQueryService; import com.idea2strategy.backend.persistence.strategy.BasicStrategyDraftJpaCommandAdapter; import com.idea2strategy.backend.persistence.strategy.BasicStructureCatalogJooqQueryAdapter; +import com.idea2strategy.backend.persistence.strategy.DelegatedBasicStrategyEditJooqAdapter; import com.idea2strategy.backend.persistence.strategy.StrategyDocumentJpaEntity; import com.idea2strategy.backend.persistence.strategy.StrategyDocumentJooqQueryAdapter; import com.idea2strategy.backend.persistence.strategy.StrategyDocumentSpringDataRepository; @@ -57,7 +59,8 @@ StrategyEditLeaseJpaCommandAdapter.class, StrategyValidationRunJpaCommandAdapter.class, StrategyValidationRunJooqQueryAdapter.class, - StrategyReleaseInputCatalogJooqQueryAdapter.class + StrategyReleaseInputCatalogJooqQueryAdapter.class, + DelegatedBasicStrategyEditJooqAdapter.class }) public class StrategyDraftConfiguration { @Bean @@ -147,6 +150,19 @@ StrategyValidationQueryService strategyValidationQueryService( return new StrategyValidationQueryService(validationQueryAdapter, documentQueryAdapter, principal); } + @Bean + DelegatedBasicStrategyEditService delegatedBasicStrategyEditService( + StrategyJooqQueryAdapter strategyQueryAdapter, + StrategyDocumentJooqQueryAdapter documentQueryAdapter, + DelegatedBasicStrategyEditJooqAdapter delegatedEditAdapter) { + return new DelegatedBasicStrategyEditService( + strategyQueryAdapter, + documentQueryAdapter, + delegatedEditAdapter, + delegatedEditAdapter, + Clock.systemUTC()); + } + @Bean StrategyReleaseInputCatalogQueryService strategyReleaseInputCatalogQueryService( StrategyReleaseInputCatalogJooqQueryAdapter queryAdapter) { diff --git a/apps/backend-api/src/test/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditRouteRegistrationTest.java b/apps/backend-api/src/test/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditRouteRegistrationTest.java new file mode 100644 index 00000000..131f0328 --- /dev/null +++ b/apps/backend-api/src/test/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditRouteRegistrationTest.java @@ -0,0 +1,102 @@ +package com.idea2strategy.backend.api.strategy; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +import com.idea2strategy.backend.application.common.CurrentPrincipal; +import com.idea2strategy.backend.application.strategy.BasicStrategyCatalogQueryService; +import com.idea2strategy.backend.application.strategy.DelegatedBasicStrategyEditService; +import com.idea2strategy.backend.application.strategy.StrategyDocumentQueryService; +import java.util.Arrays; +import java.util.List; +import org.junit.jupiter.api.Test; +import org.springframework.boot.test.context.runner.ApplicationContextRunner; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestMapping; + +/** + * Pins the routes the external tool CLI calls. + * + *

This test exists because these endpoints were absent from the API for the whole life of the + * CLI and nobody noticed: the CLI's own tests drive a stub HTTP server, so they proved the client + * sent a correct request to a path that did not exist. A stub can only agree with itself. The path + * strings below are duplicated from {@code Idea2StrategyCli} on purpose — a rename on either side + * must break a test rather than a released tool. + */ +class DelegatedBasicEditRouteRegistrationTest { + private static final String BASE = "/api/v1/strategies/{strategyId}/basic-edits"; + + private final ApplicationContextRunner contextRunner = new ApplicationContextRunner() + .withUserConfiguration(DelegatedEditDependencies.class, DelegatedBasicEditController.class) + .withPropertyValues( + "spring.datasource.url=jdbc:postgresql://unused/test", + "identity.crypto.customer-jwt-signing-key=test-customer-jwt-signing-key"); + + @Test + void registersTheDelegatedEditControllerWhenTheStrategyDraftModuleIsEnabled() { + contextRunner.run(context -> { + assertThat(context).hasNotFailed(); + assertThat(context).hasSingleBean(DelegatedBasicEditController.class); + }); + } + + @Test + void keepsTheControllerDisabledWhenTheStrategyDraftModuleIsDisabled() { + new ApplicationContextRunner() + .withUserConfiguration(DelegatedEditDependencies.class, DelegatedBasicEditController.class) + .run(context -> { + assertThat(context).hasNotFailed(); + assertThat(context).doesNotHaveBean(DelegatedBasicEditController.class); + }); + } + + @Test + void refusalAdviceStaysScopedToTheDelegatedRoute() { + var advice = DelegatedBasicEditExceptionHandler.class + .getAnnotation(org.springframework.web.bind.annotation.RestControllerAdvice.class); + + assertThat(advice.assignableTypes()).containsExactly(DelegatedBasicEditController.class); + } + + @Test + void exposesExactlyThePathsTheExternalToolCliCalls() { + RequestMapping base = DelegatedBasicEditController.class.getAnnotation(RequestMapping.class); + assertThat(base).isNotNull(); + assertThat(base.value()).containsExactly(BASE); + + assertThat(postMappings()).containsExactlyInAnyOrder("/preview", "/apply"); + } + + private static List postMappings() { + return Arrays.stream(DelegatedBasicEditController.class.getDeclaredMethods()) + .map(method -> method.getAnnotation(PostMapping.class)) + .filter(mapping -> mapping != null) + .flatMap(mapping -> Arrays.stream(mapping.value())) + .toList(); + } + + @Configuration(proxyBeanMethods = false) + static class DelegatedEditDependencies { + @Bean + DelegatedBasicStrategyEditService delegatedBasicStrategyEditService() { + return mock(DelegatedBasicStrategyEditService.class); + } + + @Bean + BasicStrategyCatalogQueryService basicStrategyCatalogQueryService() { + return mock(BasicStrategyCatalogQueryService.class); + } + + @Bean + StrategyDocumentQueryService strategyDocumentQueryService() { + return mock(StrategyDocumentQueryService.class); + } + + @Bean + CurrentPrincipal currentPrincipal() { + return mock(CurrentPrincipal.class); + } + } +} diff --git a/apps/idea2strategy-cli/README.md b/apps/idea2strategy-cli/README.md index 425ed038..3b064498 100644 --- a/apps/idea2strategy-cli/README.md +++ b/apps/idea2strategy-cli/README.md @@ -30,6 +30,7 @@ strategy create --name NAME [--description TEXT] strategy copy --strategy-id ID --name NAME strategy edit preview --strategy-id ID --authorization-id ID --credential-id ID --operations-file FILE strategy edit apply --strategy-id ID --authorization-id ID --credential-id ID --operations-file FILE --preview-hash HASH + --expected-edit-sequence SEQUENCE strategy validate --strategy-id ID strategy release --strategy-id ID --validation-run-id ID --initial-cash-amount AMOUNT --budget-cap-bps BPS --broker-rules-version VERSION --accounting-rules-version VERSION --precision-rules-version VERSION @@ -47,7 +48,9 @@ duplicate JSON keys, unknown fields, and any mismatch with the separately review External AI tools must call `tool-contract` first. The returned JSON describes the allowed Basic edit operations, forbidden capabilities, stable exit codes, and the required two-step edit flow. An AI tool must inspect the preview -`diff`, retain its `previewHash`, and send that exact hash with the same operations when applying the reviewed change. +`diff`, retain its `previewHash` and `expectedEditSequence`, and send both back with the same operations when applying +the reviewed change. The sequence is what makes the review gate hold across a concurrent owner edit: without it the +server would re-read the document and apply a diff nobody reviewed against its current state. The CLI rejects arbitrary code, external-data access, direct orders, unapproved delegation scopes, and apply requests that omit the reviewed preview hash. diff --git a/apps/idea2strategy-cli/src/main/java/com/idea2strategy/cli/Idea2StrategyCli.java b/apps/idea2strategy-cli/src/main/java/com/idea2strategy/cli/Idea2StrategyCli.java index 05b6e903..d5c41cd1 100644 --- a/apps/idea2strategy-cli/src/main/java/com/idea2strategy/cli/Idea2StrategyCli.java +++ b/apps/idea2strategy-cli/src/main/java/com/idea2strategy/cli/Idea2StrategyCli.java @@ -193,11 +193,19 @@ private static JsonNode strategyCopy(Arguments args, ApiClient api, String token private static JsonNode basicEdit(Arguments args, ApiClient api, String token, boolean apply) { args.rejectUnknown("--strategy-id", "--authorization-id", "--credential-id", - "--operations-file", "--preview-hash"); + "--operations-file", "--preview-hash", "--expected-edit-sequence"); String previewHash = args.optional("--preview-hash"); + String expectedEditSequence = args.optional("--expected-edit-sequence"); if (apply && (previewHash == null || previewHash.isBlank())) { throw Arguments.usage("Apply requires --preview-hash from a reviewed preview"); } + // The preview reports the sequence it read. Returning it on apply is what makes the review + // gate hold: without it the server would re-read, and an owner edit landing between the two + // calls would be overwritten by a diff nobody reviewed against it. + if (apply && (expectedEditSequence == null || expectedEditSequence.isBlank())) { + throw Arguments.usage( + "Apply requires --expected-edit-sequence from the same reviewed preview"); + } ArrayNode operations = readOperations(args.required("--operations-file")); for (JsonNode operation : operations) { String action = operation.path("action").asText(); @@ -210,6 +218,13 @@ private static JsonNode basicEdit(Arguments args, ApiClient api, String token, b .put("credentialId", args.required("--credential-id")); body.set("operations", operations); putOptional(body, "previewHash", previewHash); + if (expectedEditSequence != null && !expectedEditSequence.isBlank()) { + try { + body.put("expectedEditSequence", Long.parseLong(expectedEditSequence.trim())); + } catch (NumberFormatException exception) { + throw Arguments.usage("--expected-edit-sequence must be the integer a preview returned"); + } + } String suffix = apply ? "apply" : "preview"; return api.post("/api/v1/strategies/" + segment(args.required("--strategy-id")) + "/basic-edits/" + suffix, body, token); diff --git a/apps/idea2strategy-cli/src/main/resources/idea2strategy-ai-tool-contract.json b/apps/idea2strategy-cli/src/main/resources/idea2strategy-ai-tool-contract.json index 0c529e0a..e7a5b6ed 100644 --- a/apps/idea2strategy-cli/src/main/resources/idea2strategy-ai-tool-contract.json +++ b/apps/idea2strategy-cli/src/main/resources/idea2strategy-ai-tool-contract.json @@ -13,7 +13,7 @@ "--credential-id", "", "--operations-file", "" ], - "requiredOutputFields": ["data.diff", "data.previewHash"] + "requiredOutputFields": ["data.diff", "data.previewHash", "data.expectedEditSequence"] }, { "name": "apply-reviewed-basic-edit", @@ -23,13 +23,15 @@ "--authorization-id", "", "--credential-id", "", "--operations-file", "", - "--preview-hash", "" + "--preview-hash", "", + "--expected-edit-sequence", "" ] } ], "reviewGate": { "previewDiffMustBeInspected": true, "applyRequiresPreviewHash": true, + "applyRequiresPreviewEditSequence": true, "applyMustReuseReviewedOperations": true }, "allowedEditOperations": [ diff --git a/apps/idea2strategy-cli/src/test/java/com/idea2strategy/cli/ExternalAiToolE2eTest.java b/apps/idea2strategy-cli/src/test/java/com/idea2strategy/cli/ExternalAiToolE2eTest.java index fc339f25..0185ea36 100644 --- a/apps/idea2strategy-cli/src/test/java/com/idea2strategy/cli/ExternalAiToolE2eTest.java +++ b/apps/idea2strategy-cli/src/test/java/com/idea2strategy/cli/ExternalAiToolE2eTest.java @@ -73,13 +73,19 @@ void externalToolReviewsPreviewDiffBeforeApplyingExactHash() throws Exception { assertThat(previewData.path("diff").isEmpty()).isFalse(); String reviewedHash = previewData.path("previewHash").asText(); assertThat(reviewedHash).isEqualTo("sha256:reviewed-diff"); + String reviewedSequence = previewData.path("expectedEditSequence").asText(); + assertThat(reviewedSequence).isEqualTo("7"); - ProcessResult apply = invokeEdit("apply", operations, "--preview-hash", reviewedHash); + ProcessResult apply = invokeEdit( + "apply", operations, + "--preview-hash", reviewedHash, + "--expected-edit-sequence", reviewedSequence); assertThat(apply.exitCode()).isZero(); assertThat(JSON.readTree(apply.stdout()).path("data").path("applied").asBoolean()).isTrue(); assertThat(requestBodies).hasSize(2); assertThat(requestBodies.get(1).path("previewHash").asText()).isEqualTo(reviewedHash); + assertThat(requestBodies.get(1).path("expectedEditSequence").asLong()).isEqualTo(7L); } @Test @@ -112,7 +118,9 @@ void mapsScopeOverreachAndTamperedPreviewToStableSafetyExitCodes() throws Except forcedStatus.set(null); ProcessResult tampered = invokeEdit( - "apply", operations("ADD_BLOCK"), "--preview-hash", "sha256:tampered"); + "apply", operations("ADD_BLOCK"), + "--preview-hash", "sha256:tampered", + "--expected-edit-sequence", "7"); assertThat(tampered.exitCode()).isEqualTo(5); assertThat(JSON.readTree(tampered.stderr()).path("error").path("code").asText()) .isEqualTo("PREVIEW_MISMATCH"); @@ -171,10 +179,12 @@ private void handleRequest(HttpExchange exchange) throws IOException { if (exchange.getRequestURI().getPath().endsWith("/preview")) { reviewedOperations.set(body.path("operations").deepCopy()); respond(exchange, 200, - "{\"previewHash\":\"sha256:reviewed-diff\",\"diff\":[{\"op\":\"replace\",\"path\":\"/blocks/0/value\"}]}"); + "{\"previewHash\":\"sha256:reviewed-diff\",\"expectedEditSequence\":7," + + "\"diff\":[{\"op\":\"replace\",\"path\":\"/blocks/0/value\"}]}"); return; } if (!"sha256:reviewed-diff".equals(body.path("previewHash").asText()) + || body.path("expectedEditSequence").asLong(-1) != 7L || !body.path("operations").equals(reviewedOperations.get())) { respond(exchange, 409, "{\"code\":\"PREVIEW_MISMATCH\",\"message\":\"preview hash was not reviewed\"}"); diff --git a/apps/idea2strategy-cli/src/test/java/com/idea2strategy/cli/Idea2StrategyCliTest.java b/apps/idea2strategy-cli/src/test/java/com/idea2strategy/cli/Idea2StrategyCliTest.java index b2561982..de5a9609 100644 --- a/apps/idea2strategy-cli/src/test/java/com/idea2strategy/cli/Idea2StrategyCliTest.java +++ b/apps/idea2strategy-cli/src/test/java/com/idea2strategy/cli/Idea2StrategyCliTest.java @@ -107,11 +107,29 @@ void basicEditApplySendsOnlyAllowedOperationsWithPreviewHash() throws Exception Result result = run("", "--base-url", baseUrl, "--config-dir", tempDir.toString(), "strategy", "edit", "apply", "--strategy-id", "strategy-1", "--authorization-id", "auth-1", "--credential-id", "credential-1", - "--operations-file", operations.toString(), "--preview-hash", "sha256:reviewed"); + "--operations-file", operations.toString(), "--preview-hash", "sha256:reviewed", + "--expected-edit-sequence", "12"); assertThat(result.exitCode()).isZero(); assertThat(requestPath.get()).isEqualTo("/api/v1/strategies/strategy-1/basic-edits/apply"); - assertThat(requestBody.get()).contains("sha256:reviewed", "SET_VALUE", "auth-1", "credential-1"); + assertThat(requestBody.get()) + .contains("sha256:reviewed", "SET_VALUE", "auth-1", "credential-1") + .contains("\"expectedEditSequence\":12"); + } + + @Test + void basicEditApplyWithoutReviewedEditSequenceIsUsageErrorBeforeNetworkCall() throws Exception { + Files.writeString(tempDir.resolve("credentials.json"), "{\"sessionToken\":\"stored-token\"}"); + Path operations = tempDir.resolve("operations.json"); + Files.writeString(operations, "[{\"action\":\"SET_VALUE\",\"arguments\":{\"value\":14}}]"); + + Result result = run("", "--base-url", baseUrl, "--config-dir", tempDir.toString(), + "strategy", "edit", "apply", "--strategy-id", "strategy-1", + "--authorization-id", "auth-1", "--credential-id", "credential-1", + "--operations-file", operations.toString(), "--preview-hash", "sha256:reviewed"); + + assertThat(result.exitCode()).isEqualTo(2); + assertThat(requestPath.get()).isNull(); } @Test diff --git a/modules/backend-application/src/main/java/com/idea2strategy/backend/application/strategy/DelegatedBasicEditPreviewMismatchException.java b/modules/backend-application/src/main/java/com/idea2strategy/backend/application/strategy/DelegatedBasicEditPreviewMismatchException.java new file mode 100644 index 00000000..875f8cb1 --- /dev/null +++ b/modules/backend-application/src/main/java/com/idea2strategy/backend/application/strategy/DelegatedBasicEditPreviewMismatchException.java @@ -0,0 +1,8 @@ +package com.idea2strategy.backend.application.strategy; + +/** The reviewed preview hash does not describe the edit being applied. */ +public final class DelegatedBasicEditPreviewMismatchException extends DelegatedBasicEditRejectedException { + public DelegatedBasicEditPreviewMismatchException(String message) { + super(message); + } +} diff --git a/modules/backend-application/src/main/java/com/idea2strategy/backend/application/strategy/DelegatedBasicEditRejectedException.java b/modules/backend-application/src/main/java/com/idea2strategy/backend/application/strategy/DelegatedBasicEditRejectedException.java index 694b466c..171bd191 100644 --- a/modules/backend-application/src/main/java/com/idea2strategy/backend/application/strategy/DelegatedBasicEditRejectedException.java +++ b/modules/backend-application/src/main/java/com/idea2strategy/backend/application/strategy/DelegatedBasicEditRejectedException.java @@ -1,6 +1,14 @@ package com.idea2strategy.backend.application.strategy; -public final class DelegatedBasicEditRejectedException extends RuntimeException { +/** + * A delegated Basic edit was refused. + * + *

Two refusals mean something different to the caller and carry their own subtype, because the + * external tool contract maps them to distinct stable exit codes: scope denial says the delegation + * never permitted this, while a preview mismatch says the reviewed diff is not the one being + * applied. Every other refusal is an ordinary invalid operation and uses this type directly. + */ +public class DelegatedBasicEditRejectedException extends RuntimeException { public DelegatedBasicEditRejectedException(String message) { super(message); } diff --git a/modules/backend-application/src/main/java/com/idea2strategy/backend/application/strategy/DelegatedBasicStrategyEditService.java b/modules/backend-application/src/main/java/com/idea2strategy/backend/application/strategy/DelegatedBasicStrategyEditService.java index 0f2607f3..e1c684d8 100644 --- a/modules/backend-application/src/main/java/com/idea2strategy/backend/application/strategy/DelegatedBasicStrategyEditService.java +++ b/modules/backend-application/src/main/java/com/idea2strategy/backend/application/strategy/DelegatedBasicStrategyEditService.java @@ -78,7 +78,7 @@ public StrategyDocument apply( editor, strategyId, expectedEditSequence, catalog, operations, DelegatedStrategyScope.STRATEGY_EDIT); if (!preview.previewHash().equals(reviewedPreviewHash)) { - throw new DelegatedBasicEditRejectedException( + throw new DelegatedBasicEditPreviewMismatchException( "Reviewed preview does not match the requested edit"); } if (!preview.valid()) { @@ -97,7 +97,7 @@ public StrategyDocument apply( return switch (commandPort.replace(replacement, expectedEditSequence, editor, clock.instant())) { case UPDATED -> replacement; case STALE_EDIT_SEQUENCE -> throw new StrategyDraftConflictException(); - case UNAUTHORIZED -> throw new DelegatedBasicEditRejectedException( + case UNAUTHORIZED -> throw new DelegatedStrategyScopeDeniedException( "Delegated authorization is not active for strategy editing"); }; } diff --git a/modules/backend-application/src/main/java/com/idea2strategy/backend/application/strategy/DelegatedStrategyScopeDeniedException.java b/modules/backend-application/src/main/java/com/idea2strategy/backend/application/strategy/DelegatedStrategyScopeDeniedException.java new file mode 100644 index 00000000..dc11f8ef --- /dev/null +++ b/modules/backend-application/src/main/java/com/idea2strategy/backend/application/strategy/DelegatedStrategyScopeDeniedException.java @@ -0,0 +1,8 @@ +package com.idea2strategy.backend.application.strategy; + +/** The delegation does not carry an active scope for the requested edit. */ +public final class DelegatedStrategyScopeDeniedException extends DelegatedBasicEditRejectedException { + public DelegatedStrategyScopeDeniedException(String message) { + super(message); + } +} diff --git a/modules/backend-persistence/src/main/java/com/idea2strategy/backend/persistence/strategy/DelegatedBasicStrategyEditJooqAdapter.java b/modules/backend-persistence/src/main/java/com/idea2strategy/backend/persistence/strategy/DelegatedBasicStrategyEditJooqAdapter.java index 0a9f7803..b5f07c8d 100644 --- a/modules/backend-persistence/src/main/java/com/idea2strategy/backend/persistence/strategy/DelegatedBasicStrategyEditJooqAdapter.java +++ b/modules/backend-persistence/src/main/java/com/idea2strategy/backend/persistence/strategy/DelegatedBasicStrategyEditJooqAdapter.java @@ -1,11 +1,11 @@ package com.idea2strategy.backend.persistence.strategy; import com.idea2strategy.backend.application.strategy.DelegatedBasicEditCommandPort; -import com.idea2strategy.backend.application.strategy.DelegatedBasicEditRejectedException; import com.idea2strategy.backend.application.strategy.DelegatedBasicEditReplaceResult; import com.idea2strategy.backend.application.strategy.DelegatedStrategyAuthorizationPort; import com.idea2strategy.backend.application.strategy.DelegatedStrategyEditor; import com.idea2strategy.backend.application.strategy.DelegatedStrategyScope; +import com.idea2strategy.backend.application.strategy.DelegatedStrategyScopeDeniedException; import com.idea2strategy.backend.application.strategy.StrategyDocumentJson; import com.idea2strategy.backend.domain.strategy.StrategyDocument; import java.nio.charset.StandardCharsets; @@ -32,7 +32,7 @@ public void requireAuthorized( DelegatedStrategyScope scope, Instant at) { if (!isAuthorized(editor, strategyId, scope, at)) { - throw new DelegatedBasicEditRejectedException( + throw new DelegatedStrategyScopeDeniedException( "Delegated authorization is not active for " + scope.name()); } } From 79febe9b598a118b5ff583d8617e61e3f290becf Mon Sep 17 00:00:00 2001 From: Ryan Min Date: Sun, 9 Aug 2026 23:15:37 +0900 Subject: [PATCH 2/7] ci: publish the CLI as an installable archive The CLI has never been built or published by any workflow, so using it meant cloning the superproject with submodules and running Gradle. That is not an install, and it is why the tool has no users outside this repository. The Gradle application plugin already produces the archive, so the release job adds no new build. Upload uses the runner's own gh instead of a release action: one fewer third-party action to pin and to trust with a write-scoped token. The tool contract is parsed in the job because it is the first thing an external AI reads, and a release whose contract does not parse is worse than no release. CI now builds the archive on every change. A distribution can break without any test failing, and the release path should not rot between the tags that publish it. operator bootstrap stays in this CLI. Splitting it would drop the Postgres driver and the backend classes from the archive, but docs/operator-auth-enablement-handoff.md documents `idea2strategy operator bootstrap` as a live one-shot procedure and INT05 is still open on it. Four megabytes is not worth breaking a runbook in use. Refs #479 Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 6 +++ .github/workflows/cli-release.yml | 70 +++++++++++++++++++++++++++++++ apps/idea2strategy-cli/README.md | 11 +++++ 3 files changed, 87 insertions(+) create mode 100644 .github/workflows/cli-release.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index be6c3c40..3128032d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,6 +35,12 @@ jobs: chmod +x gradlew ./gradlew test --no-daemon --build-cache --parallel + # The released CLI is an ordinary Gradle distribution, so it can break without any + # test failing. Building it on every change keeps the release path from rotting + # between the tags that actually publish it. + - name: Build the distributable CLI archive + run: ./gradlew :apps:idea2strategy-cli:distZip --no-daemon --build-cache + container-contracts: runs-on: ubuntu-latest timeout-minutes: 5 diff --git a/.github/workflows/cli-release.yml b/.github/workflows/cli-release.yml new file mode 100644 index 00000000..4f4e3f7e --- /dev/null +++ b/.github/workflows/cli-release.yml @@ -0,0 +1,70 @@ +name: CLI release + +# Publishes the Idea2Strategy CLI as an installable archive. +# +# The archive is the one the Gradle application plugin already produces, so nothing +# about the build differs from a local `installDist`. Uploading uses the runner's own +# `gh` rather than a release action: one fewer third-party action to pin and to trust +# with a write-scoped token. + +on: + push: + tags: ['cli-v*'] + workflow_dispatch: + inputs: + tag: + description: Existing tag to publish (cli-vX.Y.Z) + required: true + +permissions: + contents: write + +concurrency: + group: cli-release-${{ github.ref }} + cancel-in-progress: false + +jobs: + publish: + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ github.event.inputs.tag || github.ref }} + - name: Verify immutable GitHub Actions references + run: | + chmod +x scripts/test-github-actions-pins.sh + ./scripts/test-github-actions-pins.sh + - uses: actions/setup-java@b6effb05e454b25005698d916606bdc6ffcbf961 # v5.7.0 + with: + distribution: temurin + java-version: 21 + - uses: gradle/actions/setup-gradle@9c971963bec38e04b3d30dcc455b5382be2fdbfb # v6.3.0 + with: + cache-read-only: true + + # The tool contract is what an external AI reads before it does anything. A release + # whose contract does not parse is worse than no release, so it is checked here and + # not only in the unit tests that build it. + - name: Build and verify the distribution + run: | + chmod +x gradlew + ./gradlew :apps:idea2strategy-cli:test :apps:idea2strategy-cli:distZip \ + --no-daemon --build-cache + jq -e '.contractCommand and .workflow and .exitCodes' \ + apps/idea2strategy-cli/src/main/resources/idea2strategy-ai-tool-contract.json > /dev/null + + - name: Publish the archive and its checksum + env: + GH_TOKEN: ${{ github.token }} + TAG: ${{ github.event.inputs.tag || github.ref_name }} + run: | + set -euo pipefail + archive="$(ls apps/idea2strategy-cli/build/distributions/*.zip)" + sha256sum "$archive" > "${archive}.sha256" + gh release view "$TAG" --repo "$GITHUB_REPOSITORY" > /dev/null 2>&1 \ + || gh release create "$TAG" --repo "$GITHUB_REPOSITORY" \ + --title "Idea2Strategy CLI $TAG" \ + --notes "Requires Java 21. Verify with the published .sha256, unzip, and run \`bin/idea2strategy tool-contract\`." + gh release upload "$TAG" "$archive" "${archive}.sha256" \ + --repo "$GITHUB_REPOSITORY" --clobber diff --git a/apps/idea2strategy-cli/README.md b/apps/idea2strategy-cli/README.md index 3b064498..23db0ec8 100644 --- a/apps/idea2strategy-cli/README.md +++ b/apps/idea2strategy-cli/README.md @@ -3,6 +3,17 @@ The CLI provides a JSON-only automation boundary for Basic strategy workflows. It does not expose arbitrary code, external-data fetching, or direct-order commands. +## Install + +Download `idea2strategy-.zip` from a `cli-v*` release, verify it against the published +`.sha256`, and unzip it. The launcher is `bin/idea2strategy` (`bin/idea2strategy.bat` on Windows); +put its directory on `PATH`. **Java 21 must already be installed** — the archive carries no runtime. + +An external AI tool should run `idea2strategy tool-contract` first and follow the JSON it returns +rather than this file: the contract is what the released binary actually enforces. + +## Build from source + Build a local distribution: ```powershell From 674695b164672ca9716f30fb29443624edeadea7 Mon Sep 17 00:00:00 2001 From: Ryan Min Date: Sun, 9 Aug 2026 23:20:41 +0900 Subject: [PATCH 3/7] feat: publish the delegation disclosure a customer reads before granting edit access MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit identity.delegated_authorizations.disclosure_policy_document_id is NOT NULL with a foreign key to identity.policy_documents, that table had no rows, and no migration anywhere wrote one. No delegation could be granted, on any environment, ever. The missing piece was never code: it was the text a customer reads before handing an external tool the ability to change their strategy. The text states what the delegation permits, what it can never do (orders, funds, bot runs, release, other strategies, arbitrary code, external data), that every change arrives as a reviewed preview first, and that it expires and can be revoked. content_hash is computed from the stored text instead of pasted beside it, so the two cannot drift apart here or in review. The insert is idempotent on (policy_code, version, language_code), which is the table's own unique key. Approved by product authority kcrmin on 2026-08-09: "이 문안으로 갈께". Refs #479 Co-Authored-By: Claude Opus 5 --- ...d_publish_delegation_disclosure_policy.sql | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 db-migration/src/main/resources/db/migration/V20260809140000__backend_publish_delegation_disclosure_policy.sql diff --git a/db-migration/src/main/resources/db/migration/V20260809140000__backend_publish_delegation_disclosure_policy.sql b/db-migration/src/main/resources/db/migration/V20260809140000__backend_publish_delegation_disclosure_policy.sql new file mode 100644 index 00000000..2f702a46 --- /dev/null +++ b/db-migration/src/main/resources/db/migration/V20260809140000__backend_publish_delegation_disclosure_policy.sql @@ -0,0 +1,62 @@ +-- Publishes the disclosure a customer reads before delegating Basic strategy editing +-- to an external tool. +-- +-- identity.delegated_authorizations.disclosure_policy_document_id is NOT NULL with a +-- foreign key here, and this table had no rows and no migration that wrote one, so no +-- delegation could ever be granted. The text is the product decision that unblocks it: +-- what the delegation permits, what it can never do, that every change is reviewed as a +-- preview first, and that it expires and can be revoked. +-- +-- The hash is computed from the stored text rather than pasted in, so the two cannot +-- drift apart in this migration or in review. + +INSERT INTO identity.policy_documents ( + policy_code, + version, + language_code, + title, + content_format, + content_text, + content_hash, + is_required, + published_at +) +SELECT + 'delegation.strategy-edit.disclosure', + 'v1', + 'ko', + '외부 도구에 전략 편집을 위임합니다', + 'MARKDOWN', + body, + encode(sha256(convert_to(body, 'UTF8')), 'hex'), + true, + TIMESTAMPTZ '2026-08-09 00:00:00+00' +FROM ( + SELECT $doc$# 외부 도구에 전략 편집을 위임합니다 + +이 위임을 만들면 선택한 외부 도구가 회원님을 대신해 다음을 할 수 있습니다. + +- 지정한 전략의 **Basic 블록을 추가·삭제·연결하고 값을 바꾸는 것** +- (검증 범위를 함께 준 경우) 그 전략을 검증하는 것 + +이 위임으로 **할 수 없는 것**은 다음과 같습니다. + +- 주문·체결, 자금 이동, 봇 실행이나 중단 +- 전략 출시 +- 지정하지 않은 다른 전략의 열람이나 편집 +- 임의 코드 실행, 외부 데이터 가져오기 + +도구가 전략을 바꾸려면 **먼저 변경 내용을 미리보기로 제시**해야 하고, 그 미리보기와 정확히 같은 내용만 반영됩니다. 다른 내용으로 바꿔치기할 수 없습니다. + +이 위임에는 **만료 시각**이 있으며, 그 전에도 언제든 회수할 수 있습니다. 회수하면 즉시 효력을 잃습니다. + +위임 생성·사용·회수 기록은 회원님의 계정 활동에 남습니다. +$doc$ AS body +) AS published +WHERE NOT EXISTS ( + SELECT 1 + FROM identity.policy_documents + WHERE policy_code = 'delegation.strategy-edit.disclosure' + AND version = 'v1' + AND language_code = 'ko' +); From 73dc95f250119f517a3a54a803d9aa0f45fbdeed Mon Sep 17 00:00:00 2001 From: Ryan Min Date: Sun, 9 Aug 2026 23:24:11 +0900 Subject: [PATCH 4/7] fix: report the change list, not the whole document, as the reviewable diff The tool contract tells an external AI to inspect data.diff before applying, and the preview response was putting the entire proposed semantic document there. A tool following the contract literally would skim a full document and call that a review, which is the one thing the preview gate exists to prevent. The reviewable thing is the change list the service already produces (ADD_BLOCK group/block CODE). The proposed document stays in the response under its own name, so nothing is lost for a caller that genuinely wants it. Refs #479 Co-Authored-By: Claude Opus 5 --- .../api/strategy/DelegatedBasicEditController.java | 12 +++++++++--- .../DelegatedBasicEditRouteRegistrationTest.java | 13 +++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/apps/backend-api/src/main/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditController.java b/apps/backend-api/src/main/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditController.java index dbcdeb6f..d5d58c80 100644 --- a/apps/backend-api/src/main/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditController.java +++ b/apps/backend-api/src/main/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditController.java @@ -65,8 +65,8 @@ public PreviewResponse preview( return new PreviewResponse( preview.beforeHash(), preview.previewHash(), - readJson(preview.proposedSemanticDocument()), preview.changes(), + readJson(preview.proposedSemanticDocument()), preview.valid(), expectedEditSequence); } @@ -149,11 +149,17 @@ public String toString() { public record OperationRequest(String action, Map arguments) {} + /** + * {@code diff} is the reviewable change list, not the resulting document. The tool contract + * tells an external AI to inspect {@code data.diff} before applying, so it has to be the thing + * a reviewer can actually read: labelling the whole proposed document a diff would invite the + * tool to skim it and call that a review. The full result stays available beside it. + */ public record PreviewResponse( String beforeHash, String previewHash, - Map diff, - List changes, + List diff, + Map proposedSemanticDocument, boolean valid, long expectedEditSequence) {} diff --git a/apps/backend-api/src/test/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditRouteRegistrationTest.java b/apps/backend-api/src/test/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditRouteRegistrationTest.java index 131f0328..deaa16bc 100644 --- a/apps/backend-api/src/test/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditRouteRegistrationTest.java +++ b/apps/backend-api/src/test/java/com/idea2strategy/backend/api/strategy/DelegatedBasicEditRouteRegistrationTest.java @@ -52,6 +52,19 @@ void keepsTheControllerDisabledWhenTheStrategyDraftModuleIsDisabled() { }); } + /** + * The tool contract names {@code data.diff} as the field an external AI must inspect before + * applying. If that field ever becomes the resulting document instead of the change list, the + * review gate still passes mechanically while nothing reviewable was reviewed. + */ + @Test + void previewReportsTheChangeListAsTheReviewableDiff() throws Exception { + var diff = DelegatedBasicEditController.PreviewResponse.class.getRecordComponents()[2]; + + assertThat(diff.getName()).isEqualTo("diff"); + assertThat(diff.getType()).isEqualTo(List.class); + } + @Test void refusalAdviceStaysScopedToTheDelegatedRoute() { var advice = DelegatedBasicEditExceptionHandler.class From 4db3e8a68cd85280fd2dbdaa25c3c0be5c8701d2 Mon Sep 17 00:00:00 2001 From: Ryan Min Date: Sun, 9 Aug 2026 23:29:47 +0900 Subject: [PATCH 5/7] fix: keep the delegation disclosure out of the required consent set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught this and it was a real design error, not a test detail. is_required drives RequiredPolicySet, which every account must satisfy to finish authenticating, so publishing the disclosure as required would have asked every customer to consent to a delegation notice before logging in — including the ones who never delegate anything. AccountOperationsFullJourneyIntegrationTest failed for exactly that reason. The disclosure belongs at the moment a delegation is granted, which is where the delegation flow will read it. Registering the file in the two pinned migration lists is the mechanical part of the same change. Refs #479 Co-Authored-By: Claude Opus 5 --- ...9140000__backend_publish_delegation_disclosure_policy.sql | 5 ++++- .../migration/CanonicalMigrationBundleAssemblerTest.java | 1 + .../idea2strategy/backend/migration/MigrationPolicyTest.java | 3 ++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/db-migration/src/main/resources/db/migration/V20260809140000__backend_publish_delegation_disclosure_policy.sql b/db-migration/src/main/resources/db/migration/V20260809140000__backend_publish_delegation_disclosure_policy.sql index 2f702a46..a8ec94a2 100644 --- a/db-migration/src/main/resources/db/migration/V20260809140000__backend_publish_delegation_disclosure_policy.sql +++ b/db-migration/src/main/resources/db/migration/V20260809140000__backend_publish_delegation_disclosure_policy.sql @@ -29,7 +29,10 @@ SELECT 'MARKDOWN', body, encode(sha256(convert_to(body, 'UTF8')), 'hex'), - true, + -- Not a required consent. is_required drives RequiredPolicySet, which every account must + -- satisfy to complete authentication; a customer who never delegates must never be asked + -- for this. It is disclosed at the moment a delegation is granted. + false, TIMESTAMPTZ '2026-08-09 00:00:00+00' FROM ( SELECT $doc$# 외부 도구에 전략 편집을 위임합니다 diff --git a/db-migration/src/test/java/com/idea2strategy/backend/migration/CanonicalMigrationBundleAssemblerTest.java b/db-migration/src/test/java/com/idea2strategy/backend/migration/CanonicalMigrationBundleAssemblerTest.java index 7de2f95d..ca069564 100644 --- a/db-migration/src/test/java/com/idea2strategy/backend/migration/CanonicalMigrationBundleAssemblerTest.java +++ b/db-migration/src/test/java/com/idea2strategy/backend/migration/CanonicalMigrationBundleAssemblerTest.java @@ -83,6 +83,7 @@ void assemblesOnlyOwnedCanonicalContributionsInGlobalVersionOrder() throws Excep "V20260808000000__backend_publish_live_strategy_timeframes.sql", "V20260808120000__backend_publish_production_backtest_resolutions.sql", "V20260809120000__backend_account_email_notification_preference.sql", + "V20260809140000__backend_publish_delegation_disclosure_policy.sql", DatabaseAccessPolicy.RUNTIME_GRANTS_FILE), result.orderedFileNames()); assertTrue(Files.readString(result.directory().resolve(DatabaseAccessPolicy.RUNTIME_GRANTS_FILE)) diff --git a/db-migration/src/test/java/com/idea2strategy/backend/migration/MigrationPolicyTest.java b/db-migration/src/test/java/com/idea2strategy/backend/migration/MigrationPolicyTest.java index 70b76ffb..7f53d62c 100644 --- a/db-migration/src/test/java/com/idea2strategy/backend/migration/MigrationPolicyTest.java +++ b/db-migration/src/test/java/com/idea2strategy/backend/migration/MigrationPolicyTest.java @@ -103,7 +103,8 @@ void verifiesTheCheckedInMigrationDirectoryAndBaselineChecksum() throws Exceptio "V20260807130000__backend_publish_full_basic_element_catalog.sql", "V20260808000000__backend_publish_live_strategy_timeframes.sql", "V20260808120000__backend_publish_production_backtest_resolutions.sql", - "V20260809120000__backend_account_email_notification_preference.sql"), + "V20260809120000__backend_account_email_notification_preference.sql", + "V20260809140000__backend_publish_delegation_disclosure_policy.sql"), plan.orderedFileNames()); } From 5f2cb52aea44bc49914498a25c43b363a117ccdc Mon Sep 17 00:00:00 2001 From: Ryan Min Date: Mon, 10 Aug 2026 00:05:46 +0900 Subject: [PATCH 6/7] feat: grant and revoke the delegation an external tool edits under The write path for A15 did not exist. DelegatedAuthorizationCommandPort had only a test fake and DelegatedCredentialMaterialPort had no implementation at all, so the only delegation code in the repository read authorizations and swept expired ones. Nothing could create the row those readers look for. The event row is the idempotency receipt: (authorization_id, idempotency_key) is unique, and the replay is detected before the decision runs. The other order mints a credential for a caller who will never receive it. Three inputs are read rather than accepted from the request. The auth epoch is what makes a delegation die when the account re-authenticates, so a client that could name it could outlive a password change. The disclosure document is the text the customer was actually shown, and a request that chose it could claim consent to something else. The target owner and access epoch are pinned from the strategy row, so a later owner change or access-epoch bump stops honouring the delegation instead of carrying it across the change. A delegation with no target is refused at both ends. The authorization check requires a pinned target, so granting one would return success and then deny every edit, surfacing later as an unexplained scope denial. Expiry defaults to 24 hours (delegation.default-lifetime) because the disclosure promises the customer an expiry; expiry_mode is AT_TIME whenever an instant is carried, which is the only mode the authorization check enforces against the clock. The integration test grants with the production adapter and then edits with the production check. That join had no coverage: the check reads ten columns across five tables, and any one written differently makes every edit deny with nothing to say which. Asserting the grant's own rows would not have caught it. Refs #479 Co-Authored-By: Claude Opus 5 --- .../delegation/DelegationConfiguration.java | 38 +++ .../api/delegation/DelegationController.java | 207 +++++++++++++ .../delegation/HmacDelegatedCredentials.java | 58 ++++ apps/idea2strategy-cli/README.md | 8 +- .../idea2strategy/cli/Idea2StrategyCli.java | 16 +- .../cli/Idea2StrategyCliTest.java | 17 +- .../DelegationGrantContextPort.java | 17 ++ .../DelegatedAuthorizationJooqAdapter.java | 273 ++++++++++++++++++ ...egatedStrategyTargetRejectedException.java | 15 + ...zationGrantPersistenceIntegrationTest.java | 239 +++++++++++++++ 10 files changed, 885 insertions(+), 3 deletions(-) create mode 100644 apps/backend-api/src/main/java/com/idea2strategy/backend/api/delegation/DelegationConfiguration.java create mode 100644 apps/backend-api/src/main/java/com/idea2strategy/backend/api/delegation/DelegationController.java create mode 100644 apps/backend-api/src/main/java/com/idea2strategy/backend/api/delegation/HmacDelegatedCredentials.java create mode 100644 modules/backend-application/src/main/java/com/idea2strategy/backend/application/delegation/DelegationGrantContextPort.java create mode 100644 modules/backend-persistence/src/main/java/com/idea2strategy/backend/persistence/delegation/DelegatedAuthorizationJooqAdapter.java create mode 100644 modules/backend-persistence/src/main/java/com/idea2strategy/backend/persistence/delegation/DelegatedStrategyTargetRejectedException.java create mode 100644 modules/backend-persistence/src/test/java/com/idea2strategy/backend/persistence/delegation/DelegatedAuthorizationGrantPersistenceIntegrationTest.java diff --git a/apps/backend-api/src/main/java/com/idea2strategy/backend/api/delegation/DelegationConfiguration.java b/apps/backend-api/src/main/java/com/idea2strategy/backend/api/delegation/DelegationConfiguration.java new file mode 100644 index 00000000..d0aff2e2 --- /dev/null +++ b/apps/backend-api/src/main/java/com/idea2strategy/backend/api/delegation/DelegationConfiguration.java @@ -0,0 +1,38 @@ +package com.idea2strategy.backend.api.delegation; + +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationService; +import com.idea2strategy.backend.persistence.delegation.DelegatedAuthorizationJooqAdapter; +import java.time.Clock; +import java.util.Base64; +import java.util.UUID; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; + +@Configuration(proxyBeanMethods = false) +@ConditionalOnProperty(name = {"spring.datasource.url", "identity.crypto.customer-jwt-signing-key"}) +@Import(DelegatedAuthorizationJooqAdapter.class) +public class DelegationConfiguration { + @Bean + DelegatedAuthorizationService delegatedAuthorizationService( + DelegatedAuthorizationJooqAdapter adapter, + HmacDelegatedCredentials credentials, + Clock identityClock) { + return new DelegatedAuthorizationService(adapter, credentials, identityClock, UUID::randomUUID); + } + + /** + * Falls back to the refresh-token key so a deployment that has not provisioned a dedicated + * delegation key still stores digests rather than raw credentials. Both are 256-bit identity + * secrets from the same store; a separate key is preferable and is what the property is for. + */ + @Bean + HmacDelegatedCredentials hmacDelegatedCredentials( + @Value("${identity.crypto.delegated-credential-hmac-key:${identity.crypto.refresh-token-hmac-key}}") + String key, + @Value("${identity.crypto.delegated-credential-key-version:1}") short keyVersion) { + return new HmacDelegatedCredentials(Base64.getDecoder().decode(key), keyVersion); + } +} diff --git a/apps/backend-api/src/main/java/com/idea2strategy/backend/api/delegation/DelegationController.java b/apps/backend-api/src/main/java/com/idea2strategy/backend/api/delegation/DelegationController.java new file mode 100644 index 00000000..d1ec3702 --- /dev/null +++ b/apps/backend-api/src/main/java/com/idea2strategy/backend/api/delegation/DelegationController.java @@ -0,0 +1,207 @@ +package com.idea2strategy.backend.api.delegation; + +import com.idea2strategy.backend.application.common.CurrentPrincipal; +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationCommand; +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationCommandType; +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationResult; +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationScope; +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationService; +import com.idea2strategy.backend.application.delegation.DelegationGrantContextPort; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.time.Clock; +import java.time.Duration; +import java.time.Instant; +import java.util.HexFormat; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import java.util.UUID; +import java.util.stream.Collectors; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.DeleteMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +/** + * Grants and revokes the delegation an external tool edits under. + * + *

The raw credential is returned once, here, and never again: only its digest is stored. A + * caller that loses it revokes and grants a new one. + */ +@RestController +@RequestMapping("/api/v1/delegations") +@ConditionalOnProperty(name = {"spring.datasource.url", "identity.crypto.customer-jwt-signing-key"}) +public class DelegationController { + static final String DISCLOSURE_POLICY_CODE = "delegation.strategy-edit.disclosure"; + + private final DelegatedAuthorizationService service; + private final DelegationGrantContextPort grantContext; + private final CurrentPrincipal principal; + private final Clock clock; + private final Duration defaultLifetime; + + public DelegationController( + DelegatedAuthorizationService service, + DelegationGrantContextPort grantContext, + CurrentPrincipal principal, + Clock clock, + @Value("${delegation.default-lifetime:PT24H}") Duration defaultLifetime) { + this.service = service; + this.grantContext = grantContext; + this.principal = principal; + this.clock = clock; + this.defaultLifetime = defaultLifetime; + } + + @PostMapping + public ResponseEntity create(@RequestBody CreateDelegationRequest request) { + UUID accountId = principal.accountId(); + Set scopes = scopes(request.scopes()); + Set targets = targets(request.strategyIds()); + // A delegation with no expiry never stops working. The customer-visible disclosure + // promises one, so an omitted value becomes the configured default rather than nothing. + Instant expiresAt = request.expiresAt() != null + ? request.expiresAt() + : clock.instant().plus(defaultLifetime); + if (!expiresAt.isAfter(clock.instant())) { + throw new IllegalArgumentException("A delegation must expire in the future"); + } + + UUID authorizationId = UUID.randomUUID(); + UUID correlationId = UUID.randomUUID(); + String requestHash = hash(accountId, scopes, targets, expiresAt, request.name()); + DelegatedAuthorizationResult result = service.execute(new DelegatedAuthorizationCommand( + DelegatedAuthorizationCommandType.CREATE, + accountId, + authorizationId, + null, + 0L, + grantContext.currentAuthEpoch(accountId), + requireName(request.name()), + grantContext.currentDisclosurePolicyDocumentId(DISCLOSURE_POLICY_CODE), + scopes, + targets, + expiresAt, + "USER_REQUESTED", + "delegation-create:" + requestHash, + requestHash, + correlationId)); + + return ResponseEntity.status(HttpStatus.CREATED).body(new GrantResponse( + result.authorizationId(), + result.credentialId(), + result.rawCredential().orElse(null), + result.expiresAt(), + scopes.stream().map(Enum::name).sorted().toList(), + targets.stream().map(UUID::toString).sorted().toList())); + } + + @DeleteMapping("/{authorizationId}") + public ResponseEntity revoke(@PathVariable UUID authorizationId) { + UUID accountId = principal.accountId(); + String requestHash = hash(accountId, Set.of(), Set.of(authorizationId), null, "revoke"); + service.execute(new DelegatedAuthorizationCommand( + DelegatedAuthorizationCommandType.REVOKE, + accountId, + authorizationId, + null, + 1L, + grantContext.currentAuthEpoch(accountId), + "revoked", + grantContext.currentDisclosurePolicyDocumentId(DISCLOSURE_POLICY_CODE), + Set.of(), + Set.of(), + null, + "USER_REQUESTED", + "delegation-revoke:" + requestHash, + requestHash, + UUID.randomUUID())); + return ResponseEntity.noContent().build(); + } + + /** + * Only the two Basic editing scopes are reachable here. The enum carries wider ones for other + * flows, and an external tool must not be able to name them by spelling them in a request. + */ + private static Set scopes(List requested) { + if (requested == null || requested.isEmpty()) { + throw new IllegalArgumentException("At least one delegation scope is required"); + } + Set allowed = Set.of( + DelegatedAuthorizationScope.STRATEGY_EDIT, DelegatedAuthorizationScope.STRATEGY_VALIDATE); + Set scopes = requested.stream() + .map(String::trim) + .map(value -> { + try { + return DelegatedAuthorizationScope.valueOf(value); + } catch (IllegalArgumentException exception) { + throw new IllegalArgumentException("Unknown delegation scope: " + value); + } + }) + .collect(Collectors.toUnmodifiableSet()); + if (!allowed.containsAll(scopes)) { + throw new IllegalArgumentException("Only Basic edit and validation scopes may be delegated"); + } + return scopes; + } + + /** + * The authorization check requires a pinned target, so a delegation without one is granted, + * returned, and then authorizes nothing. Refusing it here keeps that from looking like success. + */ + private static Set targets(List strategyIds) { + if (strategyIds == null || strategyIds.isEmpty()) { + throw new IllegalArgumentException("A delegation must name at least one target strategy"); + } + return Set.copyOf(strategyIds); + } + + private static String requireName(String name) { + if (name == null || name.isBlank()) { + throw new IllegalArgumentException("A delegation name is required"); + } + return name.trim(); + } + + private static String hash( + UUID accountId, + Set scopes, + Set targets, + Instant expiresAt, + String name) { + TreeSet parts = new TreeSet<>(); + scopes.forEach(scope -> parts.add("scope:" + scope.name())); + targets.forEach(target -> parts.add("target:" + target)); + String canonical = accountId + "|" + name + "|" + expiresAt + "|" + String.join(",", parts); + try { + return HexFormat.of().formatHex(MessageDigest.getInstance("SHA-256") + .digest(canonical.getBytes(StandardCharsets.UTF_8))); + } catch (NoSuchAlgorithmException exception) { + throw new IllegalStateException("SHA-256 is required", exception); + } + } + + public record CreateDelegationRequest( + String name, List scopes, List strategyIds, Instant expiresAt) {} + + public record GrantResponse( + UUID authorizationId, + UUID credentialId, + String credential, + Instant expiresAt, + List scopes, + List strategyIds) { + @Override + public String toString() { + return "GrantResponse[credential=REDACTED]"; + } + } +} diff --git a/apps/backend-api/src/main/java/com/idea2strategy/backend/api/delegation/HmacDelegatedCredentials.java b/apps/backend-api/src/main/java/com/idea2strategy/backend/api/delegation/HmacDelegatedCredentials.java new file mode 100644 index 00000000..1f406cda --- /dev/null +++ b/apps/backend-api/src/main/java/com/idea2strategy/backend/api/delegation/HmacDelegatedCredentials.java @@ -0,0 +1,58 @@ +package com.idea2strategy.backend.api.delegation; + +import com.idea2strategy.backend.application.delegation.DelegatedCredentialMaterial; +import com.idea2strategy.backend.application.delegation.DelegatedCredentialMaterialPort; +import java.nio.charset.StandardCharsets; +import java.security.GeneralSecurityException; +import java.security.SecureRandom; +import java.util.Base64; +import java.util.Objects; +import javax.crypto.Mac; +import javax.crypto.spec.SecretKeySpec; + +/** + * Issues the secret a delegated tool holds. + * + *

Only the digest is stored, so a database reader cannot replay a delegation, and the raw value + * is returned exactly once at grant. This mirrors how customer refresh tokens are handled; the key + * version travels with the digest so a future key rotation can tell old rows from new ones instead + * of invalidating every delegation at once. + */ +public final class HmacDelegatedCredentials implements DelegatedCredentialMaterialPort { + private static final SecureRandom RANDOM = new SecureRandom(); + + private final byte[] key; + private final short keyVersion; + + public HmacDelegatedCredentials(byte[] key, short keyVersion) { + Objects.requireNonNull(key, "key"); + if (key.length < 32) { + throw new IllegalArgumentException("Delegated credential HMAC key must contain at least 256 bits"); + } + if (keyVersion < 1) { + throw new IllegalArgumentException("Delegated credential key version must be positive"); + } + this.key = key.clone(); + this.keyVersion = keyVersion; + } + + @Override + public DelegatedCredentialMaterial issue() { + byte[] value = new byte[32]; + RANDOM.nextBytes(value); + String raw = Base64.getUrlEncoder().withoutPadding().encodeToString(value); + return new DelegatedCredentialMaterial(raw, digest(raw), keyVersion); + } + + private String digest(String raw) { + try { + Mac mac = Mac.getInstance("HmacSHA256"); + mac.init(new SecretKeySpec(key, "HmacSHA256")); + return Base64.getUrlEncoder() + .withoutPadding() + .encodeToString(mac.doFinal(raw.getBytes(StandardCharsets.UTF_8))); + } catch (GeneralSecurityException exception) { + throw new IllegalStateException("Delegated credential digest failed", exception); + } + } +} diff --git a/apps/idea2strategy-cli/README.md b/apps/idea2strategy-cli/README.md index 23db0ec8..842a1b8e 100644 --- a/apps/idea2strategy-cli/README.md +++ b/apps/idea2strategy-cli/README.md @@ -34,7 +34,8 @@ Supported commands: ```text tool-contract -delegation create --name NAME --scopes STRATEGY_EDIT,STRATEGY_VALIDATE +delegation create --name NAME --scopes STRATEGY_EDIT,STRATEGY_VALIDATE --strategy-id ID[,ID...] + [--expires-at ISO_8601_INSTANT] delegation revoke --authorization-id ID strategy list [--limit 1..100] [--cursor CURSOR] strategy create --name NAME [--description TEXT] @@ -50,6 +51,11 @@ strategy release --strategy-id ID --validation-run-id ID --initial-cash-amount A operator bootstrap --manifest REVIEWED.json --expected-sha256 LOWERCASE_SHA256 ``` +A delegation must name the strategies it may edit; one that names none would be granted and then +authorize nothing. `--expires-at` is optional and defaults to 24 hours from the grant. The raw +credential is returned once, in the `create` response, and only its digest is stored — a lost +credential is revoked and replaced, never recovered. + `operator bootstrap` is a one-shot SSM/deployment command, never an HTTP bootstrap route. The reviewed manifest must name the dedicated PostgreSQL role expected for the deployment and contain only HMAC-protected operator identity material. Supply database connectivity only through `I2S_BOOTSTRAP_JDBC_URL`, diff --git a/apps/idea2strategy-cli/src/main/java/com/idea2strategy/cli/Idea2StrategyCli.java b/apps/idea2strategy-cli/src/main/java/com/idea2strategy/cli/Idea2StrategyCli.java index d5c41cd1..31f902a3 100644 --- a/apps/idea2strategy-cli/src/main/java/com/idea2strategy/cli/Idea2StrategyCli.java +++ b/apps/idea2strategy-cli/src/main/java/com/idea2strategy/cli/Idea2StrategyCli.java @@ -140,7 +140,7 @@ private static JsonNode login(Arguments args, ApiClient api, CredentialStore cre } private static JsonNode delegationCreate(Arguments args, ApiClient api, String token) { - args.rejectUnknown("--name", "--scopes"); + args.rejectUnknown("--name", "--scopes", "--strategy-id", "--expires-at"); ArrayNode scopes = JSON.createArrayNode(); for (String scope : args.required("--scopes").split(",")) { String normalized = scope.trim(); @@ -151,6 +151,20 @@ private static JsonNode delegationCreate(Arguments args, ApiClient api, String t } ObjectNode body = JSON.createObjectNode().put("name", args.required("--name")); body.set("scopes", scopes); + // A delegation the server cannot pin to a strategy authorizes nothing, so the CLI refuses + // to send one rather than reporting a grant that will deny every edit. + ArrayNode strategyIds = JSON.createArrayNode(); + for (String strategyId : args.required("--strategy-id").split(",")) { + String normalized = strategyId.trim(); + if (!normalized.isEmpty()) { + strategyIds.add(normalized); + } + } + if (strategyIds.isEmpty()) { + throw Arguments.usage("--strategy-id must name at least one strategy to delegate"); + } + body.set("strategyIds", strategyIds); + putOptional(body, "expiresAt", args.optional("--expires-at")); return api.post("/api/v1/delegations", body, token); } diff --git a/apps/idea2strategy-cli/src/test/java/com/idea2strategy/cli/Idea2StrategyCliTest.java b/apps/idea2strategy-cli/src/test/java/com/idea2strategy/cli/Idea2StrategyCliTest.java index de5a9609..23003ab4 100644 --- a/apps/idea2strategy-cli/src/test/java/com/idea2strategy/cli/Idea2StrategyCliTest.java +++ b/apps/idea2strategy-cli/src/test/java/com/idea2strategy/cli/Idea2StrategyCliTest.java @@ -117,6 +117,21 @@ void basicEditApplySendsOnlyAllowedOperationsWithPreviewHash() throws Exception .contains("\"expectedEditSequence\":12"); } + /** + * A delegation with no pinned target is granted and then denies every edit, so the failure + * would surface later as an unexplained scope denial rather than as the bad request it is. + */ + @Test + void delegationCreateWithoutATargetStrategyIsUsageErrorBeforeNetworkCall() throws Exception { + Files.writeString(tempDir.resolve("credentials.json"), "{\"sessionToken\":\"stored-token\"}"); + + Result result = run("", "--base-url", baseUrl, "--config-dir", tempDir.toString(), + "delegation", "create", "--name", "assistant", "--scopes", "STRATEGY_EDIT"); + + assertThat(result.exitCode()).isEqualTo(2); + assertThat(requestPath.get()).isNull(); + } + @Test void basicEditApplyWithoutReviewedEditSequenceIsUsageErrorBeforeNetworkCall() throws Exception { Files.writeString(tempDir.resolve("credentials.json"), "{\"sessionToken\":\"stored-token\"}"); @@ -180,7 +195,7 @@ void requiredWorkflowCommandsUseVersionedApiRoutes() throws Exception { Files.writeString(tempDir.resolve("credentials.json"), "{\"sessionToken\":\"stored-token\"}"); assertRoute("delegation", "create", "--name", "assistant", "--scopes", "STRATEGY_EDIT,STRATEGY_VALIDATE", - "POST", "/api/v1/delegations"); + "--strategy-id", "s1", "POST", "/api/v1/delegations"); assertRoute("strategy", "create", "--name", "draft", "POST", "/api/v1/strategies"); assertRoute("strategy", "copy", "--strategy-id", "s1", "--name", "copy", "POST", "/api/v1/strategies/s1/copies"); diff --git a/modules/backend-application/src/main/java/com/idea2strategy/backend/application/delegation/DelegationGrantContextPort.java b/modules/backend-application/src/main/java/com/idea2strategy/backend/application/delegation/DelegationGrantContextPort.java new file mode 100644 index 00000000..4320f583 --- /dev/null +++ b/modules/backend-application/src/main/java/com/idea2strategy/backend/application/delegation/DelegationGrantContextPort.java @@ -0,0 +1,17 @@ +package com.idea2strategy.backend.application.delegation; + +import java.util.UUID; + +/** + * The two facts a grant needs that the caller must not supply. + * + *

Both are read rather than accepted from the request on purpose. The auth epoch is what makes a + * delegation die when the account re-authenticates, so a client that could name it could outlive a + * password change. The disclosure document is the text the customer was actually shown; letting a + * request choose it would let a delegation claim consent to something else. + */ +public interface DelegationGrantContextPort { + long currentAuthEpoch(UUID accountId); + + UUID currentDisclosurePolicyDocumentId(String policyCode); +} diff --git a/modules/backend-persistence/src/main/java/com/idea2strategy/backend/persistence/delegation/DelegatedAuthorizationJooqAdapter.java b/modules/backend-persistence/src/main/java/com/idea2strategy/backend/persistence/delegation/DelegatedAuthorizationJooqAdapter.java new file mode 100644 index 00000000..ef6d7549 --- /dev/null +++ b/modules/backend-persistence/src/main/java/com/idea2strategy/backend/persistence/delegation/DelegatedAuthorizationJooqAdapter.java @@ -0,0 +1,273 @@ +package com.idea2strategy.backend.persistence.delegation; + +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationCommand; +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationCommandPort; +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationCommandType; +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationDecision; +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationExecution; +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationMutation; +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationResult; +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationScope; +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationSnapshot; +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationStatus; +import com.idea2strategy.backend.application.delegation.DelegationGrantContextPort; +import java.time.Instant; +import java.time.ZoneOffset; +import java.util.Objects; +import java.util.Optional; +import java.util.TreeSet; +import java.util.UUID; +import org.jooq.DSLContext; +import org.jooq.Record; +import org.springframework.stereotype.Component; +import org.springframework.transaction.annotation.Transactional; + +/** + * Grants, replaces, and revokes delegated authorizations in one transaction. + * + *

The event row is the idempotency receipt: {@code (authorization_id, idempotency_key)} is + * unique, so a replayed request finds its own event and returns the state it already produced + * without issuing a second credential. That ordering matters — deciding first and detecting the + * replay afterwards would mint a credential the caller never receives. + */ +@Component +public class DelegatedAuthorizationJooqAdapter + implements DelegatedAuthorizationCommandPort, DelegationGrantContextPort { + private final DSLContext dsl; + + public DelegatedAuthorizationJooqAdapter(DSLContext dsl) { + this.dsl = Objects.requireNonNull(dsl, "dsl"); + } + + @Override + public long currentAuthEpoch(UUID accountId) { + Record epochRow = dsl.fetchOne( + "select auth_epoch from identity.account_security_states where account_id = ?", + accountId); + Long epoch = epochRow == null ? null : epochRow.get("auth_epoch", Long.class); + if (epoch == null) { + throw new java.util.NoSuchElementException("Account has no security state"); + } + return epoch; + } + + @Override + public UUID currentDisclosurePolicyDocumentId(String policyCode) { + Record documentRow = dsl.fetchOne( + "select id from identity.policy_documents " + + "where policy_code = ? and retired_at is null and published_at <= now() " + + "order by published_at desc, version desc, id desc limit 1", + policyCode); + UUID id = documentRow == null ? null : documentRow.get("id", UUID.class); + if (id == null) { + throw new java.util.NoSuchElementException( + "No published disclosure document for " + policyCode); + } + return id; + } + + @Override + @Transactional + public DelegatedAuthorizationExecution executeAtomically( + DelegatedAuthorizationCommand command, Instant at, DelegatedAuthorizationDecision decision) { + UUID subjectId = command.commandType() == DelegatedAuthorizationCommandType.REPLACE + ? command.replacesAuthorizationId() + : command.authorizationId(); + + Optional replayed = findReceipt(command, subjectId); + if (replayed.isPresent()) { + return new DelegatedAuthorizationExecution(replayed.get(), false); + } + + DelegatedAuthorizationMutation mutation = decision.decide(loadSnapshot(subjectId, command.accountId())); + apply(mutation, command, at); + return new DelegatedAuthorizationExecution( + new DelegatedAuthorizationResult( + mutation.authorizationId(), + mutation.authorizationVersion(), + mutation.status(), + mutation.credentialId(), + mutation.expiresAt(), + Optional.empty()), + true); + } + + /** + * A replay is answered from the authorization the receipt belongs to, never from the request, + * so a caller repeating a command with drifted arguments still learns what actually happened. + */ + private Optional findReceipt( + DelegatedAuthorizationCommand command, UUID subjectId) { + Record row = dsl.fetchOne( + "select a.id, a.authorization_version, a.status::text as status, a.expires_at, " + + "(select c.id from identity.delegated_credentials c " + + " where c.authorization_id = a.id order by c.issued_at desc limit 1) as credential_id " + + "from identity.delegated_authorization_events e " + + "join identity.delegated_authorizations a on a.id = e.authorization_id " + + "where e.idempotency_key = ? and a.account_id = ? " + + "and e.authorization_id in (?, ?)", + command.idempotencyKey(), command.accountId(), subjectId, command.authorizationId()); + if (row == null) { + return Optional.empty(); + } + return Optional.of(new DelegatedAuthorizationResult( + row.get("id", UUID.class), + row.get("authorization_version", Long.class), + DelegatedAuthorizationStatus.valueOf(row.get("status", String.class)), + row.get("credential_id", UUID.class), + instant(row, "expires_at"), + Optional.empty())); + } + + private Optional loadSnapshot(UUID subjectId, UUID accountId) { + if (subjectId == null) { + return Optional.empty(); + } + Record row = dsl.fetchOne( + "select id, account_id, authorization_version, status::text as status, " + + "auth_epoch_at_grant, expires_at, revoked_at " + + "from identity.delegated_authorizations " + + "where id = ? and account_id = ? for update", + subjectId, accountId); + if (row == null) { + return Optional.empty(); + } + return Optional.of(new DelegatedAuthorizationSnapshot( + row.get("id", UUID.class), + row.get("account_id", UUID.class), + row.get("authorization_version", Long.class), + DelegatedAuthorizationStatus.valueOf(row.get("status", String.class)), + row.get("auth_epoch_at_grant", Long.class), + instant(row, "expires_at"), + instant(row, "revoked_at"))); + } + + private void apply( + DelegatedAuthorizationMutation mutation, DelegatedAuthorizationCommand command, Instant at) { + if (mutation.status() == DelegatedAuthorizationStatus.REVOKED) { + revoke(mutation, at); + } else { + grant(mutation, at); + } + recordEvent(mutation, command, at); + } + + private void grant(DelegatedAuthorizationMutation mutation, Instant at) { + if (mutation.replacesAuthorizationId() != null) { + dsl.execute( + "update identity.delegated_authorizations set status = 'REVOKED', " + + "revoked_at = ?::timestamptz, revoke_reason_code = 'REPLACED' where id = ?", + offset(mutation.predecessorRevokedAt() == null ? at : mutation.predecessorRevokedAt()), + mutation.replacesAuthorizationId()); + } + + // AT_TIME whenever the grant carries an expiry, so the authorization check's + // `expiry_mode <> 'AT_TIME' or expires_at > now` clause actually enforces it. + String expiryMode = mutation.expiresAt() == null ? "UNTIL_REVOKED" : "AT_TIME"; + dsl.execute( + "insert into identity.delegated_authorizations (" + + "id, account_id, client_label, status, expiry_mode, auth_epoch_at_grant, " + + "disclosure_policy_document_id, scope_set_hash, authorized_at, expires_at, " + + "authorization_version, replaces_authorization_id, strategy_target_set_hash) " + + "values (?, ?, ?, 'ACTIVE', ?::identity.delegated_expiry_mode, ?, ?, ?, " + + "?::timestamptz, ?::timestamptz, ?, ?, ?)", + mutation.authorizationId(), mutation.accountId(), mutation.clientLabel(), expiryMode, + mutation.authEpochAtGrant(), mutation.disclosurePolicyDocumentId(), + setHash(mutation.scopes().stream().map(Enum::name).toList()), + offset(mutation.occurredAt()), offset(mutation.expiresAt()), + mutation.authorizationVersion(), mutation.replacesAuthorizationId(), + setHash(mutation.targetStrategyIds().stream().map(UUID::toString).toList())); + + for (DelegatedAuthorizationScope scope : mutation.scopes()) { + dsl.execute( + "insert into identity.delegated_authorization_scopes " + + "(authorization_id, scope_code, granted_at) " + + "values (?, ?::identity.delegated_scope, ?::timestamptz)", + mutation.authorizationId(), scope.name(), offset(mutation.occurredAt())); + } + + // The owner and access epoch are pinned from the strategy row at grant time. The + // authorization check compares them back, so a later owner change or access-epoch bump + // silently stops honouring a delegation instead of carrying it across the change. + for (UUID strategyId : mutation.targetStrategyIds()) { + int pinned = dsl.execute( + "insert into identity.delegated_authorization_strategy_targets " + + "(authorization_id, strategy_id, owner_account_id_at_grant, " + + " strategy_access_epoch_at_grant, granted_at) " + + "select ?, s.id, s.owner_account_id, s.delegated_access_epoch, ?::timestamptz " + + "from strategy.strategies s " + + "where s.id = ? and s.owner_account_id = ? and s.mode = 'BASIC' " + + "and s.archived_at is null and s.deleted_at is null", + mutation.authorizationId(), offset(mutation.occurredAt()), strategyId, + mutation.accountId()); + if (pinned != 1) { + throw new DelegatedStrategyTargetRejectedException( + "Delegation targets must be the account's own Basic strategies"); + } + } + + dsl.execute( + "insert into identity.delegated_credentials (" + + "id, authorization_id, credential_type, token_digest, digest_key_version, " + + "issued_at, expires_at) " + + "values (?, ?, 'ACCESS_TOKEN', ?, ?, ?::timestamptz, ?::timestamptz)", + mutation.credentialId(), mutation.authorizationId(), mutation.credentialDigest(), + mutation.digestKeyVersion(), offset(mutation.occurredAt()), + offset(mutation.expiresAt() == null ? mutation.occurredAt().plusSeconds(86_400) : mutation.expiresAt())); + } + + private void revoke(DelegatedAuthorizationMutation mutation, Instant at) { + dsl.execute( + "update identity.delegated_authorizations set status = 'REVOKED', " + + "revoked_at = ?::timestamptz, revoke_reason_code = ? where id = ?", + offset(at), mutation.reasonCode(), mutation.authorizationId()); + dsl.execute( + "update identity.delegated_credentials set revoked_at = ?::timestamptz, " + + "revoke_reason_code = ? where authorization_id = ? and revoked_at is null", + offset(at), mutation.reasonCode(), mutation.authorizationId()); + } + + private void recordEvent( + DelegatedAuthorizationMutation mutation, DelegatedAuthorizationCommand command, Instant at) { + dsl.execute( + "insert into identity.delegated_authorization_events (" + + "id, authorization_id, event_sequence, event_type, actor_type, actor_id, " + + "reason_code, correlation_id, idempotency_key, occurred_at, payload_document) " + + "select ?, ?, coalesce(max(e.event_sequence), 0) + 1, ?, 'USER', ?, ?, ?, ?, " + + "?::timestamptz, ?::jsonb " + + "from identity.delegated_authorization_events e where e.authorization_id = ?", + UUID.randomUUID(), mutation.authorizationId(), mutation.commandType().name(), + mutation.accountId(), mutation.reasonCode(), command.correlationId(), + command.idempotencyKey(), offset(at), + // The request hash is what ties the receipt to the arguments that produced it. + // No scope, target, or credential value is recorded here. + "{\"requestHash\":\"" + command.requestHash() + "\"}", + mutation.authorizationId()); + } + + /** + * Order-independent so the same set never produces two hashes, and a real digest rather than + * {@code String.hashCode}: these columns are how a later reader decides whether two grants + * carry the same scopes or targets, and a 32-bit value collides too easily to answer that. + */ + private static String setHash(Iterable values) { + TreeSet sorted = new TreeSet<>(); + values.forEach(sorted::add); + try { + byte[] digest = java.security.MessageDigest.getInstance("SHA-256") + .digest(String.join(",", sorted).getBytes(java.nio.charset.StandardCharsets.UTF_8)); + return java.util.HexFormat.of().formatHex(digest); + } catch (java.security.NoSuchAlgorithmException exception) { + throw new IllegalStateException("SHA-256 is required", exception); + } + } + + private static java.time.OffsetDateTime offset(Instant value) { + return value == null ? null : value.atOffset(ZoneOffset.UTC); + } + + private static Instant instant(Record row, String column) { + java.time.OffsetDateTime value = row.get(column, java.time.OffsetDateTime.class); + return value == null ? null : value.toInstant(); + } +} diff --git a/modules/backend-persistence/src/main/java/com/idea2strategy/backend/persistence/delegation/DelegatedStrategyTargetRejectedException.java b/modules/backend-persistence/src/main/java/com/idea2strategy/backend/persistence/delegation/DelegatedStrategyTargetRejectedException.java new file mode 100644 index 00000000..2fc94615 --- /dev/null +++ b/modules/backend-persistence/src/main/java/com/idea2strategy/backend/persistence/delegation/DelegatedStrategyTargetRejectedException.java @@ -0,0 +1,15 @@ +package com.idea2strategy.backend.persistence.delegation; + +/** + * A delegation named a strategy it cannot target. + * + *

The target insert selects from the strategy row itself, so this is raised when the strategy is + * not the granting account's own live Basic strategy. Failing here rather than skipping the row + * matters: a delegation whose targets silently vanished would be granted, returned to the caller, + * and then authorize nothing. + */ +public class DelegatedStrategyTargetRejectedException extends RuntimeException { + public DelegatedStrategyTargetRejectedException(String message) { + super(message); + } +} diff --git a/modules/backend-persistence/src/test/java/com/idea2strategy/backend/persistence/delegation/DelegatedAuthorizationGrantPersistenceIntegrationTest.java b/modules/backend-persistence/src/test/java/com/idea2strategy/backend/persistence/delegation/DelegatedAuthorizationGrantPersistenceIntegrationTest.java new file mode 100644 index 00000000..0e41a854 --- /dev/null +++ b/modules/backend-persistence/src/test/java/com/idea2strategy/backend/persistence/delegation/DelegatedAuthorizationGrantPersistenceIntegrationTest.java @@ -0,0 +1,239 @@ +package com.idea2strategy.backend.persistence.delegation; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationCommand; +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationCommandType; +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationScope; +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationService; +import com.idea2strategy.backend.application.delegation.DelegatedAuthorizationStatus; +import com.idea2strategy.backend.application.delegation.DelegatedCredentialMaterial; +import com.idea2strategy.backend.application.strategy.DelegatedStrategyEditor; +import com.idea2strategy.backend.application.strategy.DelegatedStrategyScope; +import com.idea2strategy.backend.application.strategy.DelegatedStrategyScopeDeniedException; +import com.idea2strategy.backend.persistence.strategy.DelegatedBasicStrategyEditJooqAdapter; +import java.time.Clock; +import java.time.Instant; +import java.time.ZoneOffset; +import java.util.Set; +import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.context.annotation.Import; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; +import org.testcontainers.containers.PostgreSQLContainer; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; + +/** + * Grants a delegation with the production adapter, then edits under it with the production + * authorization check. + * + *

This is the join the project had no coverage for. The edit check reads ten columns across five + * tables — status, expiry mode and instant, auth epoch, credential type and expiry, pinned owner and + * access epoch — and any one of them written differently by the grant makes every edit deny with no + * indication of which column was wrong. Asserting the grant's rows in isolation would not catch + * that; only running the real check against the real grant does. + */ +@Testcontainers(disabledWithoutDocker = true) +@SpringBootTest(classes = DelegatedAuthorizationGrantPersistenceIntegrationTest.TestApplication.class) +class DelegatedAuthorizationGrantPersistenceIntegrationTest { + private static final UUID ACCOUNT_ID = UUID.fromString("10000000-0000-4000-8000-000000000092"); + private static final UUID STRATEGY_ID = UUID.fromString("50000000-0000-4000-8000-000000000092"); + private static final UUID OTHER_STRATEGY_ID = UUID.fromString("50000000-0000-4000-8000-000000000093"); + private static final Instant NOW = Instant.parse("2026-08-01T12:00:00Z"); + private static final String HASH_A = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + private static final String HASH_B = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; + + @Container + static final PostgreSQLContainer POSTGRES = new PostgreSQLContainer("postgres:16-alpine"); + + @DynamicPropertySource + static void databaseProperties(DynamicPropertyRegistry registry) { + registry.add("spring.datasource.url", POSTGRES::getJdbcUrl); + registry.add("spring.datasource.username", POSTGRES::getUsername); + registry.add("spring.datasource.password", POSTGRES::getPassword); + registry.add("spring.jpa.hibernate.ddl-auto", () -> "none"); + registry.add("spring.flyway.enabled", () -> "true"); + } + + @Autowired + private DelegatedAuthorizationJooqAdapter grantAdapter; + + @Autowired + private DelegatedBasicStrategyEditJooqAdapter editAdapter; + + @Autowired + private JdbcTemplate jdbc; + + private DelegatedAuthorizationService service; + + /** + * Resolved from the database rather than inserted here: the migration already publishes this + * document, and a fixture that inserted its own would silently stop covering the row a real + * grant actually points at. + */ + private UUID policyId; + + @BeforeEach + void prepareAccountAndStrategies() { + var at = NOW.atOffset(ZoneOffset.UTC); + jdbc.update("delete from identity.delegated_authorization_events"); + jdbc.update( + "insert into identity.accounts (id, lifecycle_status, status_changed_at) values (?, 'ACTIVE', ?) " + + "on conflict (id) do nothing", + ACCOUNT_ID, at); + jdbc.update( + "insert into identity.account_security_states (account_id, auth_epoch, updated_at) " + + "values (?, 4, ?) on conflict (account_id) do nothing", + ACCOUNT_ID, at); + policyId = grantAdapter.currentDisclosurePolicyDocumentId("delegation.strategy-edit.disclosure"); + insertStrategy(STRATEGY_ID, "Delegated draft"); + insertStrategy(OTHER_STRATEGY_ID, "Untargeted draft"); + + service = new DelegatedAuthorizationService( + grantAdapter, + // token_digest is UNIQUE, as it must be: two delegations sharing a digest would + // authorize each other. The real HMAC of a random 256-bit value satisfies that, so + // the fake has to vary too or it tests a constraint violation instead of a grant. + () -> new DelegatedCredentialMaterial( + "raw-" + UUID.randomUUID(), + UUID.randomUUID().toString().replace("-", "").repeat(2), + (short) 1), + Clock.fixed(NOW, ZoneOffset.UTC), + UUID::randomUUID); + } + + @Test + void grantsADelegationTheEditCheckAccepts() { + UUID authorizationId = UUID.randomUUID(); + + var result = service.execute(createCommand(authorizationId, "grant-1", Set.of(STRATEGY_ID))); + + assertThat(result.status()).isEqualTo(DelegatedAuthorizationStatus.ACTIVE); + assertThat(result.rawCredential()).isPresent(); + editAdapter.requireAuthorized( + new DelegatedStrategyEditor(ACCOUNT_ID, authorizationId, result.credentialId()), + STRATEGY_ID, + DelegatedStrategyScope.STRATEGY_EDIT, + NOW.plusSeconds(60)); + } + + @Test + void deniesAStrategyTheGrantNeverTargeted() { + UUID authorizationId = UUID.randomUUID(); + var result = service.execute(createCommand(authorizationId, "grant-2", Set.of(STRATEGY_ID))); + + assertThatThrownBy(() -> editAdapter.requireAuthorized( + new DelegatedStrategyEditor(ACCOUNT_ID, authorizationId, result.credentialId()), + OTHER_STRATEGY_ID, + DelegatedStrategyScope.STRATEGY_EDIT, + NOW.plusSeconds(60))) + .isInstanceOf(DelegatedStrategyScopeDeniedException.class); + } + + /** The disclosure promises an expiry, so a grant that carries one must actually stop working. */ + @Test + void deniesTheDelegationOnceItsExpiryHasPassed() { + UUID authorizationId = UUID.randomUUID(); + var result = service.execute(createCommand(authorizationId, "grant-3", Set.of(STRATEGY_ID))); + + assertThatThrownBy(() -> editAdapter.requireAuthorized( + new DelegatedStrategyEditor(ACCOUNT_ID, authorizationId, result.credentialId()), + STRATEGY_ID, + DelegatedStrategyScope.STRATEGY_EDIT, + NOW.plusSeconds(86_400 + 60))) + .isInstanceOf(DelegatedStrategyScopeDeniedException.class); + } + + @Test + void replayingTheSameCommandReturnsTheFirstGrantWithoutIssuingASecondCredential() { + UUID authorizationId = UUID.randomUUID(); + var command = createCommand(authorizationId, "grant-4", Set.of(STRATEGY_ID)); + + var first = service.execute(command); + var replay = service.execute(command); + + assertThat(replay.authorizationId()).isEqualTo(first.authorizationId()); + assertThat(replay.credentialId()).isEqualTo(first.credentialId()); + assertThat(replay.rawCredential()).isEmpty(); + assertThat(jdbc.queryForObject( + "select count(*) from identity.delegated_credentials where authorization_id = ?", + Integer.class, + authorizationId)) + .isEqualTo(1); + } + + @Test + void revokedDelegationsStopAuthorizingImmediately() { + UUID authorizationId = UUID.randomUUID(); + var result = service.execute(createCommand(authorizationId, "grant-5", Set.of(STRATEGY_ID))); + + service.execute(new DelegatedAuthorizationCommand( + DelegatedAuthorizationCommandType.REVOKE, ACCOUNT_ID, authorizationId, null, 1L, 4L, + "revoked", policyId, Set.of(), Set.of(), null, "USER_REQUESTED", "revoke-5", + "revoke-hash-5", UUID.randomUUID())); + + assertThatThrownBy(() -> editAdapter.requireAuthorized( + new DelegatedStrategyEditor(ACCOUNT_ID, authorizationId, result.credentialId()), + STRATEGY_ID, + DelegatedStrategyScope.STRATEGY_EDIT, + NOW.plusSeconds(60))) + .isInstanceOf(DelegatedStrategyScopeDeniedException.class); + } + + @Test + void refusesToTargetAStrategyTheAccountDoesNotOwn() { + UUID foreignStrategy = UUID.randomUUID(); + + assertThatThrownBy(() -> service.execute( + createCommand(UUID.randomUUID(), "grant-6", Set.of(foreignStrategy)))) + .isInstanceOf(DelegatedStrategyTargetRejectedException.class); + } + + private DelegatedAuthorizationCommand createCommand( + UUID authorizationId, String idempotencyKey, Set targets) { + return new DelegatedAuthorizationCommand( + DelegatedAuthorizationCommandType.CREATE, + ACCOUNT_ID, + authorizationId, + null, + 0L, + 4L, + "external-assistant", + policyId, + Set.of(DelegatedAuthorizationScope.STRATEGY_EDIT), + targets, + NOW.plusSeconds(86_400), + "USER_REQUESTED", + idempotencyKey, + "request-hash-" + idempotencyKey, + UUID.randomUUID()); + } + + private void insertStrategy(UUID strategyId, String name) { + var at = NOW.atOffset(ZoneOffset.UTC); + jdbc.update( + "insert into strategy.strategies " + + "(id, owner_account_id, mode, name, edit_sequence, created_at, updated_at) " + + "values (?, ?, 'BASIC', ?, 1, ?, ?) on conflict (id) do nothing", + strategyId, ACCOUNT_ID, name, at, at); + jdbc.update( + "insert into strategy.strategy_documents " + + "(strategy_id, semantic_document, presentation_document, semantic_schema_version, " + + "presentation_schema_version, semantic_hash, presentation_hash, edit_sequence, " + + "created_at, updated_at) values (?, '{}'::jsonb, '{}'::jsonb, 'basic-semantic/v1', " + + "'basic-presentation/v1', ?, ?, 1, ?, ?) on conflict (strategy_id) do nothing", + strategyId, HASH_A, HASH_B, at, at); + } + + @SpringBootApplication + @Import({DelegatedAuthorizationJooqAdapter.class, DelegatedBasicStrategyEditJooqAdapter.class}) + static class TestApplication {} +} From 21f4355e3a291fb3999ed0b58db9f3512bb5c253 Mon Sep 17 00:00:00 2001 From: Ryan Min Date: Mon, 10 Aug 2026 00:22:32 +0900 Subject: [PATCH 7/7] test: run the external tool journey over HTTP, not against a stub Sign up, verify, log in, create a strategy, grant a delegation for it, reach the edit service under that delegation, and lose access the moment it is revoked. The refusal codes are the assertions: EDIT_REJECTED means the delegation was accepted and the request reached the edit service on its merits, SCOPE_DENIED after revoking means authorization is what stopped it, and before this branch the same call answered 404 because the route did not exist. The test stops short of applying blocks. A new strategy is {"groups":[],"mode": "BASIC"} and none of the four delegated operations creates a group, so an external tool can only fill a skeleton the owner already made. Covering a real apply needs a valid Basic assembly with a catalog and instruments, which belongs in a strategy-authoring fixture rather than in this journey. Refs #479 Co-Authored-By: Claude Opus 5 --- ...olDelegatedEditJourneyIntegrationTest.java | 160 ++++++++++++++++++ 1 file changed, 160 insertions(+) create mode 100644 apps/backend-api/src/test/java/com/idea2strategy/backend/api/journey/ExternalToolDelegatedEditJourneyIntegrationTest.java diff --git a/apps/backend-api/src/test/java/com/idea2strategy/backend/api/journey/ExternalToolDelegatedEditJourneyIntegrationTest.java b/apps/backend-api/src/test/java/com/idea2strategy/backend/api/journey/ExternalToolDelegatedEditJourneyIntegrationTest.java new file mode 100644 index 00000000..95573db3 --- /dev/null +++ b/apps/backend-api/src/test/java/com/idea2strategy/backend/api/journey/ExternalToolDelegatedEditJourneyIntegrationTest.java @@ -0,0 +1,160 @@ +package com.idea2strategy.backend.api.journey; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.idea2strategy.backend.api.identity.AccountVerificationEmailRequested; +import java.nio.charset.StandardCharsets; +import java.util.Base64; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.http.MediaType; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; +import org.springframework.test.context.event.ApplicationEvents; +import org.springframework.test.context.event.RecordApplicationEvents; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.setup.MockMvcBuilders; +import org.springframework.web.context.WebApplicationContext; +import org.testcontainers.containers.PostgreSQLContainer; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; + +/** + * The journey an external AI tool actually performs, over HTTP. + * + *

Every step here was already covered somewhere — the CLI against a stub, the edit service + * against fakes, the grant against Postgres — and the product still could not do it, because the + * routes the CLI posts to did not exist. Covering the pieces is what let that happen, so this test + * runs the line end to end over HTTP: sign up, log in, create a strategy, delegate editing of it, + * reach the edit service under that delegation, and lose access the moment it is revoked. + * + *

It stops short of applying blocks. A new strategy has no groups and the delegated operations + * cannot create one, so a real apply needs a valid Basic skeleton with a catalog and instruments; + * that belongs in a strategy-authoring fixture rather than here. What this test does establish is + * the part that was actually broken — that the routes exist and that a granted delegation carries + * a request through authorization, which no stub could show. + */ +@Testcontainers(disabledWithoutDocker = true) +@SpringBootTest +@RecordApplicationEvents +class ExternalToolDelegatedEditJourneyIntegrationTest { + private static final String EMAIL = "delegated-edit@example.com"; + private static final String PASSWORD = "CorrectHorse!2026"; + + @Container + static final PostgreSQLContainer POSTGRES = new PostgreSQLContainer("postgres:16-alpine"); + + @DynamicPropertySource + static void properties(DynamicPropertyRegistry registry) { + registry.add("spring.datasource.url", POSTGRES::getJdbcUrl); + registry.add("spring.datasource.username", POSTGRES::getUsername); + registry.add("spring.datasource.password", POSTGRES::getPassword); + registry.add("spring.jpa.hibernate.ddl-auto", () -> "none"); + registry.add("spring.flyway.enabled", () -> "true"); + String key = Base64.getEncoder().encodeToString( + "01234567890123456789012345678901".getBytes(StandardCharsets.UTF_8)); + String jwtKey = Base64.getEncoder().encodeToString( + "abcdefabcdefabcdefabcdefabcdefab".getBytes(StandardCharsets.UTF_8)); + registry.add("identity.crypto.email-encryption-key", () -> key); + registry.add("identity.crypto.lookup-hmac-key", () -> key); + registry.add("identity.crypto.verification-hmac-key", () -> key); + registry.add("identity.crypto.refresh-token-hmac-key", () -> key); + registry.add("identity.crypto.customer-jwt-signing-key", () -> jwtKey); + } + + @Autowired WebApplicationContext context; + @Autowired ObjectMapper json; + @Autowired ApplicationEvents events; + + @Test + void anExternalToolDelegatesThenPreviewsAndAppliesABasicEdit() throws Exception { + MockMvc mvc = MockMvcBuilders.webAppContextSetup(context).build(); + + mvc.perform(post("/api/v1/auth/signup") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"email":"%s","password":"%s","nickname":"delegator"} + """.formatted(EMAIL, PASSWORD))) + .andExpect(status().isAccepted()); + String verificationToken = events.stream(AccountVerificationEmailRequested.class) + .findFirst() + .map(AccountVerificationEmailRequested::verificationToken) + .orElseThrow(); + mvc.perform(post("/api/v1/auth/verify-email") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"verificationToken":"%s"} + """.formatted(verificationToken))) + .andExpect(status().isNoContent()); + + String accessToken = json.readTree(mvc.perform(post("/api/v1/auth/login") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"email":"%s","password":"%s"} + """.formatted(EMAIL, PASSWORD))) + .andExpect(status().isOk()) + .andReturn().getResponse().getContentAsString()) + .path("accessToken").asText(); + + String strategyId = json.readTree(mvc.perform(post("/api/v1/strategies") + .header("Authorization", "Bearer " + accessToken) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"name\":\"Delegated draft\",\"mode\":\"BASIC\"}")) + .andExpect(status().isCreated()) + .andReturn().getResponse().getContentAsString()) + .path("id").asText(); + + JsonNode grant = json.readTree(mvc.perform(post("/api/v1/delegations") + .header("Authorization", "Bearer " + accessToken) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + {"name":"assistant","scopes":["STRATEGY_EDIT"],"strategyIds":["%s"]} + """.formatted(strategyId))) + .andExpect(status().isCreated()) + .andReturn().getResponse().getContentAsString()); + // Returned exactly once. Nothing later in the journey can recover it. + assertThat(grant.path("credential").asText()).isNotBlank(); + + String editBody = """ + {"authorizationId":"%s","credentialId":"%s","operations":[ + {"action":"ADD_BLOCK","arguments":{"groupId":"buy","blockId":"b1", + "elementCode":"PRICE_CHANGE_PERCENT"}}]} + """.formatted( + grant.path("authorizationId").asText(), grant.path("credentialId").asText()); + + // A freshly created strategy is {"groups":[],"mode":"BASIC"} and the four delegated + // operations cannot create a group, so this edit is refused on its merits — which is the + // assertion that matters here. EDIT_REJECTED means the delegation was accepted and the + // request reached the edit service; a delegation that did not authorize would answer 403 + // SCOPE_DENIED, and a missing route would answer 404, which is what it did before this + // change. Applying real blocks needs a valid Basic skeleton and is covered separately. + JsonNode refusal = json.readTree(mvc.perform( + post("/api/v1/strategies/" + strategyId + "/basic-edits/preview") + .header("Authorization", "Bearer " + accessToken) + .contentType(MediaType.APPLICATION_JSON) + .content(editBody)) + .andExpect(status().isUnprocessableEntity()) + .andReturn().getResponse().getContentAsString()); + assertThat(refusal.path("code").asText()).isEqualTo("EDIT_REJECTED"); + + // Revoking takes effect at once: the same call now fails authorization instead of merits. + mvc.perform(org.springframework.test.web.servlet.request.MockMvcRequestBuilders + .delete("/api/v1/delegations/" + grant.path("authorizationId").asText()) + .header("Authorization", "Bearer " + accessToken)) + .andExpect(status().isNoContent()); + + JsonNode denied = json.readTree(mvc.perform( + post("/api/v1/strategies/" + strategyId + "/basic-edits/preview") + .header("Authorization", "Bearer " + accessToken) + .contentType(MediaType.APPLICATION_JSON) + .content(editBody)) + .andExpect(status().isForbidden()) + .andReturn().getResponse().getContentAsString()); + assertThat(denied.path("code").asText()).isEqualTo("SCOPE_DENIED"); + } +}