Skip to content

fix(validators): write-gate fail-opens -- loop containers, compound guards, receiver boundaries (#350, #352, #351) - #354

Merged
MattGyverLee merged 5 commits into
mainfrom
fix/350-352-351-write-gate
Oct 2, 2026
Merged

MattGyverLee merged 5 commits into
mainfrom
fix/350-352-351-write-gate

Conversation

@MattGyverLee

Copy link
Copy Markdown
Owner

Fixes three write-gate bugs in src/flextoolsmcp/server/validators.py. Two are fail-opens, where an unsafe script could be certified read-only. Do not merge until reviewed.

#350 (P1, safety): loop element taken for a local container

  • _collect_local_container_names now counts a name as a local list/set only if every binding of it builds a local container. It checks Assign, AnnAssign, NamedExpr and AugAssign, plus aliases, using a fixpoint.
  • These bindings disqualify a name: for/comprehension targets, tuple unpacking, with-as, function/lambda parameters, import aliases, except ... as, match captures, and def/class names (new _non_name_store_bindings).
  • Sibling fixed: under "last binding wins", x = e.SensesOS; x.Add(s); x = [] was certified read-only.
  • The skip is now keyed per call node, not per line (new _local_collection_mutation_sites). So tmp.Add(x); entry.SensesOS.Add(s) is now flagged. _lines_with_local_collection_mutations is removed.
  • results = []; results.append(...) and seen = set(); seen.Add(...) are still not flagged.

Closes #350

#352 (P2): compound modifyAllowed guards

  • _is_write_enabled_check accepts an and when any operand is a guard, and an or only when every operand is. not flips the polarity. _is_write_disabled_check mirrors this, so if not modifyAllowed or <cond>: return protects the code after it. The bug: unprotected_writes preflight gate rejects the early-return guard idiom, only accepts if/else #139 early-return forms are unchanged.
  • When an if test contains a call, protection starts on the line after the test. So if x.Add(s) and modifyAllowed: across lines still reports the mutation in the test.
  • Sibling fixed (fail-open): if modifyAllowed == False:, != True, is not True and project.writeEnabled == False were treated as protecting their body. The new _write_flag_compare_polarity only counts a comparison in the enabling direction.
  • _MODIFY_GUARD_RE, which sets the scaffold's FTM_ModifiesDB, now matches compound guards.

Closes #352

#351 (P3): receiver regexes with no left boundary

  • Facade-resolved names (for example fx) now need a word boundary, so prefx no longer matches.
  • Entry/sense/.../pos receivers use _RECEIVER_PREFIX_BOUNDARY. The keyword can still appear anywhere in the identifier, so subsense, subentry, lexentry, mainentry, self._sense and nonsenseEntry are all still flagged. Only a short list of exact English words is skipped: nonsense, compose, position, purpose, suppose and similar.
  • Scope change from the issue: project receivers deliberately keep matching any name ending in project, including myproject, old_project, subproject, srcProject and self._project. Any of these could be a real FLExProject handle, so flagging them fails closed. Review found that adding a \bproject boundary missed real writes, so that part of the issue is not done.

Closes #351

Pattern audit

I audited the whole write gate (find_liblcm_mutations, find_protected_ranges/ProtectionFinder, detect_cud_operations, certify_script_readonly) and the guard and receiver detectors elsewhere in src/.

  • Loop element taken for its container / line-keyed suppression:
    • Fixed: the "last binding wins" sibling.
    • Fixed: names bound as parameters, imports, except-as and match captures.
    • No change: _iter_assign_pairs For handling over-types, which is the safe direction. The other seen_lines sets only remove duplicate report rows and do not suppress the gate.
  • Guard test whose meaning is ignored:
    • Fixed (safety): the false-compare acceptance.
    • Fixed (low stakes): _MODIFY_GUARD_RE.
    • No change: local_recipes.py:544 fails closed. extend_protected_ranges_for_guarded_helper_calls is fixed through the ranges it uses.
  • Receiver regex with no left boundary:
    • Fixed: the facade templates, both property= patterns and _PATTERN_CREATE_GENERIC.
    • Left on purpose: _cache\s*\. patterns, since over-matching there fails closed.
    • Left: _PATTERN_REPORT_*, which the gate does not use.
    • Out of scope: kernel.py:631, which only feeds pattern-learning stats.
  • Residual: position.Note = is now skipped through the English-word list. Other pos\w* names still match, which fails closed. See the open question below.

Tests

  • Offline suite (.venv python, -m "not requires_flex"): 4979 passed, 4 skipped, 119 deselected, 45 subtests passed.
  • python scripts/validate_integrity.py server: exit 0. 31 tools registered, USAGE.md documents all 31, golden and minimal success payloads OK.
  • New test files: tests/test_issue350_loop_container_write_gate.py, tests/test_issue352_compound_guard.py, tests/test_issue351_regex_word_boundary.py. Each was written first and failed before its fix.
  • No response shapes or error codes changed, so I regenerated no goldens and left TOOL-CONTRACT.md and USAGE.md unchanged. CHANGELOG has three Fixed entries under Unreleased.

Live verification: none run. These are offline-only changes to static analysis.

Known merge interactions

These branches are open in parallel: feat/exclusive-access-gate (#343), fix/preflight-recovery-dx, feat/335-surface-recipes.

  • validators.py: fix/preflight-recovery-dx probably also touches the preflight and validator paths. Expect conflicts there.
  • CHANGELOG.md Unreleased: every branch adds entries. These are trivial conflicts.
  • Error-code count, goldens, execution.py: this branch changes none of them. The conflicts belong to the other branches, but rerun validate_integrity.py server and the offline suite after rebasing.

Open questions

  1. Should the pos receiver be narrowed further, for example to pos followed by a non-letter or an uppercase letter? The alternative is to leave it as a fail-closed over-match.
  2. Protected ranges are tracked by line. Is that acceptable, or should they move to AST node ranges?

🤖 Generated with Claude Code

MattGyverLee and others added 5 commits October 2, 2026 12:10
_collect_local_container_names treated a for-loop target as a local
container when its iterable was a list literal or local list, so an
unguarded `for coll in [e.SensesOS]: coll.Add(s)` certified read-only.
A name now counts as local only when every store of it binds a local
container constructor (or another local name); loop/comprehension
targets, tuple unpacking, with-as and any non-container rebinding
disqualify it (flow-insensitive, fail closed).

The suppression in find_liblcm_mutations is now keyed by call node
(line + column past the method name) instead of by line, so
`tmp.Add(x); entry.SensesOS.Add(s)` no longer hides the real Add.

closes #350

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…compares

_is_write_enabled_check now treats an `and` as a guard when any operand
is one and an `or` only when every operand is; `not` flips to the
disabled check. The early-return disabled check mirrors it, so
`if not modifyAllowed or <cond>: return` protects the tail (#139 forms
unchanged). For a compound test only the body is protected, since an
operand before the guard can itself mutate.

Pattern-audit sibling: the Compare branch accepted ANY comparison that
mentioned modifyAllowed / project.writeEnabled, so
`if modifyAllowed == False:` certified its body as protected. Compares
now count only against a literal True/False in the enabling direction.

The scaffold FTM_ModifiesDB regex also recognises compound guards.

closes #352

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The project / facade receiver patterns (_PATTERN_CREATE/DELETE/
UPDATE_PROJECT, _PATTERN_PROJECT_ACCESSOR_CALL, the facade accessor
templates) now require `\b` before the receiver, so `myproject.X.Delete(`
and `prefx.Foo.SetBar(` no longer resolve as facade writes, while
`self.project.X.Delete(` still does.

The type-prefix receivers (entry/sense/.../pos) in the property= and
generic-Add patterns use _RECEIVER_PREFIX_BOUNDARY: a word boundary, a
snake_case `_`, or a camelCase hump. `nonsense.Form =` and
`compose.Comment =` stop matching; `new_entry.LexemeFormOA =` and
`newEntry.LexemeFormOA =` stay gated. A bare `\b` would have dropped
those, turning an over-match into a missed write.

closes #351

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Refs #350, #352, #351.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- #351: drop the left boundary on `project` receivers (self._project,
  srcProject stay gated) and on entry/sense receiver prefixes (subsense,
  subentry, lexentry stay gated); only a short list of exact English
  words (nonsense, compose, position, ...) is excluded. Resolved facade
  names keep `\b`, since each is one exact identifier.
- #352: when a guard's test contains a call, protect only the lines after
  the test, so `if not (x.Add(s) or not modifyAllowed):` and one-line
  `if x.Add(s) and modifyAllowed: pass` still report the call.
- #350: parameters, lambda args, import aliases, except/match captures and
  def/class names now disqualify a name from local-container status.

Refs #350, #351, #352.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@MattGyverLee
MattGyverLee merged commit 500af47 into main Oct 2, 2026
2 checks passed
@MattGyverLee
MattGyverLee deleted the fix/350-352-351-write-gate branch October 2, 2026 17:56
MattGyverLee added a commit that referenced this pull request Oct 2, 2026
Brings in #354 (write-gate fail-opens), #353 (surface recipes), #355 (docs), #356 (preflight retry loops).

Conflict resolution keeps both sides: unknown_import (#305) and requires_exclusive_access sit side by side and the error-code count moves to 49 everywhere (response_models, both count tests, TOOL-CONTRACT, CHANGELOG). execution.py keeps main's #334 assistance-log ordering with the gate's requires_exclusive_access key. test_issue55 ladder test stubs the exclusive-access detector so it still exercises Rung 3 (gate covered in test_exclusive_access_gate.py). Full offline suite: 5273 passed, 4 skipped.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant