fix:update model data and API key authentication support#315
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds API-key authentication for ChangesBackend infrastructure and schema bootstrap
API-key authentication
Dynamic model-data services
Tests
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Note Unit test generation is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
❌ Failed to create PR with unit tests: AGENT_CHAT: Failed to open pull request |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java (1)
165-175: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject or enforce extra update conditions.
The method still accepts a conditions map, but after requiring
id, it ignores every other condition and updates by ID only. A request like{id: 1, status: "draft"}will update row1even ifstatusdoes not match.Also applies to: 193-193
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java` around lines 165 - 175, The update flow in DynamicService still validates dto.getParams() but then only uses the id for the update, so any extra conditions are ignored. Update the logic in the relevant update method(s) to either reject additional conditions beyond id or include all provided conditions when building the update criteria, and make sure the params/conditions passed to the mapper reflect the chosen behavior.
🟠 Major comments (19)
base/src/main/java/com/tinyengine/it/dynamic/controller/ModelDataController.java-71-71 (1)
71-71: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not expose raw exception messages in API responses.
These handlers catch broad
Exception, andResult.failed(String)returns the message verbatim to clients. That leaks internal validation, SQL, Redis, or persistence error details from the new dynamic-data stack. Keep the detailed exception in logs and only surface a sanitized message, or whitelist known business exceptions.🔒 Suggested fix
} catch (Exception e) { log.error("insert failed for table: {}", dto.getNameEn(), e); - return Result.failed("insert operation failed:"+e.getMessage()); + return Result.failed("insert operation failed"); } @@ } catch (Exception e) { log.error("updateApi failed for table: {}", dto.getNameEn(), e); - return Result.failed("update operation failed:"+e.getMessage()); + return Result.failed("update operation failed"); } @@ } catch (Exception e) { log.error("deleteApi failed for table: {}", dto.getNameEn(), e); - return Result.failed("delete operation failed:"+e.getMessage()); + return Result.failed("delete operation failed"); }Also applies to: 93-93, 114-114
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/dynamic/controller/ModelDataController.java` at line 71, The controller methods in ModelDataController are returning raw Exception text through Result.failed(...), which leaks internal details to clients. Update the catch blocks around the insert/update/delete handlers to log the full exception for debugging, but return a sanitized, generic failure message from Result.failed; if needed, only pass through whitelisted business exception messages. Keep the fix localized to the affected controller methods rather than changing the Result API.base/src/main/java/com/tinyengine/it/service/material/impl/ResourceServiceImpl.java-203-205 (1)
203-205: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winUse the authenticated tenant here, not the request payload.
resource.getTenantId()is caller-controlled, but this service already hasLoginUserContext, andDefaultLoginUserContext.getTenantId()is the repo’s tenant contract. If the request omits/spoofstenantId, this duplicate check runs against the wrong tenant and can either allow duplicate uploads or let callers probe another tenant’s hashes. Resolve the tenant fromloginUserContextand stamp it ontoresourcebefore both the lookup and insert.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/service/material/impl/ResourceServiceImpl.java` around lines 203 - 205, The duplicate-check in ResourceServiceImpl is using caller-controlled tenantId from resource.getTenantId(), which can target the wrong tenant. In the ResourceServiceImpl flow, resolve the tenant from LoginUserContext via DefaultLoginUserContext.getTenantId(), stamp that value onto resource before the QueryWrapper lookup and before insert, and use the authenticated tenant consistently for the hash duplicate check.base/src/main/java/com/tinyengine/it/modeldata/dao/ApiKeyRepository.java-9-10 (1)
9-10: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAlign
ApiKeyRepositorywithApiKeyEntity.id
ApiKeyEntity.idisInteger, but this repository declaresLong, so the inheritedJpaRepositorymethods expose the wrong ID type. Change it toJpaRepository<ApiKeyEntity, Integer>to keepfindById/deleteByIdconsistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/modeldata/dao/ApiKeyRepository.java` around lines 9 - 10, `ApiKeyRepository` is using the wrong primary key type, which makes inherited `JpaRepository` methods inconsistent with `ApiKeyEntity.id`. Update the `ApiKeyRepository` declaration to use the same ID type as `ApiKeyEntity` by changing the `JpaRepository` generic from `Long` to `Integer`, while keeping `findByApiKeyAndStatus` unchanged.base/src/main/java/com/tinyengine/it/modeldata/filter/ApiKeyAuthFilter.java-74-80 (1)
74-80: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReserve the nonce only after the signature is valid, and scope it to the API key.
nonce:<nonce>is written before authentication succeeds and is shared by every client. That lets an unauthenticated caller pre-burn arbitrary nonces, and two different API keys can also block each other if they reuse the same nonce value. Build the replay key from the authenticated principal (for examplenonce:{apiKey}:{nonce}) and persist it only after the HMAC check passes.Proposed fix
- // 4. Nonce 防重放（Redis 存储，有效期5分钟） - String nonceKey = "nonce:" + nonce; - Boolean isNew = stringRedisTemplate.opsForValue().setIfAbsent(nonceKey, "1", Duration.ofMinutes(5)); - if (Boolean.FALSE.equals(isNew)) { - response.setStatus(401); - response.getWriter().write("Nonce already used"); - return; - } - - // 5. 根据 API Key 获取密钥和租户信息 + // 4. 根据 API Key 获取密钥和租户信息 ApiKeyEntity keyEntity = apiKeyService.getValidKey(apiKey); if (keyEntity == null) { response.setStatus(401); response.getWriter().write("Invalid API Key"); return; @@ String expectedSignature = SignatureUtil.hmacSha256(keyEntity.getApiSecret(), stringToSign); if (!expectedSignature.equals(signature)) { response.setStatus(401); response.getWriter().write("Invalid signature"); return; } + + String nonceKey = "nonce:" + apiKey + ":" + nonce; + Boolean isNew = stringRedisTemplate.opsForValue().setIfAbsent(nonceKey, "1", Duration.ofMinutes(5)); + if (!Boolean.TRUE.equals(isNew)) { + response.setStatus(401); + response.getWriter().write("Nonce already used"); + return; + }Also applies to: 99-105
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/modeldata/filter/ApiKeyAuthFilter.java` around lines 74 - 80, The nonce replay check in ApiKeyAuthFilter is happening too early and uses a global nonce key, so move the Redis reservation until after the HMAC/signature validation succeeds and tie it to the authenticated API key. Update the nonce handling in the filter around the nonce validation block to build the Redis key from the principal, such as nonce:{apiKey}:{nonce}, and only call setIfAbsent after the signature check passes. Apply the same change to the duplicate nonce logic later in the method as well.base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java-69-69 (1)
69-69: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAvoid logging dynamic payloads and user identifiers.
These logs include arbitrary model params/data and
userId. Dynamic model data can contain PII; log IDs, field names, and counts instead of raw values.Also applies to: 144-145, 180-180, 191-191, 214-214
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java` at line 69, Remove logging of raw dynamic payloads and user identifiers from DynamicService, especially the log statements around query/init/update/delete flows. Replace log entries in methods such as the query path and the other flagged call sites with safe metadata only, using identifiers like modelId, tenantId, userId, field names, and counts instead of params/data contents or raw IDs. Keep the existing log context in DynamicService but sanitize any arbitrary model params and PII before logging.base/src/main/java/com/tinyengine/it/modeldata/service/ModelDataCacheService.java-83-116 (1)
83-116: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject unsupported filter operators instead of matching everything.
Line 115 makes an unknown operator behave as “no filter”, so a typo like
gtee:10can return broader results than requested.Proposed fix
if (strValue.contains(":")) { String[] parts = strValue.split(":", 2); operator = parts[0]; value = parts[1]; }else{ @@ } } + if (!Set.of("eq", "ne", "gt", "lt", "gte", "lte", "like", "startswith", "endswith", "in").contains(operator)) { + throw new IllegalArgumentException("Unsupported filter operator: " + operator); + } final String op = operator;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/modeldata/service/ModelDataCacheService.java` around lines 83 - 116, The filter logic in ModelDataCacheService currently treats unknown operators as a pass-through in the switch inside the item filter, which can silently broaden results on typos like gtee. Update the operator handling so unsupported operators are rejected explicitly instead of falling through to a match-all path; for example, validate the parsed operator before applying the stream.filter and make the default case return a failure/empty result or throw an error. Use the existing operator parsing and comparison logic in this method to keep supported cases like eq, gt, like, in, and startswith unchanged.base/src/main/java/com/tinyengine/it/modeldata/dao/ModelDataRepository.java-13-13 (1)
13-13: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAlign the ModelData ID type across entity, repository, and callers.
ModelData.idisInteger, but the repository and service methods still useLong(findById(Long.valueOf(dataId)),deleteById(Long), etc.). Keep the ID type consistent so CRUD lookups don’t depend on mixed conversions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/modeldata/dao/ModelDataRepository.java` at line 13, The ModelData ID type is inconsistent across the entity, repository, and service callers, since ModelData.id uses Integer while ModelDataRepository and methods like findById/deleteById still rely on Long conversions. Update ModelDataRepository to use the same ID type as ModelData, then adjust all related service and caller methods that reference ModelDataRepository, findById, and deleteById so they pass and return the consistent ID type without mixed Long/Integer conversions.base/src/main/java/com/tinyengine/it/modeldata/service/DynamicModelDataService.java-116-120 (1)
116-120: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not splice request field names into the JSON path.
fieldis caller-controlled and is concatenated directly intoJSON_EXTRACT(data_json, '$."..."'). Prepared parameters only protectvalue; a crafted key can break the path literal and inject SQL. Validate filter keys against the model's allowed properties before building this clause.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/modeldata/service/DynamicModelDataService.java` around lines 116 - 120, The filter-building logic in DynamicModelDataService currently concatenates caller-controlled keys into the JSON_EXTRACT path, which can allow SQL injection through the field name. Update the loop that builds the WHERE clause to validate each entry key against the model’s allowed properties before appending it, and only then construct the JSON path in the query. Keep using prepared parameters for the value, but ensure the key used by JSON_EXTRACT is strictly whitelisted or rejected in the same method.base/src/main/java/com/tinyengine/it/modeldata/service/DynamicModelDataService.java-44-52 (1)
44-52: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle models with null
parameters.Both create and update call
isEmpty()onmodel.getParameters()before null-checking it. Other code in this PR already guards that property as nullable, so models without parameter metadata will crash here with an NPE instead of validating against an empty schema.Suggested change
- List<ParametersDto> fieldDefs = model.getParameters(); + List<ParametersDto> fieldDefs = model.getParameters(); + if (fieldDefs == null) { + fieldDefs = new ArrayList<>(); + } if (!fieldDefs.isEmpty() && !(fieldDefs.get(0) instanceof ParametersDto)) {Also applies to: 77-88
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/modeldata/service/DynamicModelDataService.java` around lines 44 - 52, The model parameter handling in DynamicModelDataService is missing a null guard: both the create and update flows call isEmpty() on model.getParameters() before checking whether parameters is null, which can throw an NPE for models without metadata. Update the parameter retrieval logic in the validation path used by the create/update methods so it first checks for null, treats missing parameters as an empty list, and only runs the JSON conversion and validationService.validate when a non-null list is present; use the existing model.getParameters(), fieldDefs, and validationService.validate flow to keep behavior consistent.base/src/main/java/com/tinyengine/it/modeldata/service/DynamicModelDataService.java-69-71 (1)
69-71: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftScope reads and mutations by
tenant_id.Create/query persist and filter on
tenantId, but update/get/delete load rows by primary key only. The downstream callers inDynamicServicealso pass onlyid, so anyone who can guess another tenant'sdataIdcan read, modify, or delete that row across tenant boundaries.Also applies to: 97-105
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/modeldata/service/DynamicModelDataService.java` around lines 69 - 71, The read/update/delete paths in DynamicModelDataService currently load ModelData by primary key only, which bypasses tenant scoping. Update the affected methods (including update and the related get/delete code referenced in the review) to query by both dataId and tenantId, using the tenant context already used by create/query, and make sure DynamicService passes tenantId through instead of only id so all mutations and reads stay tenant-isolated.base/src/main/java/com/tinyengine/it/modeldata/service/ValidationService.java-52-53 (1)
52-53: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRemove raw payload logging from validation.
This writes every validated value to stdout, which can leak tenant data/PII and sidestep the application's logging policy.
Suggested change
private void checkType(Object value, String expectedType, String fieldName) { - System.out.println("Validating field: " + fieldName + ", Expected type: " + expectedType + ", Actual value: " + value); switch (expectedType) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/modeldata/service/ValidationService.java` around lines 52 - 53, Remove the raw payload print from ValidationService.checkType, since logging the full validated value to stdout can expose tenant data and bypass the app’s logging controls. Update checkType so it only logs safe metadata like the field name or expected type, or remove the debug output entirely, and keep validation behavior unchanged.base/src/main/java/com/tinyengine/it/modeldata/service/ValidationService.java-16-19 (1)
16-19: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard nullable
requiredflags here.ParametersDto.requiredis nullable, andDynamicModelServiceaddsParametersDtoentries without setting it, sofilter(ParametersDto::getRequired)can NPE before validation runs. UseBoolean.TRUE.equals(...)instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/modeldata/service/ValidationService.java` around lines 16 - 19, The required-properties collection in ValidationService should not rely on a nullable Boolean from ParametersDto, since DynamicModelService can create entries without setting required and `filter(ParametersDto::getRequired)` may NPE. Update the stream filter in the validation logic to use a null-safe check like `Boolean.TRUE.equals(...)` on `ParametersDto.getRequired`, keeping the rest of the requiredProps mapping unchanged.app/src/main/resources/sql/mysql/11_create_modeldata_table_ddl_2026_0630.sql-1-1 (1)
1-1: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winThis migration will delete existing model data.
drop table if existsmakes the rollout non-idempotent and destroys all rows on every re-run. For a data table, that is too risky for normal deployment or recovery workflows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/resources/sql/mysql/11_create_modeldata_table_ddl_2026_0630.sql` at line 1, The migration currently drops t_model_data unconditionally, which wipes existing rows on every re-run. Update the SQL in the model data DDL migration to preserve existing data, using a safe schema-change approach in the same migration sequence instead of a DROP TABLE statement, and keep the change aligned with the table definition handled by this script.app/src/main/resources/sql/mysql/10_create_apikey_table_ddl_2026_0630.sql-6-12 (1)
6-12: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake
api_keythe unique identity.The auth flow looks up records by
api_key, but this schema only enforces uniqueness on(api_key, api_secret)and even allowsapi_secretto be null. That permits multiple active rows for the same presented key, making authentication ambiguous.api_keyshould be unique by itself, andapi_secretshould benot null.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/resources/sql/mysql/10_create_apikey_table_ddl_2026_0630.sql` around lines 6 - 12, The current DDL for the api key table allows duplicate api_key values because the uniqueness constraint is on the combined pair instead of the key alone, and api_secret is still nullable. Update the table definition in the MySQL create script so api_key is enforced as the unique identity via a unique index or constraint on api_key only, and make api_secret not null as part of the same schema change. Use the existing column definitions in the create-apikey DDL to locate the constraint and nullability settings.app/src/main/resources/sql/mysql/11_create_modeldata_table_ddl_2026_0630.sql-5-8 (1)
5-8: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
model_iddoes not match the entity type.The entity declares
modelIdasInteger, but this DDL createsmodel_idasvarchar(255). That breaks the schema/ORM contract and is exactly the kind of mismatch Hibernate will try to “fix” at startup whenddl-auto: updateis enabled. Make the column type align with the entity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/resources/sql/mysql/11_create_modeldata_table_ddl_2026_0630.sql` around lines 5 - 8, The DDL for the model data table has a type mismatch on the modelId mapping: the entity uses Integer, but the create script defines model_id as varchar(255). Update the column definition in the create table script so it matches the entity’s Integer type, and keep the rest of the table definition aligned with the ORM mapping to avoid Hibernate schema updates trying to alter it at startup.app/src/main/resources/sql/mysql/10_create_apikey_table_ddl_2026_0630.sql-1-1 (1)
1-1: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winThis migration is destructive.
Re-running this script will wipe every issued API key before recreating the table. That turns a normal redeploy or rollback into credential loss. Use additive/guarded DDL instead of
drop table if exists.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/resources/sql/mysql/10_create_apikey_table_ddl_2026_0630.sql` at line 1, The migration in the `t_api_key` DDL is destructive because it drops the existing table on rerun, causing API key loss. Update the SQL migration to use additive or guarded schema changes instead of `drop table if exists`, preserving existing data and making the `t_api_key` creation path safe for redeploys and rollbacks.pom.xml-201-206 (1)
201-206: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRemove the explicit Hibernate version override in
pom.xml:201-206. Spring Boot 3.2.4 manages Hibernate 6.4.4.Final; pinninghibernate-coreto 6.3.2.Final downgrades the BOM-managed stack and can desynchronize the JPA/Hibernate modules. Prefer the Boot-managed version unless you override the full Hibernate set.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pom.xml` around lines 201 - 206, The pom.xml Hibernate dependency is overriding Spring Boot’s managed version, which can desynchronize the JPA/Hibernate stack. Update the hibernate-core entry in the Maven dependency list to stop pinning 6.3.2.Final and let Spring Boot 3.2.4 manage the version, or if a custom Hibernate version is required, align the full Hibernate set consistently across the BOM-managed dependencies.app/src/main/resources/application-alpha.yml-32-35 (1)
32-35: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAvoid
ddl-auto: updatein this profile. The repo also includes explicit MySQL DDL fort_model_data, andModelData.modelIdis anIntegerwhile the table definesmodel_idasvarchar(255), so Hibernate can silently mutate the schema on startup. Usevalidateornonewherever those scripts are applied.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/resources/application-alpha.yml` around lines 32 - 35, The JPA configuration in the alpha profile is using hibernate ddl-auto update, which can silently change the schema and conflict with the explicit MySQL DDL for t_model_data. Update the alpha profile settings to use validate or none instead of update, and keep the existing ModelData/JPA mapping aligned with the scripted table definition, especially the ModelData.modelId field and the t_model_data model_id column.docker-compose.yml-37-47 (1)
37-47: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUnify the Redis password source
REDIS_PASSWORDhere is expanded by Compose before the container starts, so the service-level env value won’t feed--requirepass. The app password is also quoted (SPRING_DATA_REDIS_PASSWORD="000000"), which passes the quotes into the value. Use a single unquoted source of truth for both sides.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docker-compose.yml` around lines 37 - 47, The Redis service is mixing two password sources, and the Compose variable expansion in the command is not using the container-level env value; update the Redis service definition and the app Redis setting so both read from one unquoted source of truth. Use the existing Redis-related symbols in docker-compose.yml, especially REDIS_PASSWORD, SPRING_DATA_REDIS_PASSWORD, and the redis-server --requirepass command, and make sure the password value is not wrapped in quotes so Compose and the application receive the same raw string.
🟡 Minor comments (1)
base/src/main/java/com/tinyengine/it/modeldata/entity/ModelData.java-38-40 (1)
38-40: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRemove
@VersionfromModelData.versionModelData.versionis business data exposed as_version; keep it as a plain field and use a separate numeric/timestamp lock column if row-level optimistic locking is needed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/modeldata/entity/ModelData.java` around lines 38 - 40, Remove the JPA `@Version` annotation from ModelData.version and keep this field as plain business data used for the exposed _version value. If optimistic locking is still needed, introduce a separate dedicated lock field (for example a numeric or timestamp column) in ModelData and update the entity mapping accordingly, leaving version unchanged aside from its normal Schema/documentation usage.
🧹 Nitpick comments (2)
base/src/main/java/com/tinyengine/it/modeldata/service/ModelDataCacheService.java (1)
41-57: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftAvoid caching and scanning the full model dataset for every query.
This loads every row for a model/tenant into application memory and Redis, then filters/sorts/pages in memory. Large models can create oversized Redis values and O(N) request cost; prefer DB-side filtering/count/pagination or add strict cache-size limits.
Also applies to: 125-173
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/modeldata/service/ModelDataCacheService.java` around lines 41 - 57, The loadDataList method is caching and returning the full model dataset, which forces every query path to scan all records in memory and Redis. Refactor ModelDataCacheService so query-time filtering, sorting, and pagination happen in the database instead of materializing all rows in loadDataList, and only cache narrowly scoped or bounded results if needed. Update the related list/query flow that uses loadDataList to accept page/filter parameters and avoid full-dataset Redis values for large models.base/src/main/java/com/tinyengine/it/config/RedisConfig.java (1)
33-45: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAlign the value serializer with the cache payload type.
ModelDataCacheServicecaches JSON strings, but this template still uses the default value serializer. That writes Java-serialized bytes instead of plain JSON/text and makes the cache format JVM-specific. Either switch this path toStringRedisTemplateor setvalueSerializer/hashValueSerializerexplicitly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/config/RedisConfig.java` around lines 33 - 45, The Redis template configuration in RedisConfig.redisTemplate is leaving the value serializer at the default, while ModelDataCacheService stores JSON strings; update this setup so cached values are written as plain text/JSON rather than Java-serialized bytes. Either switch the cache path to StringRedisTemplate or explicitly configure template.setValueSerializer and template.setHashValueSerializer to match the payload type, keeping the existing StringRedisSerializer usage for keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/resources/sql/mysql/init_data_2026_0630.sql`:
- Line 1: Remove the hardcoded API credential from the MySQL seed data and
replace it with a non-sensitive placeholder or omit the row entirely. Update the
initialization data in init_data_2026_0630.sql so t_api_key is not populated
with a real api_key/api_secret pair, and move any environment-specific
provisioning into a separate secret-management flow outside source control.
In `@base/src/main/java/com/tinyengine/it/common/exception/ExceptionEnum.java`:
- Around line 355-359: The tail of ExceptionEnum is malformed because the last
enum constant CM346 is followed by an extra comma before the semicolon, which
prevents compilation. Update the enum declaration so the final constant in
ExceptionEnum ends normally with the closing semicolon only, keeping the
surrounding CM345 and CM346 definitions unchanged.
In `@base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java`:
- Around line 140-146: The dynamic payload is being polluted with audit/tenant
fields before validation, which causes valid requests to fail in
ModelDataCacheService.add/update. Remove the injected created_by, tenantId, and
updated_by entries from the JSON params in DynamicService and pass those values
only through the existing service parameters / ModelData entity fields; update
both the add and update paths so dto.getParams() contains only model-defined
data while modelDataCacheService.add/update still receives userId and tenantId
separately.
- Around line 60-70: The lookup in DynamicService is selecting the first
getModelByEnName(dto.getNameEn()) result, which can resolve to the wrong tenant
and user when the same nameEn exists in multiple tenants. Update the caller flow
in DynamicService to resolve the model using the current caller tenant/context
instead of modelList.get(0), and use that resolved model’s tenantId, id, and
creator/user fields consistently in the query and update paths. Apply the same
tenant-scoped resolution in the other affected DynamicService blocks mentioned
by the reviewer so reads and writes cannot cross tenant boundaries.
In
`@base/src/main/java/com/tinyengine/it/modeldata/service/ModelDataCacheService.java`:
- Around line 207-222: The update/delete paths in ModelDataCacheService are
scoped only by row ID, so they can operate outside the intended model and tenant
boundary. Change the ModelDataCacheService update and delete flows to verify
both modelId and tenantId from the request/context before mutating, and use
repository lookups like findByIdAndModelIdAndTenantId in delete as well as the
update path via dynamicModelDataService so only records in the requested scope
are modified.
---
Outside diff comments:
In `@base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java`:
- Around line 165-175: The update flow in DynamicService still validates
dto.getParams() but then only uses the id for the update, so any extra
conditions are ignored. Update the logic in the relevant update method(s) to
either reject additional conditions beyond id or include all provided conditions
when building the update criteria, and make sure the params/conditions passed to
the mapper reflect the chosen behavior.
---
Major comments:
In `@app/src/main/resources/application-alpha.yml`:
- Around line 32-35: The JPA configuration in the alpha profile is using
hibernate ddl-auto update, which can silently change the schema and conflict
with the explicit MySQL DDL for t_model_data. Update the alpha profile settings
to use validate or none instead of update, and keep the existing ModelData/JPA
mapping aligned with the scripted table definition, especially the
ModelData.modelId field and the t_model_data model_id column.
In `@app/src/main/resources/sql/mysql/10_create_apikey_table_ddl_2026_0630.sql`:
- Around line 6-12: The current DDL for the api key table allows duplicate
api_key values because the uniqueness constraint is on the combined pair instead
of the key alone, and api_secret is still nullable. Update the table definition
in the MySQL create script so api_key is enforced as the unique identity via a
unique index or constraint on api_key only, and make api_secret not null as part
of the same schema change. Use the existing column definitions in the
create-apikey DDL to locate the constraint and nullability settings.
- Line 1: The migration in the `t_api_key` DDL is destructive because it drops
the existing table on rerun, causing API key loss. Update the SQL migration to
use additive or guarded schema changes instead of `drop table if exists`,
preserving existing data and making the `t_api_key` creation path safe for
redeploys and rollbacks.
In
`@app/src/main/resources/sql/mysql/11_create_modeldata_table_ddl_2026_0630.sql`:
- Line 1: The migration currently drops t_model_data unconditionally, which
wipes existing rows on every re-run. Update the SQL in the model data DDL
migration to preserve existing data, using a safe schema-change approach in the
same migration sequence instead of a DROP TABLE statement, and keep the change
aligned with the table definition handled by this script.
- Around line 5-8: The DDL for the model data table has a type mismatch on the
modelId mapping: the entity uses Integer, but the create script defines model_id
as varchar(255). Update the column definition in the create table script so it
matches the entity’s Integer type, and keep the rest of the table definition
aligned with the ORM mapping to avoid Hibernate schema updates trying to alter
it at startup.
In
`@base/src/main/java/com/tinyengine/it/dynamic/controller/ModelDataController.java`:
- Line 71: The controller methods in ModelDataController are returning raw
Exception text through Result.failed(...), which leaks internal details to
clients. Update the catch blocks around the insert/update/delete handlers to log
the full exception for debugging, but return a sanitized, generic failure
message from Result.failed; if needed, only pass through whitelisted business
exception messages. Keep the fix localized to the affected controller methods
rather than changing the Result API.
In `@base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java`:
- Line 69: Remove logging of raw dynamic payloads and user identifiers from
DynamicService, especially the log statements around query/init/update/delete
flows. Replace log entries in methods such as the query path and the other
flagged call sites with safe metadata only, using identifiers like modelId,
tenantId, userId, field names, and counts instead of params/data contents or raw
IDs. Keep the existing log context in DynamicService but sanitize any arbitrary
model params and PII before logging.
In `@base/src/main/java/com/tinyengine/it/modeldata/dao/ApiKeyRepository.java`:
- Around line 9-10: `ApiKeyRepository` is using the wrong primary key type,
which makes inherited `JpaRepository` methods inconsistent with
`ApiKeyEntity.id`. Update the `ApiKeyRepository` declaration to use the same ID
type as `ApiKeyEntity` by changing the `JpaRepository` generic from `Long` to
`Integer`, while keeping `findByApiKeyAndStatus` unchanged.
In `@base/src/main/java/com/tinyengine/it/modeldata/dao/ModelDataRepository.java`:
- Line 13: The ModelData ID type is inconsistent across the entity, repository,
and service callers, since ModelData.id uses Integer while ModelDataRepository
and methods like findById/deleteById still rely on Long conversions. Update
ModelDataRepository to use the same ID type as ModelData, then adjust all
related service and caller methods that reference ModelDataRepository, findById,
and deleteById so they pass and return the consistent ID type without mixed
Long/Integer conversions.
In `@base/src/main/java/com/tinyengine/it/modeldata/filter/ApiKeyAuthFilter.java`:
- Around line 74-80: The nonce replay check in ApiKeyAuthFilter is happening too
early and uses a global nonce key, so move the Redis reservation until after the
HMAC/signature validation succeeds and tie it to the authenticated API key.
Update the nonce handling in the filter around the nonce validation block to
build the Redis key from the principal, such as nonce:{apiKey}:{nonce}, and only
call setIfAbsent after the signature check passes. Apply the same change to the
duplicate nonce logic later in the method as well.
In
`@base/src/main/java/com/tinyengine/it/modeldata/service/DynamicModelDataService.java`:
- Around line 116-120: The filter-building logic in DynamicModelDataService
currently concatenates caller-controlled keys into the JSON_EXTRACT path, which
can allow SQL injection through the field name. Update the loop that builds the
WHERE clause to validate each entry key against the model’s allowed properties
before appending it, and only then construct the JSON path in the query. Keep
using prepared parameters for the value, but ensure the key used by JSON_EXTRACT
is strictly whitelisted or rejected in the same method.
- Around line 44-52: The model parameter handling in DynamicModelDataService is
missing a null guard: both the create and update flows call isEmpty() on
model.getParameters() before checking whether parameters is null, which can
throw an NPE for models without metadata. Update the parameter retrieval logic
in the validation path used by the create/update methods so it first checks for
null, treats missing parameters as an empty list, and only runs the JSON
conversion and validationService.validate when a non-null list is present; use
the existing model.getParameters(), fieldDefs, and validationService.validate
flow to keep behavior consistent.
- Around line 69-71: The read/update/delete paths in DynamicModelDataService
currently load ModelData by primary key only, which bypasses tenant scoping.
Update the affected methods (including update and the related get/delete code
referenced in the review) to query by both dataId and tenantId, using the tenant
context already used by create/query, and make sure DynamicService passes
tenantId through instead of only id so all mutations and reads stay
tenant-isolated.
In
`@base/src/main/java/com/tinyengine/it/modeldata/service/ModelDataCacheService.java`:
- Around line 83-116: The filter logic in ModelDataCacheService currently treats
unknown operators as a pass-through in the switch inside the item filter, which
can silently broaden results on typos like gtee. Update the operator handling so
unsupported operators are rejected explicitly instead of falling through to a
match-all path; for example, validate the parsed operator before applying the
stream.filter and make the default case return a failure/empty result or throw
an error. Use the existing operator parsing and comparison logic in this method
to keep supported cases like eq, gt, like, in, and startswith unchanged.
In
`@base/src/main/java/com/tinyengine/it/modeldata/service/ValidationService.java`:
- Around line 52-53: Remove the raw payload print from
ValidationService.checkType, since logging the full validated value to stdout
can expose tenant data and bypass the app’s logging controls. Update checkType
so it only logs safe metadata like the field name or expected type, or remove
the debug output entirely, and keep validation behavior unchanged.
- Around line 16-19: The required-properties collection in ValidationService
should not rely on a nullable Boolean from ParametersDto, since
DynamicModelService can create entries without setting required and
`filter(ParametersDto::getRequired)` may NPE. Update the stream filter in the
validation logic to use a null-safe check like `Boolean.TRUE.equals(...)` on
`ParametersDto.getRequired`, keeping the rest of the requiredProps mapping
unchanged.
In
`@base/src/main/java/com/tinyengine/it/service/material/impl/ResourceServiceImpl.java`:
- Around line 203-205: The duplicate-check in ResourceServiceImpl is using
caller-controlled tenantId from resource.getTenantId(), which can target the
wrong tenant. In the ResourceServiceImpl flow, resolve the tenant from
LoginUserContext via DefaultLoginUserContext.getTenantId(), stamp that value
onto resource before the QueryWrapper lookup and before insert, and use the
authenticated tenant consistently for the hash duplicate check.
In `@docker-compose.yml`:
- Around line 37-47: The Redis service is mixing two password sources, and the
Compose variable expansion in the command is not using the container-level env
value; update the Redis service definition and the app Redis setting so both
read from one unquoted source of truth. Use the existing Redis-related symbols
in docker-compose.yml, especially REDIS_PASSWORD, SPRING_DATA_REDIS_PASSWORD,
and the redis-server --requirepass command, and make sure the password value is
not wrapped in quotes so Compose and the application receive the same raw
string.
In `@pom.xml`:
- Around line 201-206: The pom.xml Hibernate dependency is overriding Spring
Boot’s managed version, which can desynchronize the JPA/Hibernate stack. Update
the hibernate-core entry in the Maven dependency list to stop pinning
6.3.2.Final and let Spring Boot 3.2.4 manage the version, or if a custom
Hibernate version is required, align the full Hibernate set consistently across
the BOM-managed dependencies.
---
Minor comments:
In `@base/src/main/java/com/tinyengine/it/modeldata/entity/ModelData.java`:
- Around line 38-40: Remove the JPA `@Version` annotation from ModelData.version
and keep this field as plain business data used for the exposed _version value.
If optimistic locking is still needed, introduce a separate dedicated lock field
(for example a numeric or timestamp column) in ModelData and update the entity
mapping accordingly, leaving version unchanged aside from its normal
Schema/documentation usage.
---
Nitpick comments:
In `@base/src/main/java/com/tinyengine/it/config/RedisConfig.java`:
- Around line 33-45: The Redis template configuration in
RedisConfig.redisTemplate is leaving the value serializer at the default, while
ModelDataCacheService stores JSON strings; update this setup so cached values
are written as plain text/JSON rather than Java-serialized bytes. Either switch
the cache path to StringRedisTemplate or explicitly configure
template.setValueSerializer and template.setHashValueSerializer to match the
payload type, keeping the existing StringRedisSerializer usage for keys.
In
`@base/src/main/java/com/tinyengine/it/modeldata/service/ModelDataCacheService.java`:
- Around line 41-57: The loadDataList method is caching and returning the full
model dataset, which forces every query path to scan all records in memory and
Redis. Refactor ModelDataCacheService so query-time filtering, sorting, and
pagination happen in the database instead of materializing all rows in
loadDataList, and only cache narrowly scoped or bounded results if needed.
Update the related list/query flow that uses loadDataList to accept page/filter
parameters and avoid full-dataset Redis values for large models.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 845d404c-baf3-43a7-bd9e-0392240e1a91
📒 Files selected for processing (28)
app/src/main/java/com/tinyengine/it/TinyEngineApplication.javaapp/src/main/resources/application-alpha.ymlapp/src/main/resources/sql/mysql/10_create_apikey_table_ddl_2026_0630.sqlapp/src/main/resources/sql/mysql/11_create_modeldata_table_ddl_2026_0630.sqlapp/src/main/resources/sql/mysql/init_data_2026_0630.sqlbase/src/main/java/com/tinyengine/it/common/exception/ExceptionEnum.javabase/src/main/java/com/tinyengine/it/config/RedisConfig.javabase/src/main/java/com/tinyengine/it/dynamic/controller/ModelDataController.javabase/src/main/java/com/tinyengine/it/dynamic/service/DynamicModelService.javabase/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.javabase/src/main/java/com/tinyengine/it/login/config/LoginConfig.javabase/src/main/java/com/tinyengine/it/login/utils/SignatureUtil.javabase/src/main/java/com/tinyengine/it/modeldata/dao/ApiKeyRepository.javabase/src/main/java/com/tinyengine/it/modeldata/dao/ModelDataRepository.javabase/src/main/java/com/tinyengine/it/modeldata/entity/ApiKeyEntity.javabase/src/main/java/com/tinyengine/it/modeldata/entity/ModelData.javabase/src/main/java/com/tinyengine/it/modeldata/exception/BusinessException.javabase/src/main/java/com/tinyengine/it/modeldata/filter/ApiKeyAuthFilter.javabase/src/main/java/com/tinyengine/it/modeldata/service/ApiKeyService.javabase/src/main/java/com/tinyengine/it/modeldata/service/DynamicModelDataService.javabase/src/main/java/com/tinyengine/it/modeldata/service/ModelDataCacheService.javabase/src/main/java/com/tinyengine/it/modeldata/service/ValidationService.javabase/src/main/java/com/tinyengine/it/modeldata/util/CachedBodyHttpServletRequest.javabase/src/main/java/com/tinyengine/it/modeldata/util/CachedBodyServletInputStream.javabase/src/main/java/com/tinyengine/it/modeldata/util/ModelParser.javabase/src/main/java/com/tinyengine/it/service/material/impl/ResourceServiceImpl.javadocker-compose.ymlpom.xml
| @@ -15,6 +15,7 @@ | |||
| import org.mybatis.spring.annotation.MapperScan; | |||
There was a problem hiding this comment.
PR变更太多，请先完成以下内容：
- 在PR描述中补充：基础PR背景介绍、实现方案、改动与影响范围
- 自测(描述下自测了哪些场景、测试点及结果)
- 本地先通过AI检视一轮及完成修复，PR描述贴上截图
- coderabbit 提的检视完成修复
- 部署环境结果，环境测试结果
msslulu
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
base/src/test/java/com/tinyengine/it/modeldata/service/ModelDataCacheServiceTest.java (1)
52-75: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the exact cache key and returned metadata.
ModelDataCacheServiceinvalidates a tenant-scoped Redis key and returns_version/_rowon successful add/update/delete, but these tests only check_idanddelete(anyString()). They will stay green if the wrong cache key is deleted or the response metadata regresses. PopulatemodelId/tenantIdon the mocked update result and verify the concrete key plus_version/_row.Example tightening
`@Test` void testUpdate_Success() throws Exception { Integer dataId = 1; String userId = "user1"; Map<String, Object> newData = new HashMap<>(); newData.put("field1", "newValue"); ModelData mockModelData = new ModelData(); mockModelData.setId(1); + mockModelData.setModelId(1); + mockModelData.setTenantId("tenant1"); mockModelData.setVersion("0.0.2"); mockModelData.setDataJson(newData); when(dynamicModelDataService.update(dataId, newData, userId)).thenReturn(mockModelData); Map<String, Object> result = modelDataCacheService.update(dataId, newData, userId); assertNotNull(result); assertEquals(1, result.get("_id")); + assertEquals("0.0.2", result.get("_version")); + assertEquals(1, result.get("_row")); verify(dynamicModelDataService, times(1)).update(dataId, newData, userId); - verify(redisTemplate, times(1)).delete(anyString()); + verify(redisTemplate, times(1)).delete("modelData:1:tenant1"); }Also applies to: 78-122
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/test/java/com/tinyengine/it/modeldata/service/ModelDataCacheServiceTest.java` around lines 52 - 75, The ModelDataCacheService tests are too loose because they only assert _id and delete(anyString()), so they won’t catch cache-key or response-metadata regressions. Update the add/update/delete success tests in ModelDataCacheServiceTest to verify the exact tenant-scoped Redis key that ModelDataCacheService invalidates, and assert the returned metadata fields (_version and _row) in addition to _id. Also make sure the mocked ModelData for update/delete includes modelId and tenantId so the assertions can verify the real cache key and metadata path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@base/src/test/java/com/tinyengine/it/modeldata/service/DynamicModelDataServiceTest.java`:
- Around line 93-127: The DynamicModelDataService.update path is not being
exercised for its merge-before-validate behavior because the test uses an empty
existing JSON map and only matches anyMap() in validation. Update
testUpdate_Success in DynamicModelDataServiceTest to seed ModelData
existing.getDataJson() with an additional pre-existing field, then capture and
assert the merged payload passed through validationService.validate and
modelDataRepo.save. Use the update method, modelDataRepo.findById, and
validationService.validate as the key symbols to verify the patch preserves
existing fields instead of validating only the incoming data.
In
`@base/src/test/java/com/tinyengine/it/modeldata/service/ValidationServiceTest.java`:
- Around line 196-220: The test named testValidate_OptionalField is not covering
optional-field handling in ValidationService.validate(...), because both
ParametersDto entries are still marked required and both are present in the
input data. Update this test so one field definition is optional, omit that
field from the data map, and optionally include an allowed system field such as
tenantId to verify the intended contract; use the existing
testValidate_OptionalField setup and ValidationService.validate reference to
keep the change localized.
---
Nitpick comments:
In
`@base/src/test/java/com/tinyengine/it/modeldata/service/ModelDataCacheServiceTest.java`:
- Around line 52-75: The ModelDataCacheService tests are too loose because they
only assert _id and delete(anyString()), so they won’t catch cache-key or
response-metadata regressions. Update the add/update/delete success tests in
ModelDataCacheServiceTest to verify the exact tenant-scoped Redis key that
ModelDataCacheService invalidates, and assert the returned metadata fields
(_version and _row) in addition to _id. Also make sure the mocked ModelData for
update/delete includes modelId and tenantId so the assertions can verify the
real cache key and metadata path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fa879e5f-8f54-4570-926c-b77e5ecfcc5f
📒 Files selected for processing (8)
app/src/main/resources/sql/mysql/init_data_2026_0630.sqlbase/src/main/java/com/tinyengine/it/modeldata/exception/BusinessException.javabase/src/main/java/com/tinyengine/it/modeldata/service/ModelDataCacheService.javabase/src/main/java/com/tinyengine/it/modeldata/service/ValidationService.javabase/src/test/java/com/tinyengine/it/modeldata/service/ApiKeyServiceTest.javabase/src/test/java/com/tinyengine/it/modeldata/service/DynamicModelDataServiceTest.javabase/src/test/java/com/tinyengine/it/modeldata/service/ModelDataCacheServiceTest.javabase/src/test/java/com/tinyengine/it/modeldata/service/ValidationServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (4)
- base/src/main/java/com/tinyengine/it/modeldata/exception/BusinessException.java
- app/src/main/resources/sql/mysql/init_data_2026_0630.sql
- base/src/main/java/com/tinyengine/it/modeldata/service/ValidationService.java
- base/src/main/java/com/tinyengine/it/modeldata/service/ModelDataCacheService.java
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
base/src/main/java/com/tinyengine/it/modeldata/filter/ApiKeyAuthFilter.java (2)
81-88: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winMove nonce reservation after signature validation and scope it per API key.
The current global
nonce:<nonce>key is written before authentication succeeds, so invalid requests can burn nonces and collide across different API keys. Validate the signature first, then reservenonce:<apiKey>:<nonce>atomically before forwarding.Suggested fix shape
-String nonceKey = "nonce:" + nonce; -Boolean isNew = stringRedisTemplate.opsForValue().setIfAbsent(nonceKey, "1", Duration.ofMinutes(5)); -if (Boolean.FALSE.equals(isNew)) { - response.setStatus(401); - response.getWriter().write("Nonce already used"); - return; -} - ... String expectedSignature = SignatureUtil.hmacSha256(keyEntity.getApiSecret(), stringToSign); if (!expectedSignature.equals(signature)) { response.setStatus(401); response.getWriter().write("Invalid signature"); return; } + +String nonceKey = "nonce:" + apiKey + ":" + nonce; +Boolean isNew = stringRedisTemplate.opsForValue().setIfAbsent(nonceKey, "1", Duration.ofMinutes(5)); +if (!Boolean.TRUE.equals(isNew)) { + response.setStatus(401); + response.getWriter().write("Nonce already used"); + return; +}Also applies to: 106-112
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/modeldata/filter/ApiKeyAuthFilter.java` around lines 81 - 88, Move the nonce replay check in ApiKeyAuthFilter so it happens only after signature validation succeeds, instead of reserving the nonce before authentication. Scope the Redis key by API key using the same apiKey/nonce values already available in the filter, and use the reservation step atomically right before forwarding the request. Update both nonce check locations in ApiKeyAuthFilter to follow the same per-key pattern and keep the existing 401 handling for reused nonces.
100-104: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winInclude query parameters in the signed canonical request.
Only
requestURIis signed. For query endpoints, an attacker can modify?page=, filters, sorting, or IDs without invalidating the signature becausegetQueryString()is excluded.Suggested fix shape
String method = request.getMethod(); -String path = request.getRequestURI(); +String query = request.getQueryString(); +String path = query == null ? request.getRequestURI() : request.getRequestURI() + "?" + query; String body = wrappedRequest.getBodyString(); String stringToSign = String.format("%s\n%s\n%s\n%s\n%s", method, path, timestamp, nonce, body);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/modeldata/filter/ApiKeyAuthFilter.java` around lines 100 - 104, The canonical request built in ApiKeyAuthFilter’s signing flow currently uses only request.getRequestURI(), so query parameters are not covered by the signature. Update the string-to-sign construction to include the query string when present (for example by appending the output of request.getQueryString() in the canonical path component) so endpoints using page, filters, sorting, or IDs are signed consistently. Keep the change localized to the signing logic around stringToSign and preserve the existing method/path/timestamp/nonce/body ordering while ensuring the full request target is authenticated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@base/src/main/java/com/tinyengine/it/modeldata/filter/ApiKeyAuthFilter.java`:
- Around line 114-120: `ApiKeyAuthFilter` sets the tenant context with
`DefaultLoginUserContext.setCurrentTenant(org)`, but the `finally` block only
calls `DefaultLoginUserContext.clear()`, which leaves `CURRENT_TENANT` behind on
the thread. Update the cleanup in this filter to also clear the tenant context
by calling `clearCurrentTenant()` alongside the existing `clear()` call so the
request-scoped state set here is fully removed.
- Around line 44-48: Bind the tenant to the API key in ApiKeyAuthFilter instead
of trusting the client-supplied X-Lowcode-Org header. In the request handling
flow around the keyEntity lookup and context population, verify the API key’s
owning tenant/org and use that value for the tenant context; if the header is
present and does not match, reject the request. Remove any direct use of the
header as the source of truth so downstream reads/writes are scoped only by the
tenant associated with the validated API key.
---
Outside diff comments:
In `@base/src/main/java/com/tinyengine/it/modeldata/filter/ApiKeyAuthFilter.java`:
- Around line 81-88: Move the nonce replay check in ApiKeyAuthFilter so it
happens only after signature validation succeeds, instead of reserving the nonce
before authentication. Scope the Redis key by API key using the same
apiKey/nonce values already available in the filter, and use the reservation
step atomically right before forwarding the request. Update both nonce check
locations in ApiKeyAuthFilter to follow the same per-key pattern and keep the
existing 401 handling for reused nonces.
- Around line 100-104: The canonical request built in ApiKeyAuthFilter’s signing
flow currently uses only request.getRequestURI(), so query parameters are not
covered by the signature. Update the string-to-sign construction to include the
query string when present (for example by appending the output of
request.getQueryString() in the canonical path component) so endpoints using
page, filters, sorting, or IDs are signed consistently. Keep the change
localized to the signing logic around stringToSign and preserve the existing
method/path/timestamp/nonce/body ordering while ensuring the full request target
is authenticated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 90fd5876-661d-4720-b070-142e3e4290a8
📒 Files selected for processing (5)
base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.javabase/src/main/java/com/tinyengine/it/login/config/context/DefaultLoginUserContext.javabase/src/main/java/com/tinyengine/it/modeldata/dao/ModelDataRepository.javabase/src/main/java/com/tinyengine/it/modeldata/filter/ApiKeyAuthFilter.javabase/src/main/java/com/tinyengine/it/modeldata/service/ModelDataCacheService.java
🚧 Files skipped from review as they are similar to previous changes (3)
- base/src/main/java/com/tinyengine/it/modeldata/dao/ModelDataRepository.java
- base/src/main/java/com/tinyengine/it/modeldata/service/ModelDataCacheService.java
- base/src/main/java/com/tinyengine/it/dynamic/service/DynamicService.java
| String org = request.getHeader("X-Lowcode-Org"); | ||
| if(org == null || org.isEmpty()) { | ||
| response.setStatus(401); | ||
| response.getWriter().write("Missing required header: X-Lowcode-Org"); | ||
| return; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Bind the tenant to the API key instead of trusting X-Lowcode-Org.
Line 115 stores a client-supplied tenant header directly into the context, while keyEntity is not used to verify tenant ownership. A valid API key holder can sign requests for another tenant by changing X-Lowcode-Org, which breaks the PR’s automatic tenant-binding goal and downstream tenant-scoped reads/writes.
Suggested fix shape
ApiKeyEntity keyEntity = apiKeyService.getValidKey(apiKey);
if (keyEntity == null) {
response.setStatus(401);
response.getWriter().write("Invalid API Key");
return;
}
+
+String boundTenant = keyEntity.getTenantId();
+if (!boundTenant.equals(org)) {
+ response.setStatus(401);
+ response.getWriter().write("Tenant does not match API Key");
+ return;
+}
...
-DefaultLoginUserContext.setCurrentTenant(org);
+DefaultLoginUserContext.setCurrentTenant(boundTenant);Also applies to: 90-115
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@base/src/main/java/com/tinyengine/it/modeldata/filter/ApiKeyAuthFilter.java`
around lines 44 - 48, Bind the tenant to the API key in ApiKeyAuthFilter instead
of trusting the client-supplied X-Lowcode-Org header. In the request handling
flow around the keyEntity lookup and context population, verify the API key’s
owning tenant/org and use that value for the tenant context; if the header is
present and does not match, reject the request. Remove any direct use of the
header as the source of truth so downstream reads/writes are scoped only by the
tenant associated with the validated API key.
There was a problem hiding this comment.
@msslulu Understood — I won’t push for a change on this thread.
If the current behavior is intentional, a short note about where tenant binding is guaranteed upstream would make that clearer for future reviewers, but I’m okay to leave it here.
(╥﹏╥)🐇
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| @@ -0,0 +1 @@ | |||
| INSERT INTO `t_api_key` (`id`, `api_key`, `api_secret`, `expire_time`, `tenant_id`, `status`) VALUES (1, '', '', '2032-11-01 11:38:23', NULL, 1); | |||
There was a problem hiding this comment.
初始化数据插入 api_key=''、api_secret=''、status=1 且长期有效。过滤器只检查 header 是否为 null，没有拒绝空字符串；ApiKeyService 也只按 key 和 status 查询。只要该 init SQL 被部署执行，请求方可以发送空 X-API-Key，再用空 secret 计算 HMAC，通过签名校验。结合信任请求头 X-Lowcode-Org问题 ，还可以自选 X-Lowcode-Org 写入租户上下文。
这条是已关闭意见的二次风险：最初 CodeRabbit 说“不要提交真实 API key/secret”。当前代码把真实值改成空字符串，但这不是安全修复，因为空字符串同样可以作为有效凭据使用。
建议改法：不要在初始化 SQL 中插入任何启用状态的默认 API Key。服务端也应拒绝空 X-API-Key、空 X-Signature、空 secret，并且 API secret 不应明文长期落库。
There was a problem hiding this comment.
数据库加密存储密钥和APIKEY，环境变量管理解密密钥
|
|
||
| import java.util.List; | ||
| @Repository | ||
| public interface ModelDataRepository extends JpaRepository<ModelData, Long>, JpaSpecificationExecutor<ModelData> { |
There was a problem hiding this comment.
[Medium] ModelData 主键类型和 Repository 泛型不一致，更新/删除可能运行时失败。
ModelData.id 当前是 Integer，但这里声明为 JpaRepository<ModelData, Long>。这样 findById / deleteById 会按 Long 处理主键，而实体实际主键是 Integer，Hibernate/JPA 运行时容易出现 ID 类型不匹配异常，导致已创建的模型数据无法更新或删除。
建议统一主键类型：要么把 Repository 改成 JpaRepository<ModelData, Integer> 并同步调整 service 中的 Long.valueOf(...) / deleteById(Long)；要么把实体 id 统一改成 Long。
There was a problem hiding this comment.
service层调用的时候已经做了转换
|
|
||
| String tenantId = DefaultLoginUserContext.getCurrentTenant(); | ||
| Integer modelId=null; | ||
| List<Model> modelList = modelService.getModelByEnName(dto.getNameEn()); |
There was a problem hiding this comment.
[High] 多租户场景下不能只按 nameEn 查模型后取第一条。
这里用 getModelByEnName(dto.getNameEn()) 查出列表后，后续直接 modelList.get(0) 获取 modelId / createdBy。如果不同租户存在相同 nameEn 的模型，就可能拿到另一个租户的模型定义或创建人，再用当前租户上下文去读写 t_model_data，造成数据错读、错写或权限边界混乱。
建议改为按当前租户精确查询模型，例如 nameEn + tenantId，并在 query/insert/update/delete 四条路径都使用同一个租户范围内解析出来的 model。多租户代码里不应依赖 “第一条记录” 决定模型归属。
There was a problem hiding this comment.
创建模型的时候校验了模型唯一性，nameEn 重复会报错，所以通过nameEn 查到的只有一条数据
| } | ||
|
|
||
| // 4. Nonce 防重放（Redis 存储，有效期5分钟） | ||
| String nonceKey = "nonce:" + nonce; |
There was a problem hiding this comment.
[Medium] nonce 在签名校验前就写入 Redis，且没有按 API Key 隔离。
当前先执行 setIfAbsent("nonce:" + nonce)，之后才校验 API Key 和签名。这样无效请求也能消耗 nonce，导致后续合法请求被误判为重放；同时 Redis key 没包含 API Key，不同 API Key 如果使用相同 nonce 也会互相冲突。
建议先完成 API Key 和签名校验，再原子写入 nonce；Redis key 也应按 key 隔离，例如 nonce:<apiKey>:<nonce>。这样只有认证通过的请求会占用 nonce，不同 API Key 之间也不会相互影响。
|
|
||
| // 7. 构建待签名字符串：method\npath\ntimestamp\nnonce\nbody | ||
| String method = request.getMethod(); | ||
| String path = request.getRequestURI(); |
There was a problem hiding this comment.
[Medium] 签名没有覆盖 query string，查询参数可被篡改。
这里使用 request.getRequestURI() 构造待签名 path，它只包含路径，不包含 ?page=...&sort=... 这类 query 参数。对于查询、分页、排序、过滤等接口，调用方或中间人可以修改 query 参数而不改变签名校验结果。
建议在 canonical path 中包含 request.getQueryString()，例如 query 存在时使用 requestURI + "?" + queryString，保持现有 method/path/timestamp/nonce/body 顺序不变，但确保完整请求目标被签名保护。
3 participants
1. PR变更背景：
模型驱动接口不能传递token的安全漏洞
2 解决方案
API Key + 请求签名 的方式，将认证信息放在 Header 中：
.X-API-Key: 客户端标识，
X-Timestamp: 时间戳
X-Nonce: 随机字符串
X-Signature: HMAC-SHA256 签名
通过 X-API-Key、X-Timestamp、X-Nonce、X-Signature 四个 Header 实现身份认证、防篡改、防重放，并自动绑定租户
postman 测试脚本：
// ========== 1. 从环境变量获取密钥 ==========
const apiKey = pm.environment.get("API_KEY") || "与服务端一致";
const secret = pm.environment.get("API_SECRET") || "与服务端一致";
// ========== 2. 生成时间戳和 Nonce ==========
const timestamp = Date.now().toString(); // 毫秒级时间戳
const nonce = Math.random().toString(36).substring(2, 18) + Math.random().toString(36).substring(2, 18); // 32位随机字符串
// ========== 3. 获取请求方法和路径 ==========
const method = pm.request.method;
const path = pm.request.url.getPath(); // 不包含 host 和 query 参数
// ========== 4. 获取请求体（如果有） ==========
let body = "";
if (pm.request.body && pm.request.body.mode === "raw") {
// 只对 raw 类型（JSON/文本）进行签名
body = pm.request.body.raw || "";
// 注意：如果 body 是空对象 {}，也应该保留为 "{}"，不能省略
} else if (pm.request.body && pm.request.body.mode === "formdata") {
// 对于 form-data，通常不签体，或者签一个空字符串，根据实际约定
// 这里为了简单，留空
body = "";
} else {
// GET/DELETE 等没有 body 的请求
body = "";
}
// ========== 5. 构造待签名字符串 ==========
// 格式: method\npath\ntimestamp\nnonce\nbody
const stringToSign =
${method}\n${path}\n${timestamp}\n${nonce}\n${body};// ========== 6. 计算 HMAC-SHA256 签名 ==========
const signature = CryptoJS.HmacSHA256(stringToSign, secret).toString(CryptoJS.enc.Hex);
// ========== 7. 设置环境变量 ==========
pm.environment.set("timestamp", timestamp)
pm.environment.set("nonce", nonce)
pm.environment.set("signature", signature)
2.数据持久化
增加t_model_data,redis存储模型驱动数据，不再创建模型表，杜绝SQL注入风险与DDL注入风险
影响范围：
模型驱动前端调用接口需要增加以上head参数
自测增删改查接口：
POST:
platform-center/api/model-data/queryApi
request :
{
"nameEn":"staff",
"nameCh":"员工",
"currentPage":2,
"pageSize":5,
"params":{
"name":"like:s",
"age":28,
"ifperm":true,
"startDate":null,
"politStatus":null
}
respose ：
{
"data": {
"total": 9,
"pages": 2,
"pageSize": 5,
"list": [
{
"id": 13,
"name": "s6",
"age": 28,
"ifperm": true
},
{
"id": 14,
"name": "s7",
"age": 28,
"ifperm": true
},
{
"id": 15,
"name": "s8",
"age": 28,
"ifperm": true
},
{
"id": 16,
"name": "s9",
"age": 28,
"ifperm": true
}
],
"pageNum": 2
},
"code": "200",
"message": "操作成功",
"error": null,
"errMsg": null,
"success": true
}
127.0.0.1:9090/platform-center/api/model-data/insertApi
POST:
Summary by CodeRabbit