From 9e98407528a4eaeb3a4c745751a52ae48db1f3bd Mon Sep 17 00:00:00 2001 From: Autumn-27 <53844290+Autumn-27@users.noreply.github.com> Date: Mon, 24 Aug 2026 10:16:17 -0400 Subject: [PATCH] =?UTF-8?q?fix(assets):=20=E4=BC=81=E4=B8=9A=E5=BD=92?= =?UTF-8?q?=E5=B1=9E=E9=87=8D=E7=AE=97=E4=B8=8D=E5=86=8D=E8=A2=AB=E5=8D=95?= =?UTF-8?q?=E8=A1=8C=E9=9D=9E=E6=B3=95=20ip=20=E6=89=93=E6=8C=82,=E5=B9=B6?= =?UTF-8?q?=E6=98=8E=E7=A1=AE=E5=91=8A=E7=9F=A5=E6=98=AF=E5=93=AA=E4=BA=9B?= =?UTF-8?q?=E8=B5=84=E4=BA=A7?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit assets.ip 是自由文本(Agent 的 insert_assets 和资产 API 都可能写进主机名), 而 db/companies.go 的 IP/CIDR 归属匹配裸转 a.ip::inet。只要表里有一行不是 合法 IP,PostgreSQL 就抛 22P02 中断整条语句,连带同事务的新增/修改企业资产 范围、删除企业全部失败回滚。这个裸转从项目初始化提交就存在。 - schema.sql 新增 try_inet(text)：非法值返回 NULL 而不是抛错。没用 pg_input_is_valid 是因为那要 PG16+,这里要兼容更老的存量库。 - 四处 IP/CIDR 匹配统一改用 try_inet。原先 task_scope.go 和 task_assets_context.go 用的正则 a.ip !~ '[^0-9a-fA-F:.]' 只检查字符集, abc.def 这类全 hex 字母的主机名照样漏过去后炸,一并换掉。 - 跳过不能是静默的:重算会统计并抽样列出这些资产(id + 取值),说明它们不会 被网段规则归属。信息通过企业范围接口新增的 warnings 字段返回并在前端以 长停留 toast 展示,同时无条件写入服务端日志,覆盖没有前端响应的路径 (删除企业、scopesentry 同步、Agent 写入)。 - warnings 与 errors 分开:errors 仍只表示本次提交里不合法的行,既有数据 问题不该被记到用户这次输入头上。 验证: 干净库上 go test -p 1 ./... 仅剩 1 个既有失败(TestCompanyScopeInvalid, 与本改动无关,stash 后同样失败);改动前在含真实数据的库上有 14 个失败,现已 全部通过。go build / vet / gofmt / tsc / biome 均通过。 --- CHANGELOG.md | 8 ++ db/companies.go | 110 +++++++++++++++++--- db/schema.sql | 12 +++ db/task_assets_context.go | 4 +- db/task_scope.go | 2 +- server/assets.go | 7 ++ web/src/app/(main)/function/assets/page.tsx | 10 ++ web/src/lib/api.ts | 5 +- web/src/lib/types.ts | 10 ++ 9 files changed, 151 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee389dc..6cb56e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,14 @@ - 任务列表勾选多个任务后可批量修改分类，目标分类支持「未分类」以将任务移出当前分类；整批写入在一个事务内完成，失败项只会是勾选后已被删除的任务。 +### 资产 + +#### 修复的问题 + +- 修复企业归属重算被单行脏数据打挂：`assets.ip` 存了主机名（Agent 或资产 API 写入）时，`a.ip::inet` 会抛 `22P02`，导致新增/修改企业资产范围和删除企业全部失败并回滚。改用安全转换 `try_inet()`，非法值跳过而不是中断整条语句。 +- 无法解析的 `ip` 不再静默跳过：企业范围保存后会明确提示有多少条资产的 `ip` 不是合法 IP（含具体 id 和取值），提醒这些资产不会被 IP/CIDR 规则归属；同一信息也写入服务端日志，覆盖没有前端响应的路径（删除企业、scopesentry 同步、Agent 写入）。 +- 任务测试范围的 IP/CIDR 匹配同步改用 `try_inet()`，替换原先只检查字符集的正则守卫（`abc.def` 这类全 hex 字母的主机名此前仍会漏过并触发同样的报错）。 + ## [0.3.3] - 2026-08-23 ### Worker diff --git a/db/companies.go b/db/companies.go index e360002..d2d5745 100644 --- a/db/companies.go +++ b/db/companies.go @@ -4,6 +4,7 @@ import ( "database/sql" "errors" "fmt" + "log" "net" "strings" "unicode/utf8" @@ -161,9 +162,11 @@ RETURNING id`, name, nkey, logoVal).Scan(&id); err != nil { return 0, 0, 0, invalid, validationErrors, err } if needsAttribution { - if err := recomputeAttributionTx(tx); err != nil { + warning, err := recomputeAttributionTx(tx) + if err != nil { return 0, 0, 0, invalid, validationErrors, err } + logAttributionWarning(warning) } if err := tx.Commit(); err != nil { return 0, 0, 0, invalid, validationErrors, err @@ -244,9 +247,13 @@ func (s *CompanyStore) DeleteCompanyWithAssets(id int64, deleteAssets bool) (ass if deleted == 0 { return 0, ErrCompanyNotFound } - if err := recomputeAttributionTx(tx); err != nil { + // This path has no per-request warning channel, so the log is the only place + // the operator can learn about unparseable ip rows here. + warning, err := recomputeAttributionTx(tx) + if err != nil { return 0, err } + logAttributionWarning(warning) if err := tx.Commit(); err != nil { return 0, err } @@ -370,9 +377,11 @@ func (s *CompanyStore) AddScopeInputsChecked(companyID int64, inputs []ScopeInpu return 0, 0, invalid, errors, err } if needsAttribution { - if err := recomputeAttributionTx(tx); err != nil { + warning, err := recomputeAttributionTx(tx) + if err != nil { return 0, 0, invalid, errors, fmt.Errorf("重新计算企业归属失败: %w", err) } + logAttributionWarning(warning) } if err := tx.Commit(); err != nil { return 0, 0, invalid, errors, err @@ -502,20 +511,26 @@ func (s *CompanyStore) RecomputeAttribution() error { if err := lockCompanyScopeMutation(tx); err != nil { return err } - if err := recomputeAttributionTx(tx); err != nil { + warning, err := recomputeAttributionTx(tx) + if err != nil { return err } + logAttributionWarning(warning) return tx.Commit() } -func recomputeAttributionTx(tx *sql.Tx) error { +// recomputeAttributionTx rebuilds scope-derived ownership. It returns a warning +// for assets whose ip column cannot be parsed: try_inet skips them instead of +// aborting the statement, so without this they would silently never receive a +// network-based company. Callers surface the warning and it is always logged. +func recomputeAttributionTx(tx *sql.Tx) (string, error) { // Only derived rows are cleared. Historical rows migrated without provenance // are marked explicit by schema.sql, which is the non-destructive default. if _, err := tx.Exec(` UPDATE assets SET company_id = NULL, company_source = 'scope' WHERE company_source = 'scope'`); err != nil { - return err + return "", err } // Domain-based attribution (root_domain exact match). @@ -533,7 +548,7 @@ UPDATE assets a SET company_id = matched.company_id, company_source = 'scope' FROM matched WHERE a.id = matched.asset_id`); err != nil { - return err + return "", err } // IP/CIDR attribution for still-unowned assets. @@ -541,7 +556,7 @@ WHERE a.id = matched.asset_id`); err != nil { WITH matched AS ( SELECT DISTINCT ON (a.id) a.id AS asset_id, cs.company_id FROM assets a - JOIN company_scope cs ON cs.kind IN ('ip','cidr') AND cs.net >>= a.ip::inet + JOIN company_scope cs ON cs.kind IN ('ip','cidr') AND cs.net >>= try_inet(a.ip) WHERE a.company_id IS NULL AND a.type IN ('ip','subdomain','service','endpoint') AND a.ip IS NOT NULL @@ -551,7 +566,7 @@ UPDATE assets a SET company_id = matched.company_id, company_source = 'scope' FROM matched WHERE a.id = matched.asset_id`); err != nil { - return err + return "", err } // Exact normalized ICP attribution after domain/network precedence. @@ -572,9 +587,78 @@ UPDATE assets a SET company_id = matched.company_id, company_source = 'scope' FROM matched WHERE a.id = matched.asset_id`); err != nil { - return err + return "", err } - return nil + return malformedIPAssetWarning(tx) +} + +// malformedIPAssetsSampled bounds how many offending ids one warning names, so a +// large batch of bad rows stays readable in a toast and in the log. +const malformedIPAssetsSampled = 5 + +// logAttributionWarning records a recompute warning in the server log. Every +// recompute path calls it, so the warning is reported even for triggers with no +// per-request response (company deletion, scopesentry sync, agent asset writes). +func logAttributionWarning(warning string) { + if warning != "" { + log.Printf("[assets] %s", warning) + } +} + +// malformedIPAssetQueryer is satisfied by both *sql.Tx and *DB so the warning +// can be produced inside a recompute transaction or read standalone by the API. +type malformedIPAssetQueryer interface { + Query(query string, args ...any) (*sql.Rows, error) +} + +// MalformedIPAssetWarning reports assets with an unparseable ip outside of any +// mutation, letting the API attach the warning to a scope response without +// widening the mutation signatures — an unrelated data problem is not one of +// this request's validation errors. +func (s *CompanyStore) MalformedIPAssetWarning() (string, error) { + return malformedIPAssetWarning(s.db) +} + +// malformedIPAssetWarning describes assets whose ip column is not a valid +// address. They are invisible to network attribution, so the operator has to be +// told which rows to fix — silently skipping them would look like scope rules +// that simply do not work. +func malformedIPAssetWarning(q malformedIPAssetQueryer) (string, error) { + rows, err := q.Query(` +SELECT id, ip, count(*) OVER () AS total +FROM assets +WHERE ip IS NOT NULL AND ip <> '' AND try_inet(ip) IS NULL + AND type IN ('ip','subdomain','service','endpoint') +ORDER BY id +LIMIT $1`, malformedIPAssetsSampled) + if err != nil { + return "", err + } + defer rows.Close() + var total int + samples := make([]string, 0, malformedIPAssetsSampled) + for rows.Next() { + var id int64 + var ip string + if err := rows.Scan(&id, &ip, &total); err != nil { + return "", err + } + samples = append(samples, fmt.Sprintf("#%d %s", id, ip)) + } + if err := rows.Err(); err != nil { + return "", err + } + if total == 0 { + return "", nil + } + warning := fmt.Sprintf( + "%d 条资产的 ip 字段不是合法 IP，已跳过 IP/CIDR 范围匹配（这些资产不会被网段规则归属到企业）：%s", + total, strings.Join(samples, "、"), + ) + if total > len(samples) { + warning += fmt.Sprintf(" 等 %d 条", total) + } + return warning, nil } // UpdateScope replaces all scope rules for a company and reattributes. @@ -635,9 +719,11 @@ func (s *CompanyStore) UpdateScopeInputsChecked(companyID int64, inputs []ScopeI } // Rebuild even for an empty replacement because removing the old rules may // detach scope-derived assets or expose a lower-precedence company match. - if err := recomputeAttributionTx(tx); err != nil { + warning, err := recomputeAttributionTx(tx) + if err != nil { return 0, invalid, errs, fmt.Errorf("重新计算企业归属失败: %w", err) } + logAttributionWarning(warning) if err := tx.Commit(); err != nil { return 0, invalid, errs, err } diff --git a/db/schema.sql b/db/schema.sql index 59b25ec..4c64c3f 100644 --- a/db/schema.sql +++ b/db/schema.sql @@ -8,6 +8,18 @@ CREATE OR REPLACE FUNCTION set_updated_at() RETURNS trigger AS $$ BEGIN NEW.updated_at = now(); RETURN NEW; END; $$ LANGUAGE plpgsql; +-- 安全的 text→inet 转换：非法值返回 NULL 而不是抛 22P02。assets.ip 是自由文本 +-- (Agent / 资产 API 可能写进主机名)，裸转 a.ip::inet 会让单独一行脏数据把整条 +-- 企业归属重算语句打挂。调用方用 try_inet(...) IS NULL 找出这些行并告警。 +-- 不用 pg_input_is_valid 是因为那要 PG16+，这里要兼容更老的存量库。 +CREATE OR REPLACE FUNCTION try_inet(value text) RETURNS inet AS $$ +BEGIN + RETURN value::inet; +EXCEPTION WHEN others THEN + RETURN NULL; +END; +$$ LANGUAGE plpgsql IMMUTABLE STRICT; + -- ===================================================================== -- A. 资产层：companies / assets / company_scope -- ===================================================================== diff --git a/db/task_assets_context.go b/db/task_assets_context.go index 7b08c0d..9d2c066 100644 --- a/db/task_assets_context.go +++ b/db/task_assets_context.go @@ -30,7 +30,7 @@ target AS ( (ts.kind='company' AND a.company_id = ts.company_id) OR (ts.kind='root_domain' AND a.root_domain = ts.domain) OR (ts.kind='subdomain' AND a.domain = ts.domain) - OR (ts.kind IN ('ip','cidr') AND a.ip IS NOT NULL AND a.ip !~ '[^0-9a-fA-F:.]' AND ts.net >>= a.ip::inet) + OR (ts.kind IN ('ip','cidr') AND ts.net >>= try_inet(a.ip)) OR (ts.kind='icp' AND ( lower(regexp_replace(COALESCE(a.icp,''), '[[:space:]]+', '', 'g')) = ts.value OR lower(regexp_replace(COALESCE(a.app_icp,''), '[[:space:]]+', '', 'g')) = ts.value @@ -180,7 +180,7 @@ context_assets AS ( (ts.kind='company' AND a.company_id=ts.company_id) OR (ts.kind='root_domain' AND a.root_domain=ts.domain) OR (ts.kind='subdomain' AND a.domain=ts.domain) - OR (ts.kind IN ('ip','cidr') AND a.ip IS NOT NULL AND a.ip !~ '[^0-9a-fA-F:.]' AND ts.net >>= a.ip::inet) + OR (ts.kind IN ('ip','cidr') AND ts.net >>= try_inet(a.ip)) OR (ts.kind='icp' AND ( lower(regexp_replace(COALESCE(a.icp,''), '[[:space:]]+', '', 'g')) = ts.value OR lower(regexp_replace(COALESCE(a.app_icp,''), '[[:space:]]+', '', 'g')) = ts.value diff --git a/db/task_scope.go b/db/task_scope.go index f3907f5..e93f79d 100644 --- a/db/task_scope.go +++ b/db/task_scope.go @@ -309,7 +309,7 @@ target AS ( (ts.kind='company' AND a.company_id = ts.company_id) OR (ts.kind='root_domain' AND a.root_domain = ts.domain) OR (ts.kind='subdomain' AND a.domain = ts.domain) - OR (ts.kind IN ('ip','cidr') AND a.ip IS NOT NULL AND a.ip !~ '[^0-9a-fA-F:.]' AND ts.net >>= a.ip::inet) + OR (ts.kind IN ('ip','cidr') AND ts.net >>= try_inet(a.ip)) OR (ts.kind='icp' AND ( lower(regexp_replace(COALESCE(a.icp,''), '[[:space:]]+', '', 'g')) = ts.value OR lower(regexp_replace(COALESCE(a.app_icp,''), '[[:space:]]+', '', 'g')) = ts.value diff --git a/server/assets.go b/server/assets.go index dc854b2..17d2b5a 100644 --- a/server/assets.go +++ b/server/assets.go @@ -230,6 +230,13 @@ func (s *Server) addCompanyScope(w http.ResponseWriter, r *http.Request) { if len(errs) > 0 { out["errors"] = errs } + // Reported separately from errors: this is pre-existing bad data, not a fault + // in the submitted rules, but the operator still has to see it or those assets + // look like the IP/CIDR rules simply never match. A failure to build the + // warning must not fail the scope write that already committed. + if warning, err := cs.MalformedIPAssetWarning(); err == nil && warning != "" { + out["warnings"] = []string{warning} + } writeJSON(w, 200, out) } diff --git a/web/src/app/(main)/function/assets/page.tsx b/web/src/app/(main)/function/assets/page.tsx index b9b1b76..bcc4438 100644 --- a/web/src/app/(main)/function/assets/page.tsx +++ b/web/src/app/(main)/function/assets/page.tsx @@ -1321,6 +1321,14 @@ function CompanyAvatar({ name, logo }: { name: string; logo?: string }) { ); } +// 后端返回的 warnings 说的是既有数据问题（不是本次提交的行有错），保存本身已经 +// 成功。给更长的停留时间，因为它需要用户去处理具体的资产，扫一眼标题不够。 +function showScopeWarnings(warnings?: string[]) { + for (const warning of warnings ?? []) { + toast.warning(warning, { duration: 15000 }); + } +} + function savedScopeRules(company: Company): CompanyScopeRule[] { return (company.scope ?? []) .map((scope) => ({ kind: scope.kind, value: scope.raw.trim() })) @@ -1442,6 +1450,7 @@ function EditScopeDialog({ company, onSaved }: { company: Company; onSaved: () = const errCount = res.invalid ?? 0; if (errCount > 0) toast.warning(`已保存；${errCount} 行无效`); else toast.success(`范围已更新，共 ${res.added} 条`); + showScopeWarnings(res.warnings); setOpen(false); onSaved(); } catch (e) { @@ -1528,6 +1537,7 @@ function AppendScopeDialog({ company, onSaved }: { company: Company; onSaved: () const errCount = res.invalid ?? 0; if (errCount > 0) toast.warning(`已保存；${errCount} 行无效`); else toast.success(`已追加 ${res.added} 条范围`); + showScopeWarnings(res.warnings); setOpen(false); onSaved(); } catch (e) { diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index aaceaff..706efb0 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -17,6 +17,7 @@ import type { ChatAttachment, CommandRecord, Company, + CompanyScopeMutation, CompanyScopeRule, Conversation, ConvTokenSummary, @@ -395,12 +396,12 @@ export const api = { }, ), addCompanyScope: (id: number, scope: CompanyScopeRule[], reason = "") => - post<{ added: number; skipped: number; invalid: number; errors?: string[] }>(`/companies/${id}/scope`, { + post(`/companies/${id}/scope`, { scope, reason, }), updateCompanyScope: (id: number, scope: CompanyScopeRule[], reason = "") => - post<{ added: number; skipped: number; invalid: number; errors?: string[] }>(`/companies/${id}/scope`, { + post(`/companies/${id}/scope`, { scope, reason, reset: true, diff --git a/web/src/lib/types.ts b/web/src/lib/types.ts index 87d8381..339e892 100644 --- a/web/src/lib/types.ts +++ b/web/src/lib/types.ts @@ -298,6 +298,16 @@ export interface CompanyScopeRule { value: string; } +// 资产范围写入的结果。errors 是本次提交里不合法的行；warnings 是与本次提交无关、 +// 但会让归属结果不符合预期的既有数据问题（如 ip 字段存了主机名的资产）。 +export interface CompanyScopeMutation { + added: number; + skipped: number; + invalid: number; + errors?: string[]; + warnings?: string[]; +} + // 公司资产范围规则的一条（归属唯一真值来源）。 export interface ScopeRow { id: number;