Close the three remaining local-config bypasses in the typecheck gate - #14
Conversation
--configpath displaces individual settings rather than whole tables, so a
suppression knob is only closed if the committed config names it. The
previous four were found by guessing at candidates a round at a time. This
set instead comes from enumerating what the server actually reads:
grep -rhoE "config\.get\([^,]*, *'Lua\.[A-Za-z.]+'" \
script/core/diagnostics/*.lua script/provider/diagnostic.lua
That is 17 keys on 3.19.0, of which six are live bypasses: globalsRegex and
enableScheme join the four already declared under diagnostics, and special
sits under runtime.
enableScheme is the serious one. It decides whether a document is diagnosed
at all rather than suppressing a single code, so a local ["git"] silences
the entire check. Its default is ["file"], which also makes it the one key
where declaring [] is not the fix, since an empty list silences the check
just as completely. Verified against a planted return-type-mismatch: a
hostile ["git"], a globalsRegex of [".*"] and a runtime.special all leave
the finding in place, as does disable, which guards the earlier four.
.gitignore widens to .luarc.json* so the .jsonc twin cannot be committed
either.
There was a problem hiding this comment.
The three declarations here are correct and I reproduced the one that matters. But "the three remaining" is not the full set: four vectors still bypass this gate silently at the PR head, and two of them are inside the 17 keys the enumeration already surfaced.
Measured on 3.19.0 (same version CI pins), against a planted return-type-mismatch in src/protobuf/, at 90f5447. exit is lua-language-server --check's own status, which is what make typecheck gates on:
local .luarc.json |
problems | exit | result |
|---|---|---|---|
| none (control) | 1 | 1 | gate holds |
diagnostics.groupFileStatus: {"type-check":"None"} |
0 | 0 | bypassed |
diagnostics.neededFileStatus: {"return-type-mismatch":"None"} |
0 | 0 | bypassed |
workspace.maxPreload: 1 |
0 | 0 | bypassed |
runtime.plugin: "zz_plugin.lua" |
0 | 0 | bypassed |
All four exit 0, so make check passes and CI goes green with a live type error in the tree.
Why groupFileStatus and neededFileStatus are live
These were in the 17 and dismissed, and I think the reasoning was that check_worker.lua clobbers them. It only partly does. downgrade_checks_to_opened rewrites statuses to Opened!, and the trailing ! is what makes getStatus return early before consulting groupFileStatus. But it only rewrites entries that are Any or Any!:
for d, status in pairs(protoDiag.getDefaultStatus()) do
if status == 'Any' or status == 'Any!' then
diagStatus[d] = 'Opened!'Every diagnostic in the type-check group is registered status = 'Opened' (script/proto/diagnostic.lua:72-86), not Any. So the whole type-check group never receives the !, and getStatus falls through to neededFileStatus[name] and then groupFileStatus, both attacker controlled. The clobber protects the Any diagnostics and leaves exactly the ones this gate exists for.
Why the enumeration missed the other two
The grep's file scope is script/core/diagnostics/*.lua and script/provider/diagnostic.lua, so it finds keys that suppress a code. It structurally cannot find keys that stop a file from being analyzed at all. Tree wide there are 94 Lua.* keys against the 17 in scope, and Lua.runtime.plugin is read at script/plugin.lua:104, outside both paths.
runtime.plugin is the strongest of the four: check_worker.lua does require 'plugin', so a plugin's OnSetText rewrites source before parsing. A three line plugin that returns an empty edit blanks every file in the repo. I confirmed it is the plugin executing rather than a load failure by running a no-op OnSetText as a control, which correctly reports 1 problem. The checkTrustLoad prompt does not stop it in --check, no trusted file needed.
Suggested patch
Verified: this holds the gate at 1 problem and exit 1 against all four vectors above plus the three this PR already closes, and stays at 0 and exit 0 on a clean tree.
"runtime": {
"version": "LuaJIT",
"special": {},
"plugin": "",
"pluginArgs": []
},
"workspace": {
...
"maxPreload": 5000,
"preloadFileSize": 500
},
"diagnostics": {
...
"groupFileStatus": {},
"groupSeverity": {},
"neededFileStatus": {}
}groupSeverity and preloadFileSize are belt and braces, not measured bypasses. groupSeverity downgrades the severity label but the finding is still counted and still exits 1, and preloadFileSize: 0 fails loud with 34 problems rather than hiding one. Declaring them costs nothing and removes the need to re-derive that.
Worth widening the CLAUDE.md guidance too. The current text points the reader at the grep as the authoritative candidate set, and the two mechanisms above are precisely the ones it cannot produce. Suggest saying that suppression keys come from the diagnostics paths, and that anything gating file loading or source text has to be enumerated separately from script/workspace.lua and script/plugin.lua.
Smaller notes
CLAUDE.mdsays "six were measured as live bypasses and are declared here" and then lists seven:enable,disable,severity,globals,globalsRegex,enableScheme, plusruntime.special. Same off by one in the PR body.- The
.gitignorewidening is right..luarc.json*catches both.luarc.jsonand.luarc.jsonc, and.luarc-typecheck.jsonstays tracked since the prefix does not match.
What I did not test
I did not build a probe for runtime.special, so I am taking that one on your measurement rather than confirming it. I did confirm enableScheme and globalsRegex are genuine: against f77b5ef's config, enableScheme: ["git"] takes a return-type-mismatch probe from 1 to 0, and globalsRegex: [".*"] takes an undefined-global probe from 1 to 0, and both hold at 1 with this PR's config. I did not sweep the remaining 77 out of scope keys, so I would not treat the four above as proof the list is now complete either.
Review found three more live vectors, and they expose two distinct holes in how the earlier set was derived. runtime.plugin is invisible to the grep by construction. It is read in script/plugin.lua, outside the diagnostics paths the enumeration covers, and check_worker.lua does require "plugin", so an OnSetText returning an empty edit blanks every file and the check passes having analysed nothing. The file scope of that grep is the shape of its blind spot. neededFileStatus and groupFileStatus were in the enumeration and were dismissed as inert, which was a measurement error rather than a scope one. downgrade_checks_to_opened force-overwrites only codes whose default status is Any, so everything defaulting to Opened stays under local control, which is exactly the type-check group. An undefined-global probe reports the key as inert; a return-type-mismatch probe shows groupFileStatus taking out the whole type-check group in one line. Immunity is per-code, so one probe does not measure a key. Verified against a planted return-type-mismatch, all holding at 1 problem and exit 1: runtime.plugin blanking every file, groupFileStatus type-check None, neededFileStatus return-type-mismatch None, enableScheme ["git"], globalsRegex [".*"], disable, runtime.special, preloadFileSize 0. Clean tree still reports no problems in all five repos. pluginArgs, groupSeverity, maxPreload and preloadFileSize are declared as belt and braces rather than measured bypasses. groupSeverity relabels a finding that is still counted and still exits non-zero, and preloadFileSize 0 fails loud rather than hiding anything. CLAUDE.md now says the grep is a floor rather than the candidate set, gives the separate enumeration for file-loading and source-rewriting vectors, and records both traps: empty is only inert if the read site falls back to a default, and immunity is per-code. Also corrects the count, which said six and listed seven.
|
You were right on all three, and they expose two different holes in how I derived the earlier set.
Verified against a planted Clean tree still reports no problems in all five repos.
I am not treating this list as complete either. Neither enumeration is a proof. |
There was a problem hiding this comment.
Approving. dbf1f898 closes all four vectors I blocked on at 90f5447, and I measured each one rather than reading the declaration and assuming it lands.
The four blockers, re-measured
Probe is a planted return-type-mismatch in src/protobuf/, server 3.19.0 locally, which is exactly the CI pin. Every row also has a strip-one-out control: the same hostile local .luarc.json against a config with just that key deleted. Without the control, "held" only proves the attack never worked.
hostile local .luarc.json |
shipped dbf1f898 |
control, key stripped |
|---|---|---|
runtime.plugin blanking every file |
exit 1, 1 problem | exit 0, gate off |
workspace.maxPreload: 1 |
exit 1, 1 problem | exit 0, gate off |
diagnostics.neededFileStatus code to None |
exit 1, 1 problem | exit 0, gate off |
diagnostics.groupFileStatus type-check to None |
exit 1, 1 problem | exit 0, gate off |
All four controls reach exit 0, so all four were live, and all four are now load-bearing declarations rather than decoration.
The plugin row needed its own control, because a plugin that fails to load looks identical to a plugin that was blocked. A no-op OnSetText returning {} still reports 1 problem under both configs, which is what proves the hostile plugin genuinely ran in the control and was genuinely displaced in the shipped config.
Both belt and braces claims check out
I tried to falsify these two rather than take them on the doc's word, and they held.
groupSeverity relabels without hiding: with the key stripped and a local groupSeverity: {"type-check": "Hint"}, the diagnostic prints as [Hint] instead of [Warning] and the run still ends 1 problems found, exit 1. Relabeled, still counted, still non-zero, exactly as written.
preloadFileSize: 0 fails loud: with the key stripped, that local takes the run from 5 problems to 34, not to zero. More findings, not fewer.
What is still open: the Lua.type table
This is not a defect in the PR, and the doc already says any unnamed setting stays reachable. Flagging it because it is a third read-site family that neither enumeration in CLAUDE.md can surface, and because three keys in it each take the gate to exit 0 on their own.
Lua.type.* is read in script/vm/type.lua and script/vm/compiler.lua. There are zero Lua.type.* reads under script/core/diagnostics/ or script/provider/diagnostic.lua, so the committed grep misses the table by construction, and it gates type comparison rather than file loading, so the plugin and workspace enumeration misses it too.
Each measured with its own probe shape, no local config versus hostile local, nothing else changed:
| key, default | hostile value | probe | result |
|---|---|---|---|
weakNilCheck, false |
true |
---@return string returning nil |
1 problem, exit 1 to exit 0 |
weakUnionCheck, false |
true |
string|number passed to ---@param s string |
1 problem, exit 1 to exit 0 |
maxUnionVariants, 0 |
1 |
bad literal against a 4 variant @alias |
1 problem, exit 1 to exit 0 |
inferParamType: true did not suppress anything on the probe I built, and I did not find a hostile direction for castNumberToInteger, checkTableShape or inferTableSize, whose defaults are already the permissive end.
Verified patch, holds against all three and leaves a clean tree at exit 0:
"type": {
"castNumberToInteger": true,
"weakUnionCheck": false,
"maxUnionVariants": 0,
"weakNilCheck": false,
"inferParamType": false,
"checkTableShape": false,
"inferTableSize": 10
}This is server behaviour, so it applies identically to lua-bitn and lua-noiseprotocol, and to lua-bthome-ble which already merged its round. Nothing tracks it yet. Happy to open a ticket and carry the same three lines across all four if you want it closed rather than noted.
Elsewhere in the sweep
I also swept the other 21 undeclared keys under diagnostics, runtime and workspace against a 5 error battery probe. All 21 left the count at 5, except runtime.builtin with basic disabled, which raised it to 70. So Lua.type is the only family I found still open.
What I did not check
I did not sweep the roughly 65 keys outside the diagnostics, runtime, workspace and type tables. I did not test combinations of keys, only one hostile key at a time. My battery probe covers return-type-mismatch, param-type-mismatch and assign-type-mismatch, so a key that only suppresses some other type-check code would read as inert in my sweep. And the three Lua.type rows are a floor for that table, not a completeness claim about it.
Checks are green at dbf1f898, 8 of 8. Leaving the merge to you.
Follow-up to the typecheck sweep. The committed config declared four
suppression knobs; three more were reachable from a local
.luarc.json.The earlier four were found by guessing at candidates a round at a time,
which is why this took several passes. This set comes from enumerating
what the server actually reads:
17 keys on 3.19.0, six of them live bypasses.
globalsRegexandenableSchemejoin the four underdiagnostics;specialsits underruntime, outside that table.enableSchemeis the one that matters. It gates whether a document isdiagnosed at all rather than suppressing a code, so a local
["git"]turns the whole gate off. Its default is
["file"], which makes it theone key where the reflex of declaring
[]is wrong: an empty listsilences the check just as completely.
Verified against a planted
return-type-mismatchin lua-bitn:.gitignorewidens to.luarc.json*so the.jsonctwin cannot becommitted either. No
src/changes;make checkclean in all four.