Skip to content

fix(public-safety): reject non-string mapping field names - #5109

Merged
huangruiteng merged 3 commits into
loopx-project:mainfrom
jackie-cqz:codex/fix-public-safety-field-keys
Sep 26, 2026
Merged

huangruiteng merged 3 commits into
loopx-project:mainfrom
jackie-cqz:codex/fix-public-safety-field-keys

Conversation

@jackie-cqz

Copy link
Copy Markdown
Contributor

Goal and source

Fixes #5069. Structured public-output field names must retain their JSON string identity before public-safety classification. Previously str(key) turned bytes keys into repr text, allowing b"raw" and b"api_key" through the guard.

Change

  • Reject every non-string mapping key before field-name or value inspection, with a type-specific error that does not print the key.
  • Preserve the existing normalization and verdicts for string keys.
  • State the string-key contract in the public/private boundary guide.

The owning boundary remains loopx/control_plane/runtime/public_safety.py. No frontend, Lark, or CLI interaction changes are needed because their projected JSON field names are strings. A related refactor of other text compaction helpers was unnecessary: their behavior is separate from this validator.

Validation

  • Confirmed five new negative cases failed before the fix and pass after it, including b"raw", b"api_key", benign non-string keys, and a nested key.
  • Public-safety tests: 57 passed.
  • Reliability-diagnostics tests: 137 passed.
  • Ruff check and format check passed; git diff --check passed.
  • loopx check scanned the three changed public files: 0 errors, 2 expected warnings because the isolated checkout has no local Goal registry.

The original report found no current caller supplying non-string keys, so this closes the boundary-consistency defect without claiming an observed data leak.

Signed-off-by: jackie-cqz <2557911191@qq.com>
Signed-off-by: jackie-cqz <2557911191@qq.com>
Signed-off-by: jackie-cqz <2557911191@qq.com>
@jackie-cqz
jackie-cqz force-pushed the codex/fix-public-safety-field-keys branch from a9e47a5 to 867c172 Compare September 26, 2026 14:29

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

动机

#5069 指出的不是一个已观察到的凭据泄露事件,而是通用 public-safety 边界存在不一致:把 bytes key 直接 str() 后,raw/api_key 变成 bytes 的显示形式,不能可靠代表调用者提交的字段名。本 PR 在现有 owner 中明确 string-only 字段契约,关闭这条误分类路径,且不把 JSON 合法字符串输入一并拒绝。

改动思路

与“隐式解码 bytes”相比,拒绝所有非字符串 key 更符合现有结构化公开输出的边界,也避免增加编码选择、解码失败及不同 key 碰撞的第二套规则。检查放在每个 Mapping entry 分类之前,递归仍走原函数。没有新增 provider、持久状态、配置开关或授权来源;公开 JSON 的正常路径不需要迁移,也不要求前端/Lark 增加一个设置入口。

具体改动

完整 diff 为一个生产文件、一份文档及一个现有测试文件,共 30 additions / 4 deletions;生产部分仅 9 additions / 4 deletions。测试覆盖 bytes/raw、bytes/api_key、benign bytes、整数和嵌套容器;文档明确以前的转换方式为何不安全。

关键代码讲解

  1. validate_public_safe_value,L88:L103 在生成 item_path、检查值或递归之前验证 key 是字符串,否则只报告容器路径和类型。bytes、整数、bool、None、tuple、UserDict 和嵌套 list/tuple 不能绕过去。我的独立对象反例也确认 head 不调用非字符串 key 的 str,因此诊断没有把 key 的原始内容展示出来。

  2. normalize_public_safe_field_name,L74:合法字符串仍进入原来的大小写、分隔符、camelCase 归一化,再由现有 credential/raw family 判断;本 PR 没有复制或者扩大这些规则。tokenCount、keyId、secretLevel 等合法字段继续接受,rawOutput、apiKey、reviewToken 的原诊断保留。归一化 helper 本身未变,string-only 承诺的 owner 是实际 recursive validator。

我还追踪了实际调用者:observer envelope、governed capability result、team-plan admission、acceptance/artifact observation 都使用这个 guard;没有新增平行 sanitizer。真实 team-plan 路径中,非法嵌套 bytes key 在任何写入前得到 ValueError。纠正为合法 proposal 后通过真实 typed planning 和 Markdown IO 创建一个 Todo,再执行返回 reused,readback 仍只有一个 Todo。这个验证没有让 mock 提供期望的持久结果。

对主干的风险

确实有一个刻意的默认输入变化:以前部分非字符串 key 会被转换后接受,现在会拒绝,即使 key 的内容本身看似无害。不能称它为完全字节等价;直接传 Python Mapping 的调用者应使用字符串 key。Issue 接受这一方向,文档 L148 已披露。诊断是强制拒绝,不是 advisory guidance;措辞只谈容器与字段类型,没有产品、benchmark 或 actor 生命周期暗示。

语义与 CI 对齐

同一份独立契约 probe 在 immutable base 和本 head 运行了 31 个输入检查及真实拒绝→纠正→重放过程:base 13 个输入反例不满足契约,head 全通过;合法字符串、原 credential/raw 拒绝诊断及纠正后 readback 保持。key/value 同时非法时,head 明确优先报告字段类型。共享 guard 的相关测试 242 passed,team-plan/Chat/Lark 确认与真实 authority 路径补充 38 passed。全范围 Ruff、docs governance、maintainability、semantic vocabulary 和标准 canary 最终 5 direct + 14 selected 全通过,公开边界扫描 clean。

额外 UTF-8 检查在 base/head 都同样 1 failed / 3 passed,完整 offenders 均为 loopx/self_update.py:1232 与 loopx/windows_install.py:331;它们及对应测试在本 diff 中未变。这是 pre_existing_unrelated,独立于字段类型修复,不据此 Request Changes。首次 canary 缺 npm 依赖的环境问题已恢复并重新完整运行。没有读取或等待远端 CI;合并门禁与此代码评审结论分开。最新 main 合入 #5068 后新增的 peer-route guard 调用也已核对:候选由 resolver 构造 thread_id/host_surface 两个字符串字段,三个合法/拒绝输入的 base/head 对照一致。

我的整体评价

APPROVE 当前完整 head,没有新的 actionable blocker。long_horizon 保留合法调用的连续执行、无副作用拒绝和纠正后可重放;user_experience 改善为可定位到具体容器和 key 类型的错误。future-facing pass 已检查 validator/normalizer 的边界:一处类型检查是这里最小且可逆的修复,不需要追加通用 schema 框架或另一个 Python/TS 决策 owner。可验证的范围是调用此 guard 的 Mapping,不声称审计了所有外部 provider 或所有 Python 对象攻击面。已有 UTF-8 红检查的恢复及最终合并资格仍由各自 owner 处理。

English verdict: APPROVE - reviewed exact head 867c172; string-only field-name change is intentional, disclosed, and independently validated.

@huangruiteng
huangruiteng merged commit a71b869 into loopx-project:main Sep 26, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: a bytes field name is classified through its repr, so it can escape the public-safety rules

2 participants