Tighten shell-guardrail 'provably clean' classification to fail-closed by default
还没有人认领这个 Issue。
评估
- 难度
- 5/5
- 预计耗时
- 一周以上
- 新手友好度
- 35/100
- Issue 类型
- 重构
- 描述清晰度
- 基本清楚
- 活跃度
- 冷清
- 技术栈
- typescript
调研方向
从 build/lib/check-legacy-owners.mjs 开始,阅读 resolveCaptureFlag、resolveHandlerNode、resolveGlobalKind/classifyGlobalDeclaration、resolvesToDocumentBody 和 hasCapturePropertyMutation。然后检查 tests/unit/shell-guardrails-arch.test.ts 及其 sabotage matrix。完成的标准是:analyzer 只接受那些有意保持狭窄且可以证明安全的形态,并将别名、突变和未解析的标识符归类为 uncheckable-*,同时更新测试预期。
由索引模型根据 Issue 内容生成。
描述
Context
Filed while shipping #592 (PR #672 — mechanical shell-primitive architecture guards in
build/check-boundaries.mjs / build/lib/check-legacy-owners.mjs).
Two formal ChatGPT pr-mode review sessions (6 total passes) each found and the
coordinator verified/fixed real defects in the new shell-body-mount and
shell-capture-escape analyzers. The first session's 3 passes were all variants of one
root cause — a hand-rolled scope-resolution layer approximating JS/TS binding semantics
— fixed by a structural restructuring to real TypeScript-checker-based symbol
resolution (checker.getSymbolAtLocation), independently confirmed sound by a second
internal review.
The second session's 3 passes (after the restructuring) were a different, adjacent
problem: proving a captured JS value/identifier is never mutated or aliased after its
own declaration. Each pass closed one concrete shape and the next pass found another:
let/varreassignment of a capture-options identifier or a Document/Window alias
(fixed forresolveCaptureFlag/resolveHandlerNode/the body resolver, then found
missing in the Document/Window resolver itself in the very next pass).- Direct property mutation on a
const-declared options object (opts.capture = true). - The SAME mutation via an alias of the original identifier (
const a = opts; a.capture = true), and via a compound assignment operator (opts.capture ||= true). - An unresolved
const ESC = 'Escape'alias for the Escape-comparison string literal. - A bracket-notation call spelling (
document['addEventListener'](...)) never
considered a candidate at all.
All of the above were fixed and are live on main as of PR #672's merge. But "does this
identifier's bound value ever change, through any alias, any operator, anywhere in
scope" is a full points-to/alias-analysis problem — genuinely open-ended. The
coordinator and the repo owner judged shipping the current state as the right call (real
adversarial code exploiting this is a much narrower threat than #592's actual concern —
an ordinary developer re-introducing a copy-pasted overlay lifecycle — and a mechanical
grep/AST-light architecture guard, per this repo's own stated design philosophy for
check-boundaries.mjs, was never going to fully close arbitrary adversarial JS), but the
gap is real and worth a deliberate follow-up decision rather than silent acceptance.
Proposal
Rather than continuing to chase individual mutation/aliasing shapes as they're found (the
pattern that produced 6 review passes), consider narrowing what the analyzer classifies
as "provably safe" to an intentionally small, easily-exhaustible set of syntactic
shapes — e.g., a bare literal directly in the call, or a const binding with a literal
initializer that a cheap same-file check proves is never re-referenced by any OTHER
identifier and never has any property-mutation expression (<name>.<prop> = ... in any
form, including compound operators) anywhere in the enclosing file. Everything else
(aliases, mutation of any kind, unresolved identifiers) fails closed as
uncheckable-options/uncheckable-handler — an unconditional violation requiring a
human-reviewed, explicit allowlist entry — rather than the analyzer trying to prove a
negative about ever-more-exotic mutation shapes.
This flips the maintenance burden: today, each new bypass shape discovered requires a
resolver code change to "understand" it; under the proposal, only the (much smaller,
finite) set of provably-safe shapes needs code, and everything else is conservatively
rejected by construction — closer to this repo's existing check-boundaries.mjs
philosophy elsewhere (frozen exact allowlists, fail loud rather than fail permissive).
Files
build/lib/check-legacy-owners.mjs—resolveCaptureFlag,resolveHandlerNode,
resolveGlobalKind/classifyGlobalDeclaration,resolvesToDocumentBody,
hasCapturePropertyMutation.tests/unit/shell-guardrails-arch.test.ts— the sabotage matrix would need updating to
match a narrower "provably safe" contract (many currently-passing-because-permissive
fixtures would flip to expectinguncheckable-*).
Why deferred
Out of scope for #592 itself (enforcement-only, and the current state already ships
real, valuable protection against the actual regrowth pattern #586/#587 fixed); this is
a deliberate hardening decision for a human to schedule, not an emergency.
- 主要语言
- TypeScript
- 星标
- 8
- 派生
- 2
- PR 合并指标
- 30 天内没有已合并 PR
环境准备
- 提供 Dockerfile 或 Docker Compose 文件
- 有 Pull Request 模板
- 阅读贡献指南
从这里开始
- 先读完整个 Issue,再读项目的贡献指南。
- 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
- Fork 仓库,在一个分支上完成修改。
- 提交 Pull Request,并在描述里引用这个 Issue 编号。
Altinity/altinity-sql-browser 的其他 Issue
-
inbox
难度 2/5 1-3 小时 新手友好度 76/100
Altinity/altinity-sql-browser#605 ·
-
inbox
难度 2/5 1-3 小时 新手友好度 78/100
Altinity/altinity-sql-browser#509 ·
-
inbox
难度 2/5 1-3 小时 新手友好度 78/100
Altinity/altinity-sql-browser#489 ·
-
flamegraph未关闭enhancement
难度 5/5 一周以上 新手友好度 25/100
Altinity/altinity-sql-browser#684 ·
-
bug
难度 4/5 3-5 天 新手友好度 68/100
Altinity/altinity-sql-browser#680 · 2 条评论 ·
查看 Altinity/altinity-sql-browser 的全部 Issue
相似的 Issue
-
area:docs bug triage:confirmed
难度 2/5 1-3 小时 新手友好度 74/100
维护者通常 1 天内回复
-
难度 2/5 1-3 小时 新手友好度 65/100
anomalyco/models.dev#8862 · 1 条评论 ·
维护者通常 1 天内回复
-
fix(data-lake): wizard source step still previews the local slug, not the server-disambiguated one未关闭data-lake
难度 2/5 1-3 小时 新手友好度 82/100
维护者通常 1 天内回复
-
ready-for-triage
难度 1/5 1 小时以内 新手友好度 88/100
konflux-ci/konflux-ui#1596 · 1 条评论 ·
维护者通常 1 天内回复
-
enhancement good first issue priority: low size: XS
难度 2/5 1-3 小时 新手友好度 82/100
维护者通常 1 天内回复