app+ui: the nine file and credential verbs, fenced and keyed

Built on grom's explicit say-so, after a round where they were withheld
and the reason recorded. Both halves of that decision stay in the doc,
because the reasoning is what makes the second half a decision rather
than a drift.

ReadCustomFile/ImportFile (one operation, two upstream names),
WriteCustomFile/ExportFile, CustomFileExists, ExecuteCustomLuaFile,
StartItemExport, EncryptPassword, EncryptedServerLogin.

**The fence is app-side and it is not a `..` scan.**
`local_state::custom_file_path` admits one relative name whose every
component is an ordinary name, inside `benilla-config/CustomFiles/`, and
refuses a leaf that is already a symlink. Each escape a string check
misses has its own rule and its own test row: an absolute path (`join`
*replaces* the base rather than appending), a Windows prefix, a `..`
nested inside a longer path, and a planted symlink — the one no amount
of sanitising the string will catch. A subdirectory rather than the
config root, because a verb that could create `config.toml` or
`autologin.toml` by name would let an addon rewrite the client's own
settings; that is a different power from "keep a file of your own".

The VM side holds three closures and no paths, so no spelling of a
`filename` reaches outside that directory.

**The credential pair's deviation is forced, not chosen.** Both are
registered by nampower under "Registering *Glue* Lua functions" and this
client has no glue Lua VM at all — login, character-select and
character-create are Rust. `EncryptPassword` is pure and works the same
anywhere. `EncryptedServerLogin` cannot mean "submit the login form" in
the only VM there is, so it stores the credential for the next login:
`enabled` on, the plaintext field cleared, the blob kept, and
`AutoLogin::effective_password` reads it back at the refill.

What that protects is an `autologin.toml` or a SavedVariables file read
by someone else or pasted into a forum: the blob is authenticated and
keyed on 32 random bytes in a different file, mode 0600. What it does
not protect is anything running as this user, which can read the key as
easily as the blob — there is no user secret in the chain and there
cannot be, because the point is logging in without typing one. Both
halves are at the module.

Encrypt-then-MAC with HMAC-SHA1 as the PRF for both halves, from the two
primitives the tree already had (sha1 moves from a dev-dependency to a
real one). The HMAC is tested against RFC 2202's own vectors, because a
hand-written one still round-trips with itself when it is wrong — and a
first draft of the key reader did exactly that kind of silent wrong: it
returned raw bytes on creation and hex text on read, and the caller
guessed between them with "32 bytes and not all hex digits". It guessed
wrong the moment a random key came out all-hex. Returning one thing is
the fix, and the note is at the function.

**The existing plaintext path is untouched.** Storing the password in
plaintext was grom's own explicit choice when the alternatives were a
keyring or nothing, so this is additive and nothing migrates by itself.
The machinery to make the Remember Password tick-box write a blob
instead is now all here — one line — and that is his call.

Base64 was missing from the whole tree; it is in `benilla-ui`'s
`script/encoding.rs` with RFC 4648's own vectors, and
`C_EncodingUtil.EncodeBase64` will want the same one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
2026-10-24 11:30:49 -06:00
co-authored by Claude Opus 5
parent 78e4e02cfe
commit e716709bca
15 changed files with 1599 additions and 20 deletions
+59 -12
View File
@@ -476,7 +476,7 @@ real when something else agrees — an addon calling it, or the mod's own docs.
What it found that this file did not already have: **38 names in `nampower.dll` that grom's addons
call**, on top of the families the nampower section above already tracks. Unported, and listed
rather than guessed at — `CancelPlayerAuraSlot`, `CancelPlayerAuraSpellId`, `CombatLogFlush`,
rather than guessed at — `CancelPlayerAuraSlot`, `CancelPlayerAuraSpellId`, `CombatLogFlush`, `CustomFileExists`, `EncryptPassword`, `EncryptedServerLogin`, `ExecuteCustomLuaFile`, `ExportFile`,
`CastSpellByNameNoQueue`, `CastSpellNoQueue`, `ChannelStopCastingNextTick`, `CombatLogFlush`,
`CustomFileExists`, `EncryptPassword`, `ExecuteCustomLuaFile`, `FindPlayerItemSlot`, `GetBagItem`,
`GetCastInfo`, `GetCurrentCastingInfo`, `GetItemIconTexture`, `GetItemIdCooldown`,
@@ -565,13 +565,60 @@ Plus the six `NP_EnableUnitEventsX` CVars, registered stored-only. Each has its
`RAW_COMBATLOG`. The script compares against `dump_globals` on a **bare** VM, so a verb installed
later and an *event* (never a global at all) both read as absent. Subtract those before counting.
**Withheld, pending a decision that is not mine (7).** `ExportFile`, `ImportFile`,
`WriteCustomFile`, `ReadCustomFile`, `CustomFileExists`, `ExecuteCustomLuaFile`, `EncryptPassword`
(with `EncryptedServerLogin`). The first six give addon Lua read, write and **execute** on files
under the client folder; the last takes a password. Every one is real in nampower and used for
config export, but they are a different kind of surface from a getter, and the right default for a
client that did not have them yesterday is not to grow them silently. `CombatLogFlush` is here only
because it is in the same family — it would flush our own `WoWCombatLog.txt`, which is benign.
**The nine file and credential functions — built, on grom's explicit say-so.** They were withheld
for a round and the reason recorded ("the right default for a client that did not have them
yesterday is not to grow them silently"); he said yes to all nine. Both halves of that decision
stay here, because the reasoning is what makes the second half a decision rather than a drift.
| Name | Where it lands |
|---|---|
| `ReadCustomFile`, `ImportFile` | one operation under two names (upstream registers both); the file's **bytes** as a Lua string, so a binary file round-trips |
| `WriteCustomFile`, `ExportFile` | `mode` `"a"`/`"append"` adds, anything else replaces; `ExportFile` has no mode and always replaces. Both answer `1` or nil, so a caller can report a failure the player can fix |
| `CustomFileExists` | `1` or nil, and `is_file` rather than `exists` — a directory of that name is not a file this family can read |
| `ExecuteCustomLuaFile` | loads and runs the chunk at global scope, named `@<filename>` so a traceback points at the player's file. A missing file is nil; a **broken** one raises, because that is the distinction worth the most on a file edited by hand |
| `StartItemExport` | writes the item cache as TSV to `ItemExport.txt`. **Deviation**: upstream's name implies a server sweep; this exports the rows the session already saw. A sweep is a few thousand `CMSG_ITEM_QUERY_SINGLE`s at the realm, which is not something a Lua call should start |
| `EncryptPassword` | the blob, from `benilla-app`'s `credential.rs` |
| `EncryptedServerLogin` | **upstream logs in; this remembers** — see below |
**The fence is app-side, and it is not a `..` scan.** `local_state::custom_file_path` admits one
relative name whose every component is an ordinary name, inside `benilla-config/CustomFiles/`, and
refuses a leaf that is already a **symlink**. The escapes a string check misses, each with its own
rule: an absolute path (`join` *replaces* the base rather than appending), a Windows prefix
(`C:\…`, `\\server\share`), a `..` nested inside a longer path, and a planted symlink — which is
the one that no amount of sanitising the string will catch. nampower writes relative to the
client's working directory, the install root, which is read-only here; a *subdirectory* of the
config folder rather than its root, because a verb that could create `config.toml` or
`autologin.toml` by name would let an addon rewrite the client's own settings. That is a different
power from "keep a file of your own", and it is not the one being granted.
**The credential pair's deviation is forced, not chosen.** Both are registered by nampower under
"Registering *Glue* Lua functions" — the login screen's VM — and this client has **no glue Lua VM
at all**: login, character-select and character-create are Rust (`autologin.rs`'s header sets that
out at length). `EncryptPassword` is a pure function and works the same wherever it is defined.
`EncryptedServerLogin` cannot mean "submit the login form" in the only VM there is, because in it
you are already in the world. What the pair is *for* is a password on disk as a blob rather than as
itself, so it stores the credential for the next login instead: `enabled` on, the plaintext field
**cleared**, the blob kept, and `AutoLogin::effective_password` reads it back at the next refill.
What that protects: an `autologin.toml` or a SavedVariables file read by someone else or pasted into
a forum. The blob is authenticated and keyed on 32 random bytes in a **different file**
(`benilla-config/credential.key`, mode `0600`). What it does not protect: anything running as this
user, which can read the key as easily as the blob — there is no user secret in the chain and there
cannot be, because the point is logging in without typing one. The construction is encrypt-then-MAC
with HMAC-SHA1 as the PRF for both halves, from the two primitives this tree already had; the HMAC
is tested against RFC 2202's own vectors, because a hand-written one still round-trips with itself
when it is wrong.
**Not wire-compatible with nampower's blobs**, and it cannot be: that scheme is undocumented and a
key file generated here would not decrypt one made there whatever the cipher. A blob copied from a
real install answers nil, which the drain reports rather than storing — a stored credential that
cannot be read back is a login box that silently refills empty.
**The existing plaintext path is untouched.** Storing the password in plaintext was grom's own
explicit choice when the alternatives were a keyring or nothing; this is additive, and nothing
migrates by itself. The machinery to make the Remember Password tick-box write a blob instead is
now all here — one line in `login/mod.rs` — and that is his call to make, not a side effect of
porting two verbs.
**Needs a feed this client does not have, with what it would take (2).**
@@ -824,13 +871,13 @@ comm -23 /tmp/np /tmp/ours
| Bucket | Count | Names |
|---|---|---|
| **Built** | 45 | `CancelPlayerAuraSlot`, `CancelPlayerAuraSpellId`, `CastSpellByName`, `CastSpellByNameNoQueue`, `CastSpellNoQueue`, `FindPlayerItemSlot`, `GetBagItem`, `GetBagItems`, `GetEquippedItem`, `GetEquippedItems`, `GetItemIconTexture`, `GetItemIdCooldown`, `GetItemLevel`, `GetItemStats`, `GetItemStatsField`, `GetNampowerVersion`, `GetPlayerAuraDuration`, `GetQuestDialogQuestId`, `GetQuestLogQuestIds`, `GetSpellDuration`, `GetSpellIconTexture`, `GetSpellIdCooldown`, `GetSpellIdForName`, `GetSpellModifiers`, `GetSpellNameAndRankForId`, `GetSpellRangeData`, `GetSpellRec`, `GetSpellRecField`, `GetSpellSlotTypeIdForName`, `GetTrinketCooldown`, `GetTrinkets`, `GetUnitField`, `GetUnitGUID`, `IsAuraHidden`, `IsSpellInRange`, `IsSpellUsable`, `PlayerIsMoving`, `PlayerIsRooted`, `PlayerIsSwimming`, `SetLocalRaidTargetIndex`, `SetMouseoverUnit`, `UseItemIdOrName`, `UseTrinket` |
| **Withheld pending grom's say-so** | 9 | `WriteCustomFile`, `ReadCustomFile`, `CustomFileExists`, `ImportFile`, `ExportFile`, `ExecuteCustomLuaFile`, `StartItemExport` (writes a file of its own), `EncryptPassword`, `EncryptedServerLogin` |
| **Built** | 54 | `CancelPlayerAuraSlot`, `CancelPlayerAuraSpellId`, `CastSpellByName`, `CastSpellByNameNoQueue`, `CastSpellNoQueue`, `FindPlayerItemSlot`, `GetBagItem`, `GetBagItems`, `GetEquippedItem`, `GetEquippedItems`, `GetItemIconTexture`, `GetItemIdCooldown`, `GetItemLevel`, `GetItemStats`, `GetItemStatsField`, `GetNampowerVersion`, `GetPlayerAuraDuration`, `GetQuestDialogQuestId`, `GetQuestLogQuestIds`, `GetSpellDuration`, `GetSpellIconTexture`, `GetSpellIdCooldown`, `GetSpellIdForName`, `GetSpellModifiers`, `GetSpellNameAndRankForId`, `GetSpellRangeData`, `GetSpellRec`, `GetSpellRecField`, `ImportFile`, `ReadCustomFile`, `StartItemExport`, `WriteCustomFile`, `GetSpellSlotTypeIdForName`, `GetTrinketCooldown`, `GetTrinkets`, `GetUnitField`, `GetUnitGUID`, `IsAuraHidden`, `IsSpellInRange`, `IsSpellUsable`, `PlayerIsMoving`, `PlayerIsRooted`, `PlayerIsSwimming`, `SetLocalRaidTargetIndex`, `SetMouseoverUnit`, `UseItemIdOrName`, `UseTrinket` |
| **Not ours to add** | 4 | `QueueSpellByName`, `QueueScript`, `ChannelStopCastingNextTick`, `LearnTalentRank` |
| **Measured out, with the reason at each** | 4 | `DisenchantAll` (queue machinery — see the table above), `GetUnitData`, `GetRaidTargets`, `GetSpellPower` |
45 + 9 + 4 + 4 = 62, and the `comm -23` above is the check: run it and the seventeen it prints are
exactly the last three rows. Nothing is in two buckets and nothing is in none.
54 + 4 + 4 = 62, and the `comm -23` above is the check: run it and the eight it prints are exactly
the last two rows. Nothing is in two buckets and nothing is in none.
The two that were at the head of that last bucket are now built — see "The cast feed" above for
why they came first and what each decision cost.