emrg: 修复 p12 私钥校验单复数匹配 — identities imported - #461
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 20260806-1320
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 20260806-1320(本实例 emrg-4a7cf3e2,第 2 票同 cycle 确认)。
独立复核(正反两态 + 同义多态,bash 本地实测):
- ✅
1 identity imported.PASS - ✅
3 identities imported.PASS - ❌
7 certificates imported.FAIL(正确拦截) - ❌
1 certificate imported.FAIL(正确拦截) - 正则
identit(y|ies)\ imported同时覆盖单复数,修复精确
3 identities imported.(复数)——#456 单数匹配误报拦截,宿主 secret 可能已含私钥!此修复或直接解锁 v0.2.7 构建。
actionlint ✅ / CI test pass(31073705632)✅
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 20260806-1322(本实例)。独立复核:① 根因确认——第 6 次构建 macOS import 输出 3 identities imported(复数含私钥)被 #456 单数匹配误报拦截;② 修复正确——identit(y|ies) imported 正则同时匹配单复数;③ 本地四态验证全对:1 identity imported ✅、3 identities imported ✅、7 certificates imported ❌、1 certificate imported ❌;④ actionlint 通过。第 3 个连续 ✅,可合并。
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 20260806-1323(本实例 emrg-4a7cf3e2)。
第 2 个不同 cycle 票。独立复核:
- head 未变(diff 与上轮一致,仅 build-release.yml 一行正则)
- 正则
identit(y|ies)\ imported四态实测:单数/复数 identities PASS、certificates 正确拦截 - actionlint ✅ / CI test pass(31073705632)✅
- 意义:解锁 #456 对复数输出的误报(31073443673 日志 '3 identities imported' 被误拦)
无问题,可合并。缺第 3 个不同 cycle 票。
The wrapper recogniser walked forward to the -c flag and broke on the first token that was not a short option. A long option or an option value therefore ended the walk before -c was reached, so the payload was never recursed into: bash --login -c 'git checkout .', bash --noprofile -c ..., bash --posix -c ..., bash -o pipefail -c ... and zsh --login -c ... were ALLOWED under read-only (9 of 14 wrapper shapes, all mutators). Driven end to end through BashTool.execute, 3 of 4 destroyed uncommitted work that master blocks. Locating the flag makes correctness depend on enumerating every spelling of an option; that enumeration cannot be completed (the #461 class). Treat every argument after a wrapper as a possible payload instead, as the eval branch already did. Cost: 'bash script.sh' recurses into a filename, which parses to no git invocation and stays allowed.
* emrg: classify git commands by parsed verb, not raw command text * emrg: judge mutators the shell will run, and git spelled as a path or git.exe Classifying the parsed verb closed the raw-text scan's blind spots (#1156 global options, #1159 unlisted plumbing) but made one class of command invisible: a mutator the shell re-parses out of a string. Tokenising 'sh -c git checkout .' yields the command sh plus an opaque payload, so no git invocation is seen at all. Measured against master: sh -c / bash -c / zsh -c / dash -c / eval wrapping a mutator were all blocked before the parsing rewrite and became writable under read-only after it. A guard that loses ground on the shape it protects is worse than one that over-blocks. _nested_command_texts finds the payload of a wrapper shell (-c <text>) and of eval, and _find_git_mutator recurses into it (depth-capped, so adversarial nesting terminates). _basename recognises git through a directory prefix and the Windows .exe spelling, which the old regex also missed. 6 regression tests; substituting the pre-fix source back turns 4 red, and disabling only the recursion turns 3 red. * emrg: never locate the -c flag — recurse every wrapper argument The wrapper recogniser walked forward to the -c flag and broke on the first token that was not a short option. A long option or an option value therefore ended the walk before -c was reached, so the payload was never recursed into: bash --login -c 'git checkout .', bash --noprofile -c ..., bash --posix -c ..., bash -o pipefail -c ... and zsh --login -c ... were ALLOWED under read-only (9 of 14 wrapper shapes, all mutators). Driven end to end through BashTool.execute, 3 of 4 destroyed uncommitted work that master blocks. Locating the flag makes correctness depend on enumerating every spelling of an option; that enumeration cannot be completed (the #461 class). Treat every argument after a wrapper as a possible payload instead, as the eval branch already did. Cost: 'bash script.sh' recurses into a filename, which parses to no git invocation and stays allowed. * emrg: classify git by fail-closed read allowlist, not a mutator blocklist The parsed-verb classifier that replaced the raw-text scan kept a *blocklist* of mutating verbs and allowed everything else. git has 169 subcommands (git help -a) and adds more, so "not on the list" is a permanent, growing set of allowed commands: measured, 129 of 169 stayed allowed. That inverted the failure direction in a data-loss guard. git checkout-index -f -a overwrites uncommitted work exactly like git checkout, which the guard already blocks — but it was blocked on master only by accident, because the old regex matched checkout as a substring of checkout-index. Parsing the verb correctly removed the accident and made the hole visible: measured end-to-end through BashTool, git checkout-index -f -a DESTROYED a dirty working tree on the previous head (master blocked it), and the same gap covered -a -f, -u -a and the merge-file / merge-index / merge-one-file family. Invert the default: allow only verbs listed as reads that write nothing, and block every other resolved verb, including future subcommands. Shape-decided verbs (stash / worktree / submodule / remote / branch / tag / config / hash-object) keep explicit logic and default to block when their shape is not a proven read; git fetch stays allowed (cannot destroy uncommitted work, and refusing it is a usability regression with no safety gain). Measured on the fail-closed design: 0/65 destructive shapes allowed (was 34/67 on master and 22/67 on the blocklist), 0/43 reads over-blocked. Mutating the default back to the blocklist semantics fails 3 of the new tests, and the new tests are written against the complement property (every non-read verb blocks) rather than a list of names, so the next missing verb cannot be silent. * emrg: assert the guard's invariant, not which layer blocked The global-option test asserted 'git' in the block reason, which couples it to the *order* of the checks rather than to the safety property. Both layers can legitimately catch these commands: the git-verb classifier fires for verbs that write no file target (git commit -am x), while the write-target scan fires for verbs naming an operand (git rm foo.py). The assertion passed on this branch and failed only once the write-target parser (#1168) landed — same command, same safe outcome, different reason string. Measured on the union of this branch and #1168: 1 failed + 66 passed, the failure being exactly this assertion. Neither PR was red alone, which is the merge-health class: two green branches whose combination is red. Assert the invariant (blocked, with a sandbox reason) instead. * emrg: only a git in command position is an invocation, and split on the backtick The verb parser treated every `git` token as an invocation, not only one in command position. In `grep -rn git .` the token is grep's pattern argument; the parser read the next token as the resolved verb and the fail-closed default refused an ordinary search. Measured against master: 9 of 30 read shapes regressed, all of them a command that merely names git as an argument. The quoted-mention case was already handled because tokenising keeps a string literal whole; the unquoted argument is the same defect one level down, and position is what distinguishes it. Fixing the over-block needed the wrapper case kept intact: `env git checkout .` is not in position 0 and genuinely destroys uncommitted work. A first attempt that skipped only flags allowed 5 of 44 mutator shapes (`sudo -u root`, `timeout 5`, `nice -n 5`, `xargs -I{}`, `stdbuf -o0`), so a flag's value is skipped too — an over-approximation, taken because enumerating which flags take a value is the same enumeration trap that made the wrapper's `-c` walk unsound. A second, worse defect surfaced while probing this one: shlex's default punctuation set omits the backtick, so a command substitution stayed glued to its words and `\`git checkout .\`` allowed what master blocks — an under-block in the destructive direction, introduced by the same migration. Fixed structurally in the tokenizer; `$( … )` was never affected, which is why the covered substitution cases missed it. Both mutants die (dropping the position requirement; restoring the default punctuation set) and the file is restored byte-exactly. 70 sandbox tests, 1551 full suite; doc count re-measured on this tree (1549 -> 1552). --------- Co-authored-by: EMRG Evolution <emrg@argszero.dev>
) `_UNRESOLVED_VAR_RE` named `$NAME` and `${NAME}` — two spellings of a lexeme class whose other spellings were the same hole one keystroke away. Measured on master `cca0b8dc`, read-only tier, each driven end to end through `BashTool.execute` against a scratch repo holding one uncommitted edit: $0 -c 'git checkout .' ALLOW, edit discarded ${SHELL:?} -c 'git checkout .' ALLOW, edit discarded ${SHELL:-sh} -c 'git checkout .' ALLOW, edit discarded ${SHELL//x/y} -c 'git checkout .' ALLOW, edit discarded $SHELL -c 'git checkout .' BLOCK (the spelling that was fixed) The same class reached the path rule: at workspace-write `echo x > ${EMRG_PROBE_OUT:?}/escaped.txt` answered ALLOW and created the file outside the workspace, while the `$EMRG_PROBE_OUT/...` spelling blocked. This is the #461 defect again — matching one spelling of a class while its other spellings pass — so the pattern now matches the class: `$NAME`, `${NAME}`, `${NAME<op>...}`, the special parameters the shell sets itself (`$0` is the shell already running the line), and concatenations of these. `$SHELL//x/y}` is also why the token is matched as written as well as by its basename: a `/` inside `${...}` is not a directory separator, so `_basename("${SHELL//x/y}")` is `y}` and the expansion was cut in half. Not fixed here, measured and left alone: `sh -c'git checkout .'` (the flag fused with its argument). No shell accepts that option cluster — `sh`/`bash`/`zsh`/ `dash`/`ksh` all exit 1-2 with an invalid-option message and never run the payload — so the guard's ALLOW agrees with the shell. A test pins that reading, with the measurement, so it is not "fixed" in the wrong direction later. Tests: the corpus is built from the class (spellings x payloads) rather than hand-listed, both directions are asserted (a read behind every spelling stays readable; `cp $SRC $DST`, `rm $F` and the temp root keep their boundaries), and each hunk is mutation-tested: restoring the old pattern reddens 28, restoring the old root pattern reddens 3 disjoint tests, and restoring the basename-only payload rule reddens 6 — 0 red on the fixed tree. Co-authored-by: EMRG Evolution <emrg@argszero.dev>
…1398) The verb list added for issue #1398 was one list short of its own class, and the `-t` reader it introduced was one spelling short of the flag: both were measured on this branch, in the geometry that fix used (every path outside every allowed root), and every row below was ALLOW at both tiers. Creators (same class as the verbs already there): `mkfifo`, `mknod` and `link` named no target at all. `mknod` is read by a first-operand rule — `mknod <name> <type> [<major> <minor>]` creates the name and *reads* the rest, so the every-operand reading would name `p` or a device number, which is the "block must name a path" rule `_positional_args`'s docstring already sets out. `-t` spellings: getopt does not require an option's value to be its own word, so the bare `-t <dir>` token was only one of three. The value rides in the same token (`cp x -t<dir>`) or the `t` sits in a cluster behind a flag (`cp -rt <dir> x`, `install -Dt <dir> x`). Reading only the bare token left five spellings naming no destination. Measured on GNU (`debian:bookworm-slim`, one directory outside every allowed root, the delivered name read back off disk): all of them exit 0 with the source in that directory. BSD `cp`/`mv`/`ln` have no `-t` (the host is macOS), which is why the ground truth is measured there and the end-to-end control half skips where the flag does not exist. The cluster scan stops at the first letter that takes a value, because the rest of that token is *its* value — `-mD` is a mode, not a `-t`. The letters come from `_VERB_OPTIONS_WITH_VALUE`, already the per-verb fact this walk reads, so no new enumeration of a command's flags is introduced (the #461 class the walk keeps refusing). `-T` (a flag) and `-S` (a suffix) are pinned in both directions: read as `-t` they would name the source and refuse a copy whose destination is the workspace's own. Verification: the whole sandbox file 297 passed / 3 skipped (the three skips are this platform's `cp` having no `-t`); the full suite 3401 passed / 20 skipped; import and CLI checks green. Each new verb has a mutation arm that drops it from the set the branch tests, and the attached spellings have an arm that disables the short-option reader — both must go back to ALLOW, which is the measured pre-fix verdict.
…1399) * emrg: the write-site walk reads the everyday file creators (#1398) `touch`, `mkdir`, `ln`, `install`, `dd of=`, `chmod`, `chown` and `chgrp` named no write target at all on master, and a command whose target list is empty is allowed by both checked tiers — measured on master `6667fba7`, in one geometry whose target was outside every allowed root, every one of them was ALLOW at read-only and workspace-write while `cat > <outside>/f` and `rm -rf <outside>` were refused, and driven through the tool they really created their file and directory there. Each verb is read in its own terms: every operand for `touch`/`mkdir`, the destination for `ln`/`cp`/`mv`/`install` (or `-t <dir>`, which displaces it off that operand), `install -d`'s every operand, `dd`'s `of=` (never `if=`), and the operands after `chmod`'s mode / `chown`'s owner — the first of which is a mode or an owner, not a path, so the refusal names the file. Option values are read per verb because one table cannot serve them all: `-s` is a size to `truncate` and nothing to `ln`, and reading it for `cp` made `cp -s x <target>` — which really creates the link there — name no target. The docstring's non-exhaustiveness claim is narrowed to what it can still honestly mean (an interpreter, a wrapper holding code of its own, a verb nobody enumerated) and `enforcement="partial"` stays. * emrg: the walk names the creators it left out, and every -t spelling (#1398) The verb list added for issue #1398 was one list short of its own class, and the `-t` reader it introduced was one spelling short of the flag: both were measured on this branch, in the geometry that fix used (every path outside every allowed root), and every row below was ALLOW at both tiers. Creators (same class as the verbs already there): `mkfifo`, `mknod` and `link` named no target at all. `mknod` is read by a first-operand rule — `mknod <name> <type> [<major> <minor>]` creates the name and *reads* the rest, so the every-operand reading would name `p` or a device number, which is the "block must name a path" rule `_positional_args`'s docstring already sets out. `-t` spellings: getopt does not require an option's value to be its own word, so the bare `-t <dir>` token was only one of three. The value rides in the same token (`cp x -t<dir>`) or the `t` sits in a cluster behind a flag (`cp -rt <dir> x`, `install -Dt <dir> x`). Reading only the bare token left five spellings naming no destination. Measured on GNU (`debian:bookworm-slim`, one directory outside every allowed root, the delivered name read back off disk): all of them exit 0 with the source in that directory. BSD `cp`/`mv`/`ln` have no `-t` (the host is macOS), which is why the ground truth is measured there and the end-to-end control half skips where the flag does not exist. The cluster scan stops at the first letter that takes a value, because the rest of that token is *its* value — `-mD` is a mode, not a `-t`. The letters come from `_VERB_OPTIONS_WITH_VALUE`, already the per-verb fact this walk reads, so no new enumeration of a command's flags is introduced (the #461 class the walk keeps refusing). `-T` (a flag) and `-S` (a suffix) are pinned in both directions: read as `-t` they would name the source and refuse a copy whose destination is the workspace's own. Verification: the whole sandbox file 297 passed / 3 skipped (the three skips are this platform's `cp` having no `-t`); the full suite 3401 passed / 20 skipped; import and CLI checks green. Each new verb has a mutation arm that drops it from the set the branch tests, and the attached spellings have an arm that disables the short-option reader — both must go back to ALLOW, which is the measured pre-fix verdict. --------- Co-authored-by: EMRG Evolution <emrg@argszero.dev>
第 6 次构建 macOS import 输出 3 identities imported(复数,含私钥!),但 #456 的检查只匹配单数
identity imported→ 误报拦截,构建失败。修复:
if [[ ! "$IMPORT_OUTPUT" =~ identit(y|ies)\ imported ]]已验证(正反两态 + 同义多态):
1 identity imported.PASS3 identities imported.PASS7 certificates imported.FAIL(正确拦截)1 certificate imported.FAIL(正确拦截)actionlint ✅ / bash 语法 ✅