diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatAppService.java index 801b2ed2..4c8c9491 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/compat/ClawHubCompatAppService.java @@ -12,6 +12,7 @@ import com.iflytek.skillhub.compat.dto.ClawHubUnstarResponse; import com.iflytek.skillhub.compat.dto.ClawHubWhoamiResponse; import com.iflytek.skillhub.controller.support.MultipartPackageExtractor; import com.iflytek.skillhub.controller.support.ZipPackageExtractor; +import com.iflytek.skillhub.domain.audit.AuditDetail; import com.iflytek.skillhub.domain.audit.AuditLogService; import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; @@ -345,7 +346,7 @@ public class ClawHubCompatAppService { confirmWarnings ); recordCompatPublishAudit(principal.userId(), result.version().getId(), clientIp, userAgent, - "{\"namespace\":\"" + namespace + "\",\"slug\":\"" + extracted.payload().slug() + "\"}"); + AuditDetail.of("namespace", namespace, "slug", extracted.payload().slug())); return new ClawHubPublishResponse(result.skillId().toString(), result.version().getId().toString()); } @@ -364,7 +365,7 @@ public class ClawHubCompatAppService { confirmWarnings ); recordCompatPublishAudit(principal.userId(), result.version().getId(), clientIp, userAgent, - "{\"namespace\":\"" + namespace + "\"}"); + AuditDetail.of("namespace", namespace)); return new ClawHubPublishResponse(result.skillId().toString(), result.version().getId().toString()); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/DeviceAuthWebController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/DeviceAuthWebController.java index 71f27a19..44dd64d2 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/DeviceAuthWebController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/DeviceAuthWebController.java @@ -2,6 +2,7 @@ package com.iflytek.skillhub.controller; import com.iflytek.skillhub.auth.device.DeviceAuthService; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.domain.audit.AuditDetail; import com.iflytek.skillhub.domain.audit.AuditLogService; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; @@ -51,7 +52,7 @@ public class DeviceAuthWebController extends BaseApiController { requestIdAccessor.current(), httpRequest.getRemoteAddr(), httpRequest.getHeader("User-Agent"), - "{\"userCode\":\"" + request.userCode() + "\"}" + AuditDetail.of("userCode", request.userCode()) ); return ok("response.success.updated", new MessageResponse("Device authorized successfully")); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSearchController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSearchController.java index 87770644..899d80ad 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSearchController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/AdminSearchController.java @@ -2,6 +2,7 @@ package com.iflytek.skillhub.controller.admin; import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; import com.iflytek.skillhub.controller.BaseApiController; +import com.iflytek.skillhub.domain.audit.AuditDetail; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.ApiResponseFactory; import com.iflytek.skillhub.domain.audit.AuditLogService; @@ -48,7 +49,7 @@ public class AdminSearchController extends BaseApiController { requestIdAccessor.current(), httpRequest.getRemoteAddr(), httpRequest.getHeader("User-Agent"), - "{\"scope\":\"ALL\"}" + AuditDetail.of("scope", "ALL") ); return ok("response.success.updated", null); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/LabelAdminAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/LabelAdminAppService.java index 88316c50..8b69b1fe 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/LabelAdminAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/LabelAdminAppService.java @@ -1,6 +1,7 @@ package com.iflytek.skillhub.service; import com.iflytek.skillhub.auth.rbac.RbacService; +import com.iflytek.skillhub.domain.audit.AuditDetail; import com.iflytek.skillhub.domain.audit.AuditLogService; import com.iflytek.skillhub.domain.label.LabelDefinition; import com.iflytek.skillhub.domain.label.LabelDefinitionService; @@ -62,7 +63,7 @@ public class LabelAdminAppService { userId, platformRoles(userId) ); - recordAudit("LABEL_CREATE", userId, labelDefinition.getId(), auditContext, "{\"slug\":\"" + labelDefinition.getSlug() + "\"}"); + recordAudit("LABEL_CREATE", userId, labelDefinition.getId(), auditContext, AuditDetail.of("slug", labelDefinition.getSlug())); return toResponse(labelDefinition); } @@ -87,7 +88,7 @@ public class LabelAdminAppService { if (!affectedSkillIds.isEmpty()) { afterCommit(() -> labelSearchSyncService.rebuildSkills(affectedSkillIds)); } - recordAudit("LABEL_UPDATE", userId, updated.getId(), auditContext, "{\"slug\":\"" + updated.getSlug() + "\"}"); + recordAudit("LABEL_UPDATE", userId, updated.getId(), auditContext, AuditDetail.of("slug", updated.getSlug())); return toResponse(updated); } @@ -102,7 +103,7 @@ public class LabelAdminAppService { if (!affectedSkillIds.isEmpty()) { afterCommit(() -> labelSearchSyncService.rebuildSkills(affectedSkillIds)); } - recordAudit("LABEL_DELETE", userId, existing.getId(), auditContext, "{\"slug\":\"" + slug + "\"}"); + recordAudit("LABEL_DELETE", userId, existing.getId(), auditContext, AuditDetail.of("slug", slug)); } @Transactional @@ -118,7 +119,7 @@ public class LabelAdminAppService { List responses = labelDefinitionService.updateSortOrders(updates, platformRoles(userId)).stream() .map(this::toResponse) .toList(); - recordAudit("LABEL_SORT_ORDER_UPDATE", userId, null, auditContext, "{\"count\":" + request.items().size() + "}"); + recordAudit("LABEL_SORT_ORDER_UPDATE", userId, null, auditContext, AuditDetail.of("count", request.items().size())); return responses; } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/PromotionPortalAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/PromotionPortalAppService.java index 1c7f6b44..be6fbb47 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/PromotionPortalAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/PromotionPortalAppService.java @@ -1,6 +1,7 @@ package com.iflytek.skillhub.service; import com.iflytek.skillhub.auth.rbac.RbacService; +import com.iflytek.skillhub.domain.audit.AuditDetail; import com.iflytek.skillhub.domain.audit.AuditLogService; import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.review.PromotionRequest; @@ -67,7 +68,7 @@ public class PromotionPortalAppService { userId, promotion.getId(), auditContext, - "{\"sourceSkillId\":" + sourceSkillId + ",\"sourceVersionId\":" + sourceVersionId + "}" + AuditDetail.of("sourceSkillId", sourceSkillId, "sourceVersionId", sourceVersionId) ); return governanceQueryRepository.getPromotionResponse(promotion); } @@ -247,24 +248,9 @@ public class PromotionPortalAppService { private String detailWithComment(String comment, boolean selfReview) { boolean hasComment = comment != null && !comment.isBlank(); - if (!hasComment && !selfReview) { - return null; - } - StringBuilder detail = new StringBuilder("{"); - if (hasComment) { - detail.append("\"comment\":\"").append(escapeJson(comment)).append("\""); - } - if (selfReview) { - if (hasComment) { - detail.append(","); - } - detail.append("\"selfReview\":true"); - } - detail.append("}"); - return detail.toString(); - } - - private String escapeJson(String value) { - return value.replace("\\", "\\\\").replace("\"", "\\\""); + return AuditDetail.builder() + .put("comment", hasComment ? comment : null) + .put("selfReview", selfReview ? Boolean.TRUE : null) + .build(); } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewPortalAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewPortalAppService.java index 96663c9e..b05b9b19 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewPortalAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewPortalAppService.java @@ -1,6 +1,7 @@ package com.iflytek.skillhub.service; import com.iflytek.skillhub.auth.rbac.RbacService; +import com.iflytek.skillhub.domain.audit.AuditDetail; import com.iflytek.skillhub.domain.audit.AuditLogService; import com.iflytek.skillhub.domain.namespace.Namespace; import com.iflytek.skillhub.domain.namespace.NamespaceRepository; @@ -62,7 +63,7 @@ public class ReviewPortalAppService { normalizeRoles(userNsRoles), platformRoles(userId) ); - recordAudit("REVIEW_SUBMIT", userId, task.getId(), auditContext, "{\"skillVersionId\":" + skillVersionId + "}"); + recordAudit("REVIEW_SUBMIT", userId, task.getId(), auditContext, AuditDetail.of("skillVersionId", skillVersionId)); return governanceQueryRepository.getReviewTaskResponse(task); } @@ -109,7 +110,7 @@ public class ReviewPortalAppService { userId, reviewTaskId, auditContext, - "{\"skillVersionId\":" + task.getSkillVersionId() + "}" + AuditDetail.of("skillVersionId", task.getSkillVersionId()) ); } @@ -270,6 +271,6 @@ public class ReviewPortalAppService { if (comment == null || comment.isBlank()) { return null; } - return "{\"comment\":\"" + comment.replace("\"", "\\\"") + "\"}"; + return AuditDetail.of("comment", comment); } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLabelAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLabelAppService.java index 5947934e..86a16948 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLabelAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLabelAppService.java @@ -1,6 +1,7 @@ package com.iflytek.skillhub.service; import com.iflytek.skillhub.auth.rbac.RbacService; +import com.iflytek.skillhub.domain.audit.AuditDetail; import com.iflytek.skillhub.domain.audit.AuditLogService; import com.iflytek.skillhub.domain.label.LabelDefinition; import com.iflytek.skillhub.domain.label.LabelDefinitionService; @@ -96,7 +97,7 @@ public class SkillLabelAppService { platformRoles(userId) ); afterCommit(() -> labelSearchSyncService.rebuildSkill(skill.getId())); - recordAudit("SKILL_LABEL_ATTACH", userId, skill.getId(), auditContext, "{\"labelSlug\":\"" + labelSlug + "\"}"); + recordAudit("SKILL_LABEL_ATTACH", userId, skill.getId(), auditContext, AuditDetail.of("labelSlug", labelSlug)); return toDtos(List.of(attached)).getFirst(); } @@ -116,7 +117,7 @@ public class SkillLabelAppService { platformRoles(userId) ); afterCommit(() -> labelSearchSyncService.rebuildSkill(skill.getId())); - recordAudit("SKILL_LABEL_DETACH", userId, skill.getId(), auditContext, "{\"labelSlug\":\"" + labelSlug + "\"}"); + recordAudit("SKILL_LABEL_DETACH", userId, skill.getId(), auditContext, AuditDetail.of("labelSlug", labelSlug)); return new MessageResponse("Label detached"); } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLifecycleAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLifecycleAppService.java index 3e2119d9..4aa57b8d 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLifecycleAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillLifecycleAppService.java @@ -1,5 +1,6 @@ package com.iflytek.skillhub.service; +import com.iflytek.skillhub.domain.audit.AuditDetail; import com.iflytek.skillhub.domain.audit.AuditLogService; import com.iflytek.skillhub.domain.namespace.Namespace; import com.iflytek.skillhub.domain.namespace.NamespaceRepository; @@ -128,7 +129,7 @@ public class SkillLifecycleAppService { null, auditContext.clientIp(), auditContext.userAgent(), - "{\"version\":\"" + version.replace("\"", "\\\"") + "\"}" + AuditDetail.of("version", version) ); return new SkillLifecycleMutationResponse( skill.getId(), @@ -165,8 +166,7 @@ public class SkillLifecycleAppService { null, auditContext.clientIp(), auditContext.userAgent(), - "{\"sourceVersion\":\"" + version.replace("\"", "\\\"") - + "\",\"targetVersion\":\"" + targetVersion.replace("\"", "\\\"") + "\"}" + AuditDetail.of("sourceVersion", version, "targetVersion", targetVersion) ); return new SkillLifecycleMutationResponse( result.skillId(), @@ -201,7 +201,7 @@ public class SkillLifecycleAppService { null, auditContext.clientIp(), auditContext.userAgent(), - "{\"version\":\"" + version.replace("\"", "\\\"") + "\",\"targetVisibility\":\"" + targetVisibility + "\"}" + AuditDetail.of("version", version, "targetVisibility", targetVisibility) ); return new SkillLifecycleMutationResponse( skill.getId(), @@ -234,7 +234,7 @@ public class SkillLifecycleAppService { null, auditContext.clientIp(), auditContext.userAgent(), - "{\"version\":\"" + version.replace("\"", "\\\"") + "\"}" + AuditDetail.of("version", version) ); return new SkillLifecycleMutationResponse( skill.getId(), diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/PromotionPortalAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/PromotionPortalAppServiceTest.java index 9d8a5acc..b19ce879 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/PromotionPortalAppServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/PromotionPortalAppServiceTest.java @@ -134,6 +134,37 @@ class PromotionPortalAppServiceTest { ); } + @Test + void approvePromotion_escapesMultiLineReviewCommentIntoValidJson() { + // Regression: the detail column is JSONB. A reviewer pressing Enter used + // to produce a raw newline inside the JSON string, so the audit insert + // failed after the promotion had already been approved. + String comment = "looks good\nbut rename it \"foo\"\tfirst\\done"; + PromotionRequest promotion = promotionRequest(PROMOTION_ID, SUBMITTER_ID); + when(rbacService.getUserRoleCodes(REVIEWER_ID)).thenReturn(Set.of("SKILL_ADMIN")); + when(promotionService.approvePromotion(PROMOTION_ID, REVIEWER_ID, comment, Set.of("SKILL_ADMIN"))) + .thenReturn(promotion); + when(governanceQueryRepository.getPromotionResponse(promotion)).thenReturn(response(promotion)); + + service.approvePromotion( + PROMOTION_ID, + comment, + REVIEWER_ID, + new AuditRequestContext("127.0.0.1", "JUnit") + ); + + verify(auditLogService).record( + eq(REVIEWER_ID), + eq("PROMOTION_APPROVE"), + eq("PROMOTION_REQUEST"), + eq(PROMOTION_ID), + eq(null), + eq("127.0.0.1"), + eq("JUnit"), + eq("{\"comment\":\"looks good\\nbut rename it \\\"foo\\\"\\tfirst\\\\done\"}") + ); + } + private PromotionResponseDto response(PromotionRequest request) { return new PromotionResponseDto( request.getId(), diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/audit/AuditDetail.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/audit/AuditDetail.java new file mode 100644 index 00000000..69c6dac1 --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/audit/AuditDetail.java @@ -0,0 +1,86 @@ +package com.iflytek.skillhub.domain.audit; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; + +import java.util.LinkedHashMap; +import java.util.Map; + +/** + * Builds the JSON stored in {@code audit_log.detail_json}. + * + *

That column is PostgreSQL {@code JSONB}, so the value has to be valid JSON or the + * insert fails. Hand-concatenating it does not survive real input: a value containing a + * newline, tab, backslash, or any other control character produces a string PostgreSQL + * rejects, and the audit write then fails after the domain mutation has already + * committed. Escaping only {@code "} — or only {@code "} and {@code \} — is not enough. + * + *

Every audit detail payload should be built here so there is exactly one place where + * that escaping is decided. + * + *

The mapper is a private static instance rather than the injected application bean on + * purpose: audit records are a stored format, and they should not change shape because + * someone reconfigures Jackson elsewhere in the application. + */ +public final class AuditDetail { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + private AuditDetail() { + } + + /** + * Renders a single-field detail payload, e.g. {@code {"slug":"my-skill"}}. + */ + public static String of(String key, Object value) { + return builder().put(key, value).build(); + } + + /** + * Renders a two-field detail payload, preserving argument order. + */ + public static String of(String key1, Object value1, String key2, Object value2) { + return builder().put(key1, value1).put(key2, value2).build(); + } + + public static Builder builder() { + return new Builder(); + } + + /** + * Accumulates fields in insertion order. Use it when a field is conditional. + */ + public static final class Builder { + + private final Map fields = new LinkedHashMap<>(); + + private Builder() { + } + + /** + * Adds a field. A {@code null} value is skipped, so an optional field can be + * offered unconditionally. + */ + public Builder put(String key, Object value) { + if (value != null) { + fields.put(key, value); + } + return this; + } + + /** + * Returns the rendered JSON, or {@code null} when no field was set — the audit + * log stores {@code null} rather than an empty object for "no detail". + */ + public String build() { + if (fields.isEmpty()) { + return null; + } + try { + return MAPPER.writeValueAsString(fields); + } catch (JsonProcessingException e) { + throw new IllegalStateException("Failed to serialize audit detail JSON", e); + } + } + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/namespace/NamespaceGovernanceService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/namespace/NamespaceGovernanceService.java index 4809a264..984fe4fc 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/namespace/NamespaceGovernanceService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/namespace/NamespaceGovernanceService.java @@ -1,5 +1,6 @@ package com.iflytek.skillhub.domain.namespace; +import com.iflytek.skillhub.domain.audit.AuditDetail; import com.iflytek.skillhub.domain.audit.AuditLogService; import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; @@ -206,7 +207,7 @@ public class NamespaceGovernanceService { requestId, clientIp, userAgent, - reason == null || reason.isBlank() ? null : "{\"reason\":\"" + reason.replace("\"", "\\\"") + "\"}" + reason == null || reason.isBlank() ? null : AuditDetail.of("reason", reason) ); } } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/report/SkillReportService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/report/SkillReportService.java index 3a9b82b1..a94d43c7 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/report/SkillReportService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/report/SkillReportService.java @@ -1,5 +1,6 @@ package com.iflytek.skillhub.domain.report; +import com.iflytek.skillhub.domain.audit.AuditDetail; import com.iflytek.skillhub.domain.audit.AuditLogService; import com.iflytek.skillhub.domain.event.ReportResolvedEvent; import com.iflytek.skillhub.domain.event.ReportSubmittedEvent; @@ -78,7 +79,7 @@ public class SkillReportService { normalize(details) )); auditLogService.record(reporterId, "REPORT_SKILL", "SKILL", skillId, null, clientIp, userAgent, - "{\"reportId\":" + saved.getId() + "}"); + AuditDetail.of("reportId", saved.getId())); eventPublisher.publishEvent(new ReportSubmittedEvent( saved.getId(), saved.getSkillId(), saved.getReporterId())); return saved; @@ -120,7 +121,7 @@ public class SkillReportService { "SKILL_REPORT", reportId, "Report handled", - "{\"status\":\"RESOLVED\"}" + AuditDetail.of("status", "RESOLVED") ); return saved; } @@ -146,7 +147,7 @@ public class SkillReportService { "SKILL_REPORT", reportId, "Report dismissed", - "{\"status\":\"DISMISSED\"}" + AuditDetail.of("status", "DISMISSED") ); return saved; } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java index 1a1429d0..97751186 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionService.java @@ -1,5 +1,6 @@ package com.iflytek.skillhub.domain.review; +import com.iflytek.skillhub.domain.audit.AuditDetail; import com.iflytek.skillhub.domain.event.PromotionApprovedEvent; import com.iflytek.skillhub.domain.event.PromotionRejectedEvent; import com.iflytek.skillhub.domain.event.PromotionSubmittedEvent; @@ -266,7 +267,7 @@ public class PromotionService { "PROMOTION_REQUEST", promotionId, "Promotion approved", - "{\"status\":\"APPROVED\"}" + AuditDetail.of("status", "APPROVED") ); return savedRequest; @@ -323,7 +324,7 @@ public class PromotionService { "PROMOTION_REQUEST", promotionId, "Promotion rejected", - "{\"status\":\"REJECTED\"}" + AuditDetail.of("status", "REJECTED") ); return request; diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java index 56ea7884..18275f3e 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewService.java @@ -1,6 +1,7 @@ package com.iflytek.skillhub.domain.review; import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.domain.audit.AuditDetail; import com.iflytek.skillhub.domain.namespace.Namespace; import com.iflytek.skillhub.domain.namespace.NamespaceRepository; import com.iflytek.skillhub.domain.namespace.NamespaceRole; @@ -241,7 +242,7 @@ public class ReviewService { "REVIEW_TASK", reviewTaskId, "Review approved", - "{\"status\":\"APPROVED\"}" + AuditDetail.of("status", "APPROVED") ); return task; @@ -294,7 +295,7 @@ public class ReviewService { "REVIEW_TASK", reviewTaskId, "Review rejected", - "{\"status\":\"REJECTED\"}" + AuditDetail.of("status", "REJECTED") ); return task; diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java index 552d9e1f..e4bfc81c 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java @@ -1,5 +1,6 @@ package com.iflytek.skillhub.domain.skill.service; +import com.iflytek.skillhub.domain.audit.AuditDetail; import com.iflytek.skillhub.domain.audit.AuditLogService; import com.iflytek.skillhub.domain.event.SkillStatusChangedEvent; import com.iflytek.skillhub.domain.namespace.NamespaceRole; @@ -205,7 +206,7 @@ public class SkillGovernanceService { null, clientIp, userAgent, - "{\"version\":\"" + version.getVersion().replace("\"", "\\\"") + "\"}" + AuditDetail.of("version", version.getVersion()) ); } @@ -311,7 +312,7 @@ public class SkillGovernanceService { if (reason == null || reason.isBlank()) { return null; } - return "{\"reason\":\"" + reason.replace("\"", "\\\"") + "\"}"; + return AuditDetail.of("reason", reason); } private Instant currentInstant() { diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/audit/AuditDetailTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/audit/AuditDetailTest.java new file mode 100644 index 00000000..950de0c6 --- /dev/null +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/audit/AuditDetailTest.java @@ -0,0 +1,147 @@ +package com.iflytek.skillhub.domain.audit; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; + +class AuditDetailTest { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + // ------------------------------------------------------------------ + // Shape — the rendered JSON must match what callers wrote by hand + // ------------------------------------------------------------------ + + @Test + void of_rendersSingleField() { + assertThat(AuditDetail.of("slug", "my-skill")).isEqualTo("{\"slug\":\"my-skill\"}"); + } + + @Test + void of_preservesArgumentOrderForTwoFields() { + assertThat(AuditDetail.of("version", "1.0.0", "targetVisibility", "PUBLIC")) + .isEqualTo("{\"version\":\"1.0.0\",\"targetVisibility\":\"PUBLIC\"}"); + } + + @Test + void of_keepsNumbersAndBooleansUnquoted() { + assertThat(AuditDetail.of("count", 3)).isEqualTo("{\"count\":3}"); + assertThat(AuditDetail.of("reportId", 42L)).isEqualTo("{\"reportId\":42}"); + assertThat(AuditDetail.of("selfReview", Boolean.TRUE)).isEqualTo("{\"selfReview\":true}"); + } + + @Test + void builder_preservesInsertionOrder() { + assertThat(AuditDetail.builder() + .put("comment", "ship") + .put("selfReview", Boolean.TRUE) + .build()) + .isEqualTo("{\"comment\":\"ship\",\"selfReview\":true}"); + } + + // ------------------------------------------------------------------ + // Escaping — the regression this class exists for + // ------------------------------------------------------------------ + + /** + * Every one of these breaks a hand-rolled builder. The audit column is JSONB, + * so an unescaped control character makes the insert fail after the domain + * mutation has already committed. + */ + @ParameterizedTest + @ValueSource(strings = { + "line one\nline two", + "carriage\rreturn", + "tab\tseparated", + "back\\slash", + "quote \" inside", + "both \\ and \"", + "form\ffeed", + "backspace\bhere", + "中文评审意见\n第二行", + "emoji 🚀 and \"quotes\"", + }) + void escapedValuesRoundTripAsValidJson(String raw) throws Exception { + String json = AuditDetail.of("comment", raw); + + JsonNode parsed = MAPPER.readTree(json); + assertThat(parsed.get("comment").asText()).isEqualTo(raw); + } + + /** + * Built at runtime rather than in the annotation: a raw control character in + * a source literal is fragile, and these are exactly the bytes JSON forbids + * unescaped inside a string. + */ + @Test + void lowControlCharactersRoundTripAsValidJson() throws Exception { + for (char c = 0; c < 0x20; c++) { + String raw = "before" + c + "after"; + String json = AuditDetail.of("comment", raw); + + JsonNode parsed = MAPPER.readTree(json); + assertThat(parsed.get("comment").asText()) + .as("control character U+%04X", (int) c) + .isEqualTo(raw); + } + } + + @Test + void newlineIsEscapedRatherThanEmbeddedRaw() { + // The literal two-character sequence backslash-n, not a raw 0x0A. + assertThat(AuditDetail.of("comment", "a\nb")).isEqualTo("{\"comment\":\"a\\nb\"}"); + } + + @Test + void multiFieldPayloadWithControlCharactersStaysParseable() throws Exception { + String json = AuditDetail.of("sourceVersion", "1.0.0\n", "targetVersion", "2.0.0\t\"x\""); + + JsonNode parsed = MAPPER.readTree(json); + assertThat(parsed.get("sourceVersion").asText()).isEqualTo("1.0.0\n"); + assertThat(parsed.get("targetVersion").asText()).isEqualTo("2.0.0\t\"x\""); + } + + @Test + void everyAwkwardCharacterAtOnceStillSerializes() throws Exception { + // Assembled from char casts: a \\uXXXX escape is expanded by the Java + // lexer before parsing, which would put the raw byte back in the literal. + String awkward = "" + (char) 0x00 + (char) 0x1F + "\\\"" + "\n\r\t" + "中文"; + + assertThatCode(() -> AuditDetail.of("reason", awkward)).doesNotThrowAnyException(); + assertThat(MAPPER.readTree(AuditDetail.of("reason", awkward)).get("reason").asText()) + .isEqualTo(awkward); + } + + // ------------------------------------------------------------------ + // Null handling — "no detail" must stay null, not become "{}" + // ------------------------------------------------------------------ + + @Test + void nullValueIsSkipped() { + assertThat(AuditDetail.of("comment", null, "selfReview", Boolean.TRUE)) + .isEqualTo("{\"selfReview\":true}"); + } + + @Test + void allNullValuesProduceNull() { + assertThat(AuditDetail.of("comment", null)).isNull(); + assertThat(AuditDetail.of("comment", null, "selfReview", null)).isNull(); + } + + @Test + void emptyBuilderProducesNull() { + assertThat(AuditDetail.builder().build()).isNull(); + } + + @Test + void emptyStringIsStillARecordedValue() { + // Only null is treated as absent; callers that want to drop blanks keep + // their own isBlank() guard. + assertThat(AuditDetail.of("comment", "")).isEqualTo("{\"comment\":\"\"}"); + } +}