# xlyAi `agent-main` 审计报告

分支 `agent-main` @ `f3086c3` · 62 files / ~6k LOC · 离线编译通过（42 classes）

方法：10 个维度并行审计 → 每维度对抗式复核（默认反驳）→ 去重排序。原始 132 条，反驳掉 21 条，存活 111 条，去重后 35 条。


## 结论

> Not safe to ship: the service has no authentication layer at all, and a request that simply omits `authorization` is silently promoted to an ERP sysadmin session (admin/666666) that can read every tenant's data and — via the equally anonymous POST /op/{id}/confirm — commit real ERP writes.


## 合并前必须修（Blockers）

- src/main/java/com/xly/web/AgentChatController.java:515 — resolveIdentity falls back to dev-admin (sysadmin, all modules, admin/666666 ERP login) when authorization is blank OR when the permission query throws; no auth layer exists anywhere in the app
- src/main/java/com/xly/web/OpController.java:105 — confirm/cancel/pending execute, cancel and dump staged ERP writes with no credential and no owner/tenant check; opIds are handed out by the anonymous /op/pending
- src/main/resources/application.yml:31 — production MySQL (public IP, useSSL=false) and Redis credentials committed since the initial commit; rotate before anything else
- src/main/java/com/xly/service/FormResolverService.java:231 — fkOptionPage's tenant predicate is conditional on an optional client query param and FormController:32 is unauthenticated: full cross-tenant customer/supplier/product dump
- src/main/java/com/xly/web/ConversationController.java:31 — list/history/delete are keyed on client-supplied userid/convId with no ownership check; chat memory is loadable and poisonable via a chosen conversationId
- src/main/java/com/xly/service/FormResolverService.java:360 — resolveFk has no sBrandsId predicate, so a staged write can bind another tenant's customer/product sId (and acts as a cross-tenant existence oracle)
- src/main/java/com/xly/tool/ProposeWriteTool.java:110 — doUpdate stages the raw string: FK id columns receive Chinese names, numeric columns receive text, and sBillNo/sMakePerson are editable via chat
- src/main/java/com/xly/service/FormResolverService.java:289 — coerce turns 「1,000」 into 1 and 「一千」 into "0" while ProposeWriteTool:225 shows the original text on the confirm card
- src/main/java/com/xly/web/OpController.java:112 — no compare-and-set on sStatus: concurrent confirms double-execute, and a timed-out write is recorded as 'failed' while the ERP committed
- src/main/java/com/xly/tool/ProposeWriteTool.java:245 — sBillNo is MAX+1 at propose time, so any two drafts staged before either is confirmed carry the same 单号

## 系统性根因

1. Identity is client-declared, end to end. Every trust decision — module grants, tenant scoping, audit attribution, conversation ownership, bill-number series — reads a field out of the JSON request body. The ERP token is only ever forwarded, never introspected, so the service has no server-derived notion of who is calling. Ranks 1, 5, 6, 7 are all one missing token-introspection call.

2. Fallbacks fail open instead of closed. Blank token -> dev admin. Permission query throws -> dev admin. Missing brandsid -> no tenant predicate. Unresolvable search field -> unfiltered read. Non-numeric input -> literal "0". Unparseable gate response -> 其他 with the anti-fabrication guard off. Every one of these turns a failure into a silently broader capability rather than an error.

3. The HITL queue is a bearer capability with no state machine. ai_op_queue rows have no owner predicate, no tenant columns (declared in DDL, never written), no expiry (tExpireAt declared, never written), no compare-and-set on status, and no idempotency key — and all three endpoints that touch them are anonymous. The one control the whole architecture rests on is the weakest object in the system.

4. Propose-time snapshots are executed at confirm time without revalidation. Bill numbers, old values, FK resolutions and column validity are all computed when the draft is staged and replayed verbatim minutes-to-months later. Nothing re-reads the target row, re-derives the sequence, or validates the payload before the write lands.

5. The confirmation card is built from user input, not from the payload that will execute. descParts is appended outside the coerce guard; the located record's real name is discarded when nameField is null; the update path never shows the resolved FK. The human reviewing the write is shown a different document than the one that runs — which structurally defeats human-in-the-loop for exactly the mistakes (money, quantity, wrong record) it exists to catch.

6. Failures are made invisible by design. Bare catch (Exception ignore) {} in the FK/KG paths, HTTP 200 for every exception, a client that deletes the message row when it gets zero SSE frames, and a permanently cached degraded system prompt mean an outage or a misconfiguration reads as an empty result rather than an error — for users and for monitoring alike.

7. Documentation asserts controls that were deleted or never built. security-findings.md credits a SQL sandbox that no longer exists, FormController's javadoc and §14 claim tenant filtering the code contradicts, ai_op_queue.sql and §10 document a tenant dimension nothing writes, and application-saaslocal.yml declares an erp.dev-login.enabled flag with no consumer. The docs are not merely stale; they would cause a reviewer to sign off on absent mitigations.


## 全部发现（按影响排序）


### CRITICAL

**1. No authentication anywhere; blank/failed `authorization` silently becomes ERP dev-admin with all-module grants**  
`src/main/java/com/xly/web/AgentChatController.java:515` · auth-tenant

`curl -X POST http://host:8099/xlyAi/api/agent/chat -d '{"text":"列出所有客户"}'` with no credentials returns real ERP data: resolveIdentity() sees blank authorization (or catches ANY exception from authz.userIdentity, e.g. a dropped connection on the unguarded queryForList at AuthzService:80/98) and returns authz.devIdentity() — token=null, userType default "sysadmin" so grantedIds() returns null = every module, and ErpClient.resolveToken(null) logs into the production ERP with the @Value defaults admin/666666. There is no Spring Security, no Filter, no HandlerInterceptor in the whole app (only CorsFilter). The committed application.yml has no erp.dev-login block at all, so those admin defaults ARE the production configuration, and the `erp.dev-login.enabled: true` flag in application-saaslocal.yml has zero @Value/Environment consumers (verified: grep returns nothing) — the fallback is unconditional in every profile.

*修复：* Reject with 401 when `authorization` is blank or userIdentity() throws (fail closed, never fall through to devIdentity). Add a real @Value("${erp.dev-login.enabled:false}") gate in AuthzService/ErpClient, and delete the `admin`/`666666`/`sysadmin` literal @Value defaults so an unconfigured deploy cannot authenticate at all.

<sub>合并自 5 条独立发现：auth-tenant/no-auth-dev-admin-fallback, agent-core/identity-fail-open-admin, docs-drift/devlogin-admin-fallback, config-ops/devlogin-admin-on-by-default, auth-tenant/authz-fail-open</sub>

**2. POST /op/{id}/confirm, /cancel and GET /op/pending are anonymous and have no owner or tenant check**  
`src/main/java/com/xly/web/OpController.java:105` · write-path

confirm() loads the row by path id, checks only sStatus=='draft', and executes against the ERP; it never compares op.sUserId/sBrandsId to the caller, and the Authorization header is `required = false` so ErpClient.resolveToken(null) executes the write as dev-admin. The opId does not need guessing: GET /api/agent/op/pending?conversationId=<victim-userid>:default (OpController:94, unauthenticated, SELECT * including sPayload) hands it over, and conversationIds are enumerable via the equally open GET /api/agent/conversations?userid=<victim>. cancel() takes no token at all. OpService.createDraft/createDraftPayload never populate the sBrandsId/sSubsidiaryId columns that ai_op_queue.sql:10-12 declares, so no tenant predicate is even possible. Net: an anonymous caller reads another user's staged payload (prices, customer ids, bill number), then executes their 作废/审核/新增 against the ERP, with ai_audit_log attributing it to the victim.

*修复：* Require the token on confirm/cancel/pending, resolve the caller identity from it, persist sUserId/sBrandsId/sSubsidiaryId on insert, and add `AND sUserId=? AND sBrandsId=?` to ops.get/pending; also mint opIds from SecureRandom instead of millis+Math.random.

<sub>合并自 8 条独立发现：auth-tenant/confirm-no-owner-check, write-path/confirm-no-authz, sql-safety/op-queue-no-owner-predicate, auth-tenant/pending-op-leak, auth-tenant/cancel-no-auth, docs-drift/op-queue-no-owner-no-tenant, frontend/op-confirm-no-ownership-check, write-path/opid-guessable</sub>

**3. Production MySQL and Redis credentials committed to git, against a public IP with useSSL=false**  
`src/main/resources/application.yml:31` · config-ops

application.yml (the default profile, i.e. what ships in the WAR) hardcodes jdbc:mysql://118.178.19.35:3318/xlyweberp_saas?...useSSL=false&allowMultiQueries=true, xlyprint/xlyXLYprint2016 and Redis xlyXLY2015. `git show 6263521:src/main/resources/application.yml` shows the same values in the initial commit, so they are in every clone and every reflog. That account is read-write (AuditService INSERTs) on the shared multi-tenant schema — anyone with repo access gets full cross-tenant ERP read/write directly, bypassing every application-level control, and useSSL=false exposes the password to any on-path observer.

*修复：* Rotate both passwords now, move them to environment variables/secrets store (SPRING_DATASOURCE_PASSWORD), enable TLS, drop allowMultiQueries, and restrict port 3318 to app hosts. Purging git history alone is insufficient — assume compromised.

**4. GET /api/agent/form/options is unauthenticated and its tenant predicate is an optional client query parameter**  
`src/main/java/com/xly/service/FormResolverService.java:231` · auth-tenant

fkOptionPage appends `AND sBrandsId=?` only if (brand != null && !brand.isBlank()), and FormController:34 takes brandsid as an optional request param with no token and no identity. `curl 'http://host:8099/xlyAi/api/agent/form/options?table=elecustomer&pageSize=100&page=1'` therefore returns customer names plus three business columns across EVERY tenant in the shared xlyweberp_saas DB; pageSize is clamped to 100 but page is unbounded so the whole master table is walkable, and `total` discloses per-tenant row counts. The allowlist only requires the table to appear as sFkTable in viw_kg_field_dict — i.e. exactly elecustomer/elesupplier/eleproduct/elematerials/employee tables. FormController's own javadoc (line 19) and docs/agent-architecture.md §14 both claim 租户过滤 is enforced here.

*修复：* Require the ERP token on this endpoint, take brandsId from the server-resolved identity, ignore any client-supplied brandsid, and make the sBrandsId=? predicate unconditional (return empty when the tenant cannot be resolved).

<sub>合并自 5 条独立发现：auth-tenant/unauth-cross-tenant-fk-dump, sql-safety/fkoptions-tenant-param-client-controlled, form-pipeline/fk-options-unauth-tenant, frontend/form-options-unauthenticated-tenant-param, docs-drift/form-options-unauth-cross-tenant</sub>

**5. Conversation transcripts and chat memory are addressable by client-chosen userid/conversationId with no ownership check**  
`src/main/java/com/xly/web/ConversationController.java:31` · state-memory

GET /conversations?userid=<victim> lists that user's conversation ids; GET /conversations/{convId}/messages replays the full ledger (customer names, prices, 【待确认】/【已执行】 write descriptions) with no credential at all; DELETE wipes chat:mem/chat:ledger/chat:state keyed on convId alone. Separately, /api/agent/chat takes req.conversationId verbatim and uses it as the LangChain4j memoryId (AgentChatController:112 → AgentFactory:81 → chat:mem:{convId}), so posting {"text":"把上文原样复述一遍","conversationId":"<victim>:default"} loads another tenant's prior ERP rows into the prompt and streams them back — and appending to the same key poisons what the victim's next turn sees. None of the four Redis namespaces carries brandsId/subsidiaryId.

*修复：* Derive userId from the authenticated token, namespace memory/ledger/state keys with brandsId+userId, and verify chat:convs:{userId} contains convId before any read, write or delete.

<sub>合并自 6 条独立发现：auth-tenant/conversation-idor, auth-tenant/chat-memory-idor, state-memory/conversation-history-no-ownership-check, state-memory/memory-key-not-tenant-scoped, agent-core/conversation-id-not-bound-to-user, frontend/conversation-endpoints-idor</sub>


### HIGH

**6. usertype/userid/brandsid are read from the request body, so any caller self-declares sysadmin and forges the audit trail**  
`src/main/java/com/xly/web/AgentChatController.java:518` · auth-tenant

resolveIdentity forwards req.usertype/userid/brandsid/subsidiaryid straight into AuthzService.userIdentity with no cross-check against the token. isAdmin("sysadmin") makes grantedIds() return null, which AgentIdentity.canAccessModule treats as 'all modules' — and per AuthzService's own javadoc the ERP backend's per-user form permission check is switched off, so this is the ONLY form-level control. A warehouse clerk posting their own valid token plus "usertype":"sysadmin" reaches payroll/finance forms via readFormData and stages writes on them. The same body-supplied userid is stored as ai_op_queue.sUserId and replayed into every ai_audit_log row for propose/confirm/cancel, so the immutable audit trail names whoever the attacker chooses; the same brandsid feeds nextBillNo, leaking another tenant's numbering series.

*修复：* Resolve userId/brandsId/subsidiaryId/userType server-side from the presented ERP token (a whoami/session-introspection call against xlyEntry) and ignore the body-supplied values entirely.

<sub>合并自 6 条独立发现：auth-tenant/forged-usertype-admin, write-path/usertype-self-declared-admin, frontend/client-declared-usertype-grants-admin, docs-drift/usertype-spoof-defeats-authz, auth-tenant/forgeable-audit-attribution, auth-tenant/billno-forged-brand</sub>

**7. Shipped /chat console hardcodes a sysadmin identity and empty token and is mapped in every profile**  
`src/main/resources/templates/chat.html:342` · frontend

MvcConfig:12-13 registers /chat and redirects / to it with no @Profile and no guard. chat.html hardcodes userid=1752296756…, brandsid/subsidiaryid=1111111111, usertype="sysadmin", authorization="" and there is not a single Thymeleaf expression in the file, so the host ERP cannot inject a real token — the page can ONLY take the dev-admin path. Anyone who opens http://host:8099/xlyAi/ gets a working point-and-click agent that can stage a 作废 and click 确认 against the production ERP. Because every visitor shares the same hardcoded userid, the sidebar also lists and can delete every other visitor's conversations.

*修复：* Gate the view controller behind a dev profile, and have the host ERP shell inject identity/token server-side (or via postMessage) instead of shipping literals; hard-fail when authorization is blank.

<sub>合并自 3 条独立发现：frontend/hardcoded-superadmin-identity, auth-tenant/unauth-chat-console, config-ops/debug-chat-ui-exposed</sub>

**8. LLM-controlled formId/moduleId are concatenated unencoded into the ERP request URL, and the authz check gates the wrong parameter**  
`src/main/java/com/xly/service/ErpClient.java:132` · sql-safety

doRead builds baseUrl + "/business/getBusinessDataByFormcustomId/" + formId + "?sModelsId=" + moduleId + "&sName=" with no encoding, no validation and no allowlist; URI.create does not normalize dot-segments, so formId=../../ai/execStaging/op-… is sent raw and Tomcat re-maps it — an LLM-steered tool argument reaches ANY endpoint under erp.baseurl carrying the caller's token. Both values are free-text @P arguments the user can steer in Chinese. Compounding it, ErpReadTool:126 authorizes moduleId while the ERP selects the dataset by the path formId, which is never checked against the granted set — and KgQueryTool.findForms hands the model formIds for all 1748 forms with no authz filter, so a non-admin can pair a granted moduleId with a payroll formId and read it.

*修复：* Validate both ids against ^[0-9A-Za-z_-]{1,64}$ and URL-encode before concatenation; better, look the (formId, moduleId) pair up in viw_ai_useful_forms and use the values read back from the DB, authorizing the resolved module.

<sub>合并自 3 条独立发现：sql-safety/erp-url-path-injection, sql-safety/erp-url-query-param-injection, sql-safety/authz-checks-wrong-parameter</sub>

**9. resolveFk binds foreign keys by fuzzy name across all tenants, and the FK picker throws away the sId it already selected**  
`src/main/java/com/xly/service/FormResolverService.java:360` · sql-safety

resolveFk runs SELECT sId FROM <fkTable> WHERE <nameField> LIKE ? ORDER BY CHAR_LENGTH(...) LIMIT 1 with no sBrandsId predicate and no identity parameter — while the sibling fkOptionPage on the same tables does filter by brand. Brand B's user saying 「给 华为 报价」 when only brand A has that customer gets brand A's sId staged into quoquotationmaster.sCustomerId, and the error/success difference is a cross-tenant existence oracle. Picking from the dropdown does not help: chat.html:776 passes back only the display NAME, discarding the sId already selected at FormResolverService:227, so the server re-resolves by fuzzy match. LIKE metacharacters are unescaped, so a material code like 'PE_005' widens the pattern and CHAR_LENGTH ASC deterministically selects a shorter unintended row; a value of '%' matches an arbitrary record.

*修复：* Have the picker submit {id, name} and use the sId directly; for free-text input pass identity.brandsId() into resolveFk and add AND sBrandsId=?, escape %/_/backslash with ESCAPE, and reject non-unique matches instead of LIMIT 1.

<sub>合并自 4 条独立发现：auth-tenant/fk-resolution-cross-tenant, sql-safety/resolvefk-no-tenant-filter, form-pipeline/fk-picker-drops-sid, sql-safety/like-metacharacters-unescaped</sub>

**10. doUpdate stages the raw user string: no FK resolution, no type coercion, no system-column guard**  
`src/main/java/com/xly/tool/ProposeWriteTool.java:110` · write-path

doUpdate resolves only the technical column name (discarding sFkTable) and stores newValue untouched; OpController:147 hands it to erp.updateForm which does col.put(field, value) verbatim. doCreate does the opposite three things correctly at lines 207-224 (isSystemColumn skip, resolveFk, coerce) — none exist on the update path. 「把销售订单 XS…的业务员改成李四」 writes sSalesManId='李四' (an FK id column) so the order points at a non-existent employee and every join/报表 on it breaks; 「把数量改成一千个」 writes '一千个' into a decimal column; 「把报价单 BJD202607001 的单号改成 BJD202606001」 rewrites sBillNo, the primary business key, and 制单人 (sMakePerson) is editable the same way, falsifying the ERP's own audit trail.

*修复：* In doUpdate: select sFkTable alongside sField and run resolveFk (hard-failing when the name does not resolve, as doCreate does), run resolver.coerce(columnTypes(table).get(field), newValue), and reject when resolver.isSystemColumn(field).

<sub>合并自 2 条独立发现：write-path/update-writes-raw-text-to-fk, write-path/update-bypasses-system-column-guard</sub>

**11. Numeric coercion keeps only the first digit run, and the confirmation card is built from the raw input rather than the coerced payload**  
`src/main/java/com/xly/service/FormResolverService.java:289` · form-pipeline

coerce() returns the FIRST -?\d+(\.\d+)? match, or the literal "0" when there are no digits. chat.html:661 renders number fields as type="text", so 数量=「1,000」 and 单价=「1,200.50」 both become "1", and 「一千」 becomes "0". Meanwhile ProposeWriteTool adds descParts.add(zh + "=" + v) (line 225, and 364 in proposeQuote) OUTSIDE the if (cv != null) guard, so the confirm card reads 「数量=1,000，单价=1,200.50」 while the payload carries dQty=1, dPrice=1. The human-in-the-loop gate — the one control standing between the LLM and real money — is structurally incapable of catching the exact class of error it exists for.

*修复：* Strip thousand separators/full-width digits, parse with BigDecimal, return an error instead of substituting "0", and build the description from the coerced payload map (surfacing any dropped field explicitly) rather than from the raw input.

<sub>合并自 2 条独立发现：form-pipeline/numeric-coercion-first-digit-run, write-path/summary-diverges-from-payload</sub>

**12. confirm() is a check-then-act with no compare-and-set, so a draft can execute twice and a timed-out write is recorded as failed**  
`src/main/java/com/xly/web/OpController.java:112` · concurrency-resources

The status is read in Java at line 112, the ERP call runs at 132-150 (up to 40s for createMulti, 120s for execStaging), and ops.setStatus fires only afterwards as an unconditional UPDATE ... WHERE sId=? with no AND sStatus='draft', no row lock and no @Transactional. Two overlapping POSTs (host ERP panel polling /pending in a second tab, a proxy retry, a scripted call) both see 'draft' and both call erp.createMulti with the same pre-generated sId. The timeout half needs no concurrency at all: a mid-write HttpClient timeout throws, the catch forces sStatus='failed' and writes 「用户确认后执行失败」 into the ledger and chat memory even though the ERP committed — the user re-proposes and creates a second real quotation. In the exec-staging branch the exception path never sets status at all, leaving the row 'draft' and replayable.

*修复：* Claim the row first: UPDATE ai_op_queue SET sStatus='executing', tConfirmDate=NOW() WHERE sId=? AND sStatus='draft' and proceed only when rowsAffected==1; on exception leave it 'executing'/'unknown', never 'failed', and require an explicit reconcile before retry.

<sub>合并自 3 条独立发现：write-path/confirm-no-idempotency, concurrency-resources/confirm-toctou-double-exec, write-path/execstaging-leaves-draft</sub>

**13. sBillNo is computed as MAX+1 at propose time and frozen in the payload, so two un-confirmed drafts share one 单号**  
`src/main/java/com/xly/tool/ProposeWriteTool.java:245` · write-path

nextBillNo derives the sequence from MAX(CAST(SUBSTRING(sBillNo,...)))+1 over rows already committed in the TARGET table, while the draft lives only in ai_op_queue.sPayload — so this is not even a thread race: any two drafts staged before either is confirmed deterministically get the identical string. One salesperson creating two quotations in a session, or two salespeople in the same brand, end up with two 报价 sharing BJD202607083 (or the second confirm dies on a unique-index violation after the human already approved). The YYYYMM segment is frozen too, so a draft staged 31 July and confirmed 1 August carries a July number.

*修复：* Do not pre-allocate: omit sBillNo and let the ERP mint it at insert (ErpClient:333's own javadoc says the ERP reuses 校验/单号), or allocate from a sequence table inside the confirm transaction with a duplicate-key retry.

<sub>合并自 3 条独立发现：write-path/billno-reserved-at-propose, concurrency-resources/billno-race-preallocated, form-pipeline/billno-race-at-propose-time</sub>


### MEDIUM

**14. Staged drafts never expire and sOldValue is never re-checked, so a stale proposal executes against data that has moved on**  
`src/main/java/com/xly/service/OpService.java:29` · write-path

ai_op_queue.sql declares tExpireAt and an 'expired' status and docs/agent-architecture.md:130 documents 「久不确认按 tExpireAt 过期」, but grep across src/main finds zero references to either; there is no @Scheduled sweep and confirm() gates only on sStatus=='draft'. OpService.pending() returns the newest draft for a conversation with no age filter and convId defaults to the stable '<userid>:default', so a 1 July 「作废 XSD202606031」 draft is still returned as the live confirm card in August after the order has been 审核 and shipped. Likewise doUpdate captures sOldValue and renders 「由「10000」改为「20000」」 but confirm() never re-reads the record, so a concurrent edit by finance is silently overwritten.

*修复：* Set tExpireAt = NOW() + INTERVAL 15 MINUTE on insert, add AND tExpireAt > NOW() to the confirm guard plus a scheduled sweep to 'expired', and re-read the target row at confirm time, aborting when sField no longer equals sOldValue.

<sub>合并自 2 条独立发现：write-path/no-expiry-gc, write-path/no-optimistic-concurrency</sub>

**15. deriveWriteAction substring-matches the whole utterance, so editing a field named 审核人 stages an approval of the whole record**  
`src/main/java/com/xly/web/AgentChatController.java:476` · agent-core

On the 操作已有单据 branch the action is chosen purely by scanning the raw sentence for 审核/审批 then 删除/删掉/删了, ignoring the extracted slots. 「把销售订单 XS20250101 的审核人改成张三」 yields action="examine"; handleWrite only requires field+newValue when action=="update", so it goes straight to proposeWrite and doExamineOp discards fieldChinese/newValue entirely. The user's actual edit is dropped and a 审核 of the whole sales order is offered under a 确认 button; 「把这张送货单的备注删掉」 likewise stages a 作废 of the entire delivery note.

*修复：* Derive the action from the slots as well: when field and newValue are both present force "update" regardless of keywords, and only treat 审核/删除 as the verb when they are not part of a field name (not followed by 人/状态/时间/日期, not inside a 把…的…改成 frame).

**16. Hitting maxSequentialToolsInvocations persists an AiMessage with unanswered tool_calls into Redis for 30 days**  
`src/main/java/com/xly/config/AgentFactory.java:79` · agent-core

Verified against the langchain4j 1.14.0 sources: AiServiceStreamingResponseHandler.onCompleteResponse does addToMemory(aiMessage) at line 283 BEFORE the sequentialToolsInvocationsLeft-- == 0 throw at 287, and ProjectedChatMemory.add writes through to Redis synchronously. Nothing rolls it back, and neither project() nor trimToCap() validates request/result pairing, so every later turn replays an assistant tool_calls message with no matching tool result. On the configured Ollama endpoint that is a permanently corrupt transcript (repeat tool calls, confused model); if llm.base-url is ever repointed at a strict OpenAI-compatible gateway — which the config comment explicitly invites — the conversation hard-fails with 400 on every subsequent turn. The user meanwhile sees the raw English 'Something is wrong, exceeded 8 sequential tool invocations' because onError forwards err.getMessage() verbatim.

*修复：* In project()/add, drop any AiMessage whose toolExecutionRequests have no matching ToolExecutionResultMessage (or synthesise a '已中断' result for each), and map the cap exception to a Chinese user-facing message.

<sub>合并自 2 条独立发现：agent-core/react-cap-dangling-toolcall, state-memory/dangling-tool-call-bricks-conv</sub>

**17. CORS reflects any origin with credentials on /**, turning any employee browser into a proxy to this unauthenticated host**  
`src/main/java/com/xly/config/CorsConfig.java:29` · auth-tenant

Both a CorsFilter bean (lines 26-49) and a duplicate WebMvcConfigurer (56-67) register /** with allowedOriginPattern("*"), all methods/headers, exposedHeaders("*") and allowCredentials(true). Because it is a pattern rather than a fixed list, Spring echoes the caller's Origin, so a hostile page an employee visits can both send AND READ responses: fetch the cross-tenant customer dump from /form/options, drive /api/agent/chat, read the opId out of the SSE stream, and POST /op/{id}/confirm. The service sits behind nginx-saas on an internal path, so origin reflection is what makes an otherwise network-restricted, unauthenticated host reachable from the public web.

*修复：* Replace the wildcard with an explicit allow-list of the host ERP origins, delete one of the two duplicate registrations, and stop exposing * response headers.

<sub>合并自 3 条独立发现：auth-tenant/cors-wildcard-credentials, frontend/cors-wildcard-with-credentials, config-ops/cors-wildcard-with-credentials</sub>

**18. Redis chat memory is mutated by non-atomic GET/SET from two request threads, silently losing messages**  
`src/main/java/com/xly/config/RedisChatMemoryStore.java:54` · state-memory

appendTurn (GET → mutate → SET) and ProjectedChatMemory.add (same read-modify-write on the same key, from the model callback thread) have no WATCH/MULTI/Lua/lock. AgentFactory builds a fresh memory instance per request, and chat.html leaves the 确认 button enabled while a later turn streams — so clicking 确认 mid-stream makes OpController.recordOutcome write back a stale snapshot, most often dropping its own 「（系统）用户已确认，操作执行成功」 note. The next turn's model then still believes the op is pending and re-proposes the same ERP write. In the worse interleaving the AiMessage carrying tool_calls is dropped and its result message is left orphaned.

*修复：* Serialize per-conversation memory mutation (Redis Lua or a per-convId lock), or make appendTurn an append-only RPUSH instead of rewriting the whole blob.

**19. Collect-form fields come from form-control metadata and are never checked against real table columns**  
`src/main/java/com/xly/tool/FormCollectTool.java:100` · form-pipeline

viw_kg_field_dict maps sTable = gdsconfigformmaster.sTbName x sField = gdsconfigformslave.sName, i.e. every control on a form is attributed to the master table — including display aliases and slave-table columns. FormResolverService:130-132 documents exactly this ('表单里 sProductName/dProductQty 并非真实列'), which is why quoquotationmaster is hand-curated; every OTHER entity uses the generic path. FormCollectTool already holds the authoritative information_schema types map at line 77 but uses it only for inferType, and ProposeWriteTool passes a null type to coerce and puts the value in the payload anyway. The user fills an alias, approves the card, and the ERP insert either errors on an unknown column or silently drops the input — after human approval.

*修复：* Filter businessFields to types.containsKey(col) in FormCollectTool and re-check the same before putting a column into the payload in ProposeWriteTool.doCreate.

**20. locateRecord issues an unfiltered ERP read when the name field cannot be resolved and treats the single row returned as the target**  
`src/main/java/com/xly/tool/ProposeWriteTool.java:504` · form-pipeline

searchField() returns null when the table has no '%Name' column in the dictionary and the keyword is not a bare bill-number pattern (e.g. 「作废 BJD-202607082」 — the dash defeats the [A-Za-z]{1,6}\d{4,} regex). ErpClient.doRead only adds a bFilter when filterField is non-blank, so the request degrades to 'page 1, 5 rows, unfiltered'; if exactly one row comes back locateRecord accepts it as the located record, and because nameField is null the summary falls back to the user's raw keyword (line 538). The card reads 作废【BJD-202607082】 for a record that was never matched, and confirming voids an unrelated ERP document.

*修复：* Abort with an explicit error when the search field cannot be resolved instead of issuing an unfiltered read, and never label the located record with the raw user keyword.

**21. Write proposals become unconfirmable after a reload or conversation switch, stranding drafts forever**  
`src/main/resources/templates/chat.html:455` · frontend

switchConv replays history as plain bubbles only; ConversationService.history:114 renders a proposal as an AI text bubble 【待确认】… with no buttons, and chat.html never calls GET /op/pending (grep: zero hits) even though OpController:93's javadoc says the frontend polls it. newConv() also runs unconditionally at load (line 830), so a refresh never returns to the prior conversation. A user who switches away from a pending 报价 card can never confirm or cancel it: the ai_op_queue row stays 'draft' forever — and, combined with the missing expiry above, remains a live executable capability.

*修复：* Poll /api/agent/op/pending after load/switchConv and re-render live drafts as actionable cards; restore the last conversation on load instead of always creating a new one.

<sub>合并自 2 条独立发现：frontend/proposal-cards-unrecoverable, frontend/newconv-on-every-load</sub>

**22. In-flight cards land in the wrong conversation, and clicking an askUser chip or pressing Enter mid-stream silently discards the input**  
`src/main/resources/templates/chat.html:595` · frontend

Three related client bugs, all triggered by the normal case of a slow turn. (1) sendMessage has no AbortController and no conversation guard, while newConv/switchConv only clear messagesEl — so a proposal card from an abandoned turn is appended into a different conversation and remains clickable. (2) The question event is emitted mid-stream (handleToolExecuted), so the option chips render while sending===true; the chip handler does card.remove() then sendMessage(o), and sendMessage bails at line 467 — the card is destroyed, nothing is sent, and the clarification is unrecoverable. (3) The Enter handler clears the textarea before sendMessage's sending guard rejects, so a typed follow-up vanishes with no toast.

*修复：* Capture const myConv = conversationId at the top of sendMessage and drop non-matching events (aborting the fetch in newConv/switchConv); make sendMessage return a boolean and only remove the question card / clear the textarea when the send was accepted.

<sub>合并自 3 条独立发现：frontend/cards-leak-across-conversation-switch, frontend/askuser-answer-dropped-while-streaming, frontend/typed-text-lost-when-sending</sub>

**23. ErpClient.login() holds the singleton monitor across a blocking 20s HTTP call and never re-checks the cached token**  
`src/main/java/com/xly/service/ErpClient.java:62` · concurrency-resources

token() tests the volatile cachedToken and then calls synchronized login(), which never re-checks the field on entry and performs http.send (20s request / 10s connect timeout) inside the monitor. Every ERP read and write without a pass-through user token funnels through it — which is exactly the dev-login configuration that is live today. With the ERP slow or down, N concurrent chats serialize N full /checklogin round-trips: the 10th user waits ~180s and blows past the SseEmitter timeout with no output at all, and because cachedToken stays null on failure the condition never self-heals.

*修复：* Double-check cachedToken inside the synchronized block and use a ReentrantLock with tryLock(timeout) so callers fail fast instead of queueing behind a dead backend.

**24. A transient DB failure on the first prompt render permanently caches a system prompt with no business-domain map**  
`src/main/java/com/xly/service/SystemPromptService.java:33` · agent-core

prompt() renders lazily into a volatile field with no TTL, invalidation or success check, and renderDomainMap() swallows any JDBC exception and returns '（业务域地图暂不可用）'. The first render happens on a live chat request (AgentFactory:82) — i.e. during the first post-deploy burst while the 20-connection Hikari pool is filling. One connection timeout there and every conversation for the JVM's lifetime runs without the 业务域地图 grounding, degrading form/domain selection with nothing but one WARN line to explain it.

*修复：* Only assign the field on a successful render (return the fallback without caching), or build the prompt eagerly at @PostConstruct with a scheduled refresh.

<sub>合并自 2 条独立发现：agent-core/system-prompt-caches-failed-domain-map, concurrency-resources/prompt-cache-poisoned-fallback</sub>

**25. Every exception returns HTTP 200, and the frontend renders that envelope as nothing or the literal string "undefined"**  
`src/main/java/com/xly/exception/GlobalExceptionHandler.java:37` · config-ops

All eight handlers on the @RestControllerAdvice return a bare Map with no @ResponseStatus/ResponseEntity, so failures — including 404s, malformed JSON and DataAccessException — go out as 200 with {code,message,timestamp}. Concrete chain: Redis down → ConversationService.touch throws before the SseEmitter is returned → 200 + JSON on a text/event-stream endpoint → chat.html's reader parses zero frames and then DELETES the whole message row (line 508), so the user sees their own message vanish and nothing else. On the confirm path a DataAccessException from ops.get (outside the try) yields {code,message}, and chat.html:574's `d.msg || d.status` paints the literal 'undefined' on a permanently disabled card. Monitoring and uptime probes record every outage as a success.

*修复：* Return ResponseEntity with the matching HTTP status from each handler; in chat.html check resp.ok and content-type before reading the stream, fall back to d.message, and render an explicit 响应中断 message when the stream ends without a done event.

<sub>合并自 4 条独立发现：config-ops/exception-handler-always-200, frontend/non-sse-response-silently-swallowed, frontend/confirm-result-renders-undefined, frontend/confirm-failure-locks-card</sub>

**26. Actuator health/metrics exposed with show-details: always and no security on the classpath**  
`src/main/resources/application.yml:62` · config-ops

exposure.include: health,info,metrics plus health.show-details: always with no spring-security and the permissive CorsFilter means `curl https://host/xlyAi/actuator/health` returns DataSource/Redis/diskSpace component details (DB product+version, absolute paths, free bytes) to anyone, and /actuator/metrics/http.server.requests enumerates every internal endpoint. Free reconnaissance plus a liveness oracle for the backing infrastructure, cross-origin readable.

*修复：* Set show-details to when-authorized or never, drop metrics from the exposure list, and bind actuator to a separate management port that nginx does not proxy.

**27. markdown-it is a hard dependency loaded from cdnjs with no SRI and no fallback, so a blocked CDN kills the entire page**  
`src/main/resources/templates/chat.html:7` · frontend

Line 349 calls window.markdownit({...}) at top-level script scope. If the CDN is unreachable — the norm for on-prem 印刷 ERP installs behind restricted egress — that line throws and aborts the whole classic script, so every handler bound afterwards (sendBtn:821, keydown:808, FK modal:796-799) and the bootstrap newConv() at 830 never run: a fully styled, completely dead page with no error shown. There is no integrity hash either, so a hijacked cdnjs response gets full control of the rendered messages and the 确认 buttons.

*修复：* Vendor markdown-it into src/main/resources/static and serve it same-origin; add an SRI hash and a typeof window.markdownit !== 'function' guard that degrades to textContent rendering.


### LOW

**28. Unbounded cached thread pool on the chat controller, never shut down, in a WAR deployed to a shared Tomcat**  
`src/main/java/com/xly/web/AgentChatController.java:77` · concurrency-resources

Executors.newCachedThreadPool() as a plain instance field with no @PreDestroy/shutdown anywhere in the tree. Each /chat request submits a task that blocks through the intent-gate and slot-extraction OkHttp calls (120s read timeout each) plus the ERP reads inside proposeWrite; because the controller returns an SseEmitter, Tomcat's own max-threads provides no back-pressure. With the Ollama host hung (a documented flake for this deployment) concurrent chats create unbounded non-daemon threads, and each WAR hot-redeploy leaves them running and pins the old webapp classloader.

*修复：* Use a bounded ThreadPoolExecutor (or a Spring ThreadPoolTaskExecutor bean, which the context shuts down) with a rejection policy that emits a 503-style SSE error, and add @PreDestroy shutdownNow().

<sub>合并自 3 条独立发现：agent-core/unbounded-executor-never-shutdown, concurrency-resources/unbounded-executor-no-shutdown, config-ops/unbounded-chat-executor</sub>

**29. Redis growth is unbounded: ledger TTL is refreshed on every append, the conversation index has no TTL, and appendTurn bypasses the message cap**  
`src/main/java/com/xly/service/LedgerService.java:52` · concurrency-resources

append() RPUSHes then re-expires, so an actively used conversation's ledger never ages out and has no LTRIM; events() LRANGEs 0..-1 and Jackson-parses all of it on every panel open with no pagination. chat:convs:{userId} is HSET with no TTL at all while the data it points at expires in 30 days, and chat.html calls newConv() on every page load — so the sidebar accumulates permanent 「新会话」 rows that open empty. RedisChatMemoryStore.appendTurn (used by every deterministic 新增/操作 route and by OpController.recordOutcome) skips ProjectedChatMemory's 80-message trimToCap entirely.

*修复：* LTRIM the ledger to a bounded window and paginate history(); give chat:convs the same 30-day TTL and create the meta lazily on first send; apply trimToCap inside appendTurn.

<sub>合并自 5 条独立发现：state-memory/ledger-unbounded, concurrency-resources/ledger-unbounded-list, state-memory/conv-index-never-expires, concurrency-resources/conv-meta-no-ttl, state-memory/appendturn-bypasses-hardcap</sub>

**30. SseEmitter registers no lifecycle callbacks, so an abandoned turn keeps running tools and writing to memory**  
`src/main/java/com/xly/web/AgentChatController.java:110` · concurrency-resources

No onTimeout/onCompletion/onError is registered and runAgentAttempt discards the handle from ts.start(), so nothing can cancel an in-flight generation when the user refreshes or closes the panel. Failed send() calls are swallowed silently in sendEvent, so the orphaned run keeps executing tools and still appends to the ledger and chat:mem for a dead turn — racing with the new turn's memory writes (rank 18) and queueing extra generations on a single Ollama instance.

*修复：* Register onCompletion/onTimeout/onError to set an aborted AtomicBoolean that the token/tool callbacks check and that suppresses ledger/memory writes; log the abandonment instead of swallowing it.

**31. Silent failure paths in the form pipeline: fkOptionPage swallows every exception, required flags are always false, and prefilled FK inputs cannot be cleared**  
`src/main/java/com/xly/service/FormResolverService.java:257` · form-pipeline

fkOptionPage wraps its whole body in catch (Exception ignore) {} with no logging and returns the pre-seeded empty page, which chat.html:782 renders as 「没有匹配的记录」 with HTTP 200 — a missing view, revoked grant or missing column is byte-identical to 'no rows', and because fkselect inputs are readOnly with no clear affordance the user cannot type a value or empty a wrong slot-fill prefill either, making the form permanently unsubmittable. FormCollectTool:86 hardcodes required=false for every dictionary-derived field, so the frontend's red * and its submit check are dead code and ProposeWriteTool.requiredCols silently default-fills the missing NOT NULL business columns.

*修复：* Log the exception with table/field context and return an explicit error field the UI can distinguish; add a clear (x) control to fkselect; populate required from the column's NOT NULL / bNotEmpty metadata in the businessFields branch.

<sub>合并自 4 条独立发现：form-pipeline/fkoptions-swallow-exception, config-ops/swallowed-fk-option-errors, form-pipeline/fkselect-prefill-uncleanable, frontend/required-flag-never-set</sub>

**32. Anti-fabrication and clarification guards are mutually exclusive, disabled on gate failure, and leave the fabricated answer in memory**  
`src/main/java/com/xly/web/AgentChatController.java:345` · agent-core

The digit guard runs only when queryGuard is true and the WRITE_CLAIM guard only when it is false, so a write misclassified as 查询 that answers 「已为您把电话修改为…」 with no proposal gets no correction. Any LlmJsonClient failure (it returns null for every failure mode — HTTP error, timeout, non-JSON) degrades to intent=其他, which runs with queryGuard=false, so the anti-fabrication check is off precisely when the model backend is flaky. And because langchain4j commits the AiMessage before onCompleteResponse, the fabricated answer stays in chat:mem even after the retry — the ledger shows only the corrected figure.

*修复：* Run the WRITE_CLAIM guard unconditionally (it is already predicated on !proposed); distinguish 'classified as 其他' from 'classification failed' and treat failure as queryGuard=true; roll the offending AiMessage out of the store before retrying, and strip ``` fences / <think> blocks before parsing in LlmJsonClient.

<sub>合并自 4 条独立发现：agent-core/guards-mutually-exclusive, agent-core/intent-gate-failure-drops-antifab-guard, state-memory/fabricated-answer-stays-in-memory, agent-core/llmjson-no-tolerant-parse</sub>

**33. The tool-driven proposal path stores the whole proposal sentence in the state slot's record field and echoes ERP text back into the intent gate**  
`src/main/java/com/xly/web/AgentChatController.java:548` · state-memory

state.setActiveDoc(convId, "", summary, opId, "proposed") puts a full sentence where StateService documents a record name/单号, and leaves entity blank — unlike the two deterministic call sites at lines 194 and 451 which pass a real record name. StateService.digest then renders 在办单据=【将【宏达纸品】的【电话】由「1」改为「2」】 and prefixes that to every later classify/extractWrite call, whose schema requires a record — breaking the very cross-turn reference the slot exists to resolve. The same string carries ERP-controlled text (customer names, old field values) verbatim into the intent gate's input with no escaping or length cap, a prompt-injection surface the gate did not otherwise have.

*修复：* Have ProposeWriteTool return the located entity type and record name in its JSON and pass those into setActiveDoc, keeping the summary in the ledger only; strip control characters and cap the length of any ERP-sourced text that reaches a prompt.

<sub>合并自 4 条独立发现：state-memory/activedoc-record-set-to-summary, agent-core/tool-path-state-record-is-summary, agent-core/erp-text-into-classifier-input, state-memory/entities-cap-not-enforced</sub>

**34. Documentation asserts security controls and an architecture that no longer exist**  
`docs/agent-architecture.md:7` · docs-drift

The reading guide sends readers to §17/§18 as current state, but §17 lists 9 @Tool methods including QueryTool.queryData, KgQueryTool.kgSearch and SkillTool.loadSkill and a ToolScope narrowing mechanism — none of which exist (there are exactly 6 @Tool methods and AgentFactory.build takes no scope); it also claims the work is unmerged on kg-edge-flow. Worse, docs/security-findings.md:39/42 says findings 3-5 are mitigated by 「新 queryData 已按此实现」 — a SQL sandbox that was deleted, so a reviewer signing off the security report concludes ad-hoc SQL is safely sandboxed when neither the tool nor any of its listed controls (SELECT-only, forced LIMIT, tenant AST injection) exists. FormController:19 and §14 similarly claim tenant filtering that rank 4 disproves. legacy-tool-inventory.md still says the 64 legacy tools load at startup.

*修复：* Point the reading guide at §19, mark §17/§18 superseded, rewrite security-findings 3-5 as 'closed by removal of the NL2SQL stack' (or as requirements for any future SQL tool), and correct the tenant-filtering claims to match the code.

<sub>合并自 4 条独立发现：docs-drift/arch-reading-guide-points-at-obsolete-section, docs-drift/secfindings-queryData-gone, docs-drift/legacy-inventory-stale-progress, docs-drift/kg-views-orphaned</sub>

**35. Dead no-token ErpClient overloads, an undefined logback appender, and inert config keys**  
`src/main/java/com/xly/service/ErpClient.java:109` · docs-drift

Four ErpClient overloads (readForm:109, updateForm:161, deleteForm:175, newUuid:313) and two IntentService overloads have zero callers; the ErpClient ones are the shorter, more obvious signatures and resolve the token to null, i.e. they are foot-guns that silently execute as dev-admin the moment someone calls them. logback-spring.xml:17-19 declares <root level="DEBUGE"><appender-ref ref="DB"/></root> with no DB appender defined, so Joran prints a configuration ERROR and dumps its status report on every start and every 60s rescan, masking real problems during deploy triage; it also configures loggers for org.mybatis and ai.djl, both removed from the POM. application.yml:47's block-when-exhausted binds to nothing. KgQueryTool:27 advertises '最多返回 15 条' while the SQL is LIMIT 12, with no truncation marker.

*修复：* Delete the six dead overloads, lines 17-19 of logback-spring.xml plus the mybatis/djl loggers and the unused console appender, and the orphan Redis key; derive the findForms limit and its tool description from one constant and emit a truncation flag.

<sub>合并自 5 条独立发现：docs-drift/erpclient-dead-overloads, docs-drift/logback-retired-stack-leftovers, docs-drift/orphan-redis-config-key, docs-drift/findforms-limit-mismatch, config-ops/logback-dead-root-block</sub>


## 本次审计未覆盖 / 无法定论

- No runtime or network access, so several impact ceilings are unproven: whether the target ERP actually accepts admin/666666 for brand 1111111111 (bounds rank 1's ERP-read half, though the JDBC-backed endpoints need no ERP login), whether nginx-saas in front of /xlyAi enforces an ERP session (this single unknown moves every unauthenticated finding between 'internet-exposed' and 'internal-only'), and whether port 3318 accepts arbitrary internet connections (rank 3).

- xlyEntry's own handlers were never read — they are out of repo. Unsettled: whether getBusinessDataByFormcustomId binds query parameters into stored-procedure IN params (this decides whether the URL injection at rank 8 escalates from 'reach any ERP endpoint' to 'bUpdate=1 write via the read tool'); whether /ai/execStaging marks the row before doing work (decides the exec-staging replay); whether addUpdateDelBusinessData rejects unknown columns, re-mints sBillNo, or applies business-required validation (decides ranks 13, 19 and the required-flag outcome); and whether quoquotationmaster has a unique index on (sBrandsId, sBillNo).

- No database access, so three findings could not be settled and are excluded or downgraded: (a) whether gdsmodule/gdsconfigformmaster carry sBrandsId — SHOW COLUMNS FROM gdsmodule LIKE 'sBrandsId' would decide whether the KG catalog views leak other tenants' form titles and table names via findForms (the reported kg-views-no-brand-scope finding is dropped as unproven); (b) whether any table has two distinct columns sharing a shortest Chinese label, which is what makes the label-keyed /form/submit protocol lossy — real in mechanism but with no verified instance, so it is not in the ranked list; (c) whether every sFkTable actually has sBrandsId and a resolvable name column, which decides how often the silent-empty FK picker fires.

- No LLM access, so all model-behaviour-dependent claims are stated as mechanisms rather than outcomes: gate accuracy (how often a write lands on the 查询 branch), whether Ollama's /v1 shim tolerates an orphan assistant tool_calls message, whether the model self-answers its own askUser call (askUser uses the default ReturnBehavior.TO_LLM, and the obvious IMMEDIATE fix is illegal on a TokenStream-returning service), and whether the intent extractor copies a whole sentence out of the corrupted state slot. Related: LlmJsonClient's strict parse is only a live outage if the configured endpoint returns fenced or thinking-prefixed content or 400s on reasoning_effort — one POST against 112.82.245.194:41434 would settle it.

- Files/flows nobody traced end to end: AuditService and ai_audit_log were only read as writers — retention, PII content and whether the audit table is itself queryable cross-tenant were not examined; TracingChatModelListener's Langfuse export was checked only for being gated and non-blocking, not for WHAT it exports (if it ships prompts containing ERP row data to a third party once keys are configured, that is an undiscovered data-egress path); SlotFillService's role tagging was read only at its FormCollectTool boundary, so mis-roled prefills were not traced from the gate output; and the build/deploy surface (the packaged WAR under target/, the nginx-saas config, the xlyEntry deploy scripts) is outside the repo and unaudited.

- git history was checked only for application.yml. Other credentials, tokens or IPs committed and later removed in the two-commit history were not swept, and no secret-scanning tool was run.

- There are no tests of any kind in this repo, so none of the 35 entries above is regression-guarded. Every merged fix — particularly the confirm compare-and-set, the coerce/summary alignment, and bringing doUpdate to parity with doCreate on FK/coerce/system-column handling — will need its own verification path built from scratch; there is no existing harness to extend.