fix(assets): 企业归属重算不再被单行非法 ip 打挂,并明确告知是哪些资产
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 均通过。
This commit is contained in:
1 parent
fea78b2d6f
commit
9e98407528
9 files changed
+151
-17
No files matched your search
+98
-12
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
-- =====================================================================
|
||||
|
||||
@@ -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
|
||||
|
||||
+1
-1
@@ -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
|
||||
|
||||
Reference in new issue
Block a user