Skip to content

Forge: quote any table key that is not a plain Lua identifier - #770

Draft
jfuglas wants to merge 2 commits into
fitzforge:nextfrom
jfuglas:forge/quoted-table-keys
Draft

jfuglas wants to merge 2 commits into
fitzforge:nextfrom
jfuglas:forge/quoted-table-keys

Conversation

@jfuglas

@jfuglas jfuglas commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Issues

No issue: this came out of a repository audit.

  • I checked for open pull requests on the same subject (gh pr list --state open --search for emit.rs, parity, luaKey) and found none that overlap.

What and why

#750 made the JS emitter quote any table key that is not a plain Lua identifier, but only in tools/lib/dsl/emit.js. Forge, the compiler that builds packs (forge guide compile, forge release pack), still wrote those keys bare. This ports the same behaviour to rust/crates/forge-guide/src/emit.rs: lua_key, LUA_KEYWORDS, is_int_key, and an id_map that writes a non-integer key as a string after the integers.

This is defence in depth, not a live exploit. Tracing today's inputs found no author text that reaches those bare keys: names, written and nextWhen are already quoted, legs.json keys are validated, and preTitles keys come from data.

Guides.lua should not change: a bare key that was not an identifier or a keyword would already have been a Lua syntax error, so valid output only held keys lua_key leaves bare. The one exception is goto, which Lua 5.1 accepts bare and the list quotes as #750 does; addon/Polestar_Guides/Guides.lua has no goto = key. The guide_parity goldens in cargo test compare the mini and slice output byte for byte.

The report, checked

Not a bug report. The Rust tests use the same hostile keys as tests/emit-keys.test.js and read the output back through Lua 5.1 (mlua).

Left as it is

  • data_inline (the import string's twin) writes JSON values, not Lua source, so it has no bare key. Its preTitles still assumes numeric keys and panics on any other key, as dataInline throws in JS. Emitter: quote any table key that is not a plain Lua identifier #750 left that alone and so does this.
  • emit_class_trainers still writes {cls} = { ... } bare, as emitClassTrainers does in JS. Those keys are class names from our own data, in Core/ClassTrainers.lua, not a pack.

Parity in CI: not added, and why

No workflow changes here. parity.sh mini does not pass on clean next, so it was not added. I ran it on my fork at 463c52c and did not change the script or the fixture to make it pass.

Without reference data (what CI has):

  • Forge stops with one error ("no reference data") and writes no pack, as its help says it will. js-ref.js calls the compiler directly and carries on, giving 12 errors, 6 warnings and a full pack. Only in ref-mini: pack.
  • diagnostics.json also names the data folder differently: absolute in JS, data/forever in Rust.

With the committed data slice (--data rust/tests/golden/world/root/data/forever): I compared live JS output with the guide_parity goldens, which are Forge's output. For mini and slice, Guides.lua, Pack.lua and the TOC differ, and only Forge writes pack_Horde / pack_Alliance. That is expected: both compile as Polestar's own pack, and the JS compiler never learnt format 8 (rust/tests/golden/guide/README.md).

guide_parity does run in CI, but against frozen output, so it catches Forge changing and not the two compilers drifting apart.

parity.sh can only compare our own pack: js-ref.js and forge guide compile --guides both compile as Polestar's pack, whatever pack.json the folder holds. I tried the 12 fixture packs in tests/fixtures/packs that way and every one came out as our own pack with Guides.lua differing, so that run says nothing about community packs. A live check that could pass would have to compile a community pack with --pack on both sides, which means changing js-ref.js and parity.sh. I have not tried that, so I don't know if it passes.

Checks

  • A changelog fragment in changes/ (written by hand; forge changelog check runs in CI)
  • An AI agent wrote or helped with this — Model and tool: Claude Opus 5.5 via Claude Code
  • Not run, and why: nothing Rust ran on my machine, which has no Rust toolchain. Format, clippy, tests and the four new tests ran only in CI on this pull request. node --test tests/emit-keys.test.js was not run: nothing in tools/ changed.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jfuglas
jfuglas force-pushed the forge/quoted-table-keys branch from eca249d to 83e8f2e Compare October 31, 2026 19:46
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

1 participant