Safely handle secret values in aura countdown updates by DanderBot · Pull Request #5 · DanderBot/DandersFrames

DanderBot · 2025-12-01T12:24:57Z

Summary

guard countdown updates with a safe number coercion to avoid operating on secret values
fall back to hiding countdown text when cooldown data cannot be safely read

Testing

not run (not requested)

…uffs-sk77o8

Resource bar border (BUG #12): - FullFrameRefresh and OnAttributeChanged called non-existent DF:PositionResourceBar(), renamed to DF:ApplyResourceBarLayout() Heal absorb bar (BUG #5): - Add calc:SetHealAbsorbMode(1) so GetHealAbsorbs() returns the full absorb amount instead of subtracting incoming heals (mode 0 default) Performance: - Replace pcall wrappers with nil checks in all three calculator usages (2x damage absorb, 1x heal absorb) — pcall is expensive and these run per-frame every update

Replaces the scaffolded stubs with the full cast-lifecycle implementation. Debug output via DF:Debug("TARGETEDLIST", ...) is used as a stand-in for visual bars until commit #5 lands the render pipeline — this lets in-game verification happen against the debug console without requiring the full rendering work first. All 13 correctness gotchas from the TS3 cross-reference in _Reference/targeted-spells-findings.md are handled and tagged inline: #1 0.2s delay before reading cast data (start events fire before UnitSpellTargetName / duration are populated) #2 Cast-ID matching at both pickup (via UnitCastingInfo) and stop (via event castGuid) — prevents rapid-restart flicker #3 UNIT_SPELLCAST_SUCCEEDED during an active channel is treated as a no-op (channel tick, not a stop) #4 INTERRUPTED with nil interrupter or a dead caster is treated as a normal stop (no interrupter flash) #6 Empower spellId/castGuid offset handled (same shape as regular start on current retail — documented inline) #7 Interrupter lookup via UnitNameFromGUID / UnitClassFromGUID #8 Uninterruptible flag from UnitCastingInfo/UnitChannelInfo is treated as secret-tainted and only ever fed to SetVertexColorFromBoolean (applied in commit #5) #9 UNIT_SPELLCAST_INTERRUPTIBLE / NOT_INTERRUPTIBLE overwrite the stored uninterruptible field with a clean boolean #11 LOADING_SCREEN_DISABLED triggers a full release sweep Also adds the cast-targeting filter pipeline: * TargetedList_IsRelevantCaster filters out non-nameplate units, friendly units, and party-member nameplates * TargetedList_ReadCastData unifies UnitCastingInfo / UnitChannelInfo read paths, handling the positional offset difference for castID vs notInterruptible between the two APIs * Important-spells filter via C_Spell.IsSpellImportant * Hide-own-casts filter * Roster name cache used for O(1) target lookup Event dispatcher extended to route NAME_PLATE_UNIT_ADDED and UNIT_TARGET through the start handler so new nameplates and mid-cast target swaps are picked up. UNIT_TARGET also fires an immediate stop to drop the old tracking — the 0.2s delayed re-pickup recreates it if the new target is still a party member. Content-type filter (gotcha #13) is intentionally deferred to commit #5 since the existing TargetedSpells content-type detection can be reused rather than duplicated.

Live testing surfaced two more secret-tainting restrictions that collectively kill the name-matching approach: Finding A: Equality comparison on secret strings errors. Not just arithmetic — "apiCastId ~= scheduledCastId" raised "attempt to compare a secret string value". Cast-ID matching is impossible. Finding B: UnitSpellTargetName returns a SECRET string on nameplates. "targetedListRosterNames[targetName]" raised "table index is secret". The findings doc was wrong: this string is only safe as input to secret-safe sinks (FontString:SetText), never as a key or compare. Reading TS3 again with this knowledge clarifies their architecture — they DO use UnitInParty("nameplateXtarget") as their primary filter (Driver.lua:317), and we have empirical evidence (TS3 functioning post-hotfix) that the compound-vs-party comparison still works despite what the original findings doc warned. The doc's listing of this as "the highest-leverage probe to verify" is now answered: it works. Refactor: * NEW: TargetedList_CastTargetIsPartyMember(casterUnit) — uses UnitInParty(casterUnit .. "target") just like TS3. Returns a bool, not a unit token. We don't need to know WHICH party member. * REMOVED: TargetedList_RebuildRosterCache, targetedListRosterNames, DF._TargetedListRebuildRoster export, TargetedList_IsCastTargetingGroupMember, GROUP_ROSTER_UPDATE event registration, and the dispatcher branch that called the rebuild. * REMOVED: cast-ID matching from both DelayedPickup and OnCastStop. Equality compare on secret-tainted castID errors. Without it, rapid same-spell restarts may briefly show stale state. Acceptable. * REMOVED: stored targetUnit / targetName / targetClass on the active-cast record. The render pipeline (commit #5) will fetch these via UnitSpellTargetName / UnitSpellTargetClass at render time and pipe them directly into FontString:SetText and C_ClassColor.GetClassColor — both of which are secret-safe sinks that accept secret string arguments. * REMOVED: hide-own-casts filter. The previous implementation compared targetUnit == "player". Without that field, the filter needs to be implemented at render time via SetAlphaFromBoolean on UnitIsUnit(casterUnit .. "target", "player") — "player" is in the always-allowed list and returns a secret-safe boolean. Deferred to commit #5. The active-cast record is now down to nine fields, all either clean or only fed to secret-safe sinks. Total file diff: -63 lines net.

Live testing surfaced more secret-tainting: the spellId from the UNIT_SPELLCAST_START event payload is itself secret-tainted on nameplates, not just the values from UnitCastingInfo. So: C_Spell.IsSpellImportant(secretSpellId) returns a SECRET boolean. not <secretBool> raises 'attempt to perform boolean test on a secret boolean value'. Combined with earlier findings, the rule is: essentially everything derived from a nameplate context is secret. The only safe pattern is to read at render time and pipe directly into secret-safe sinks (SetText, SetTexture, SetAlphaFromBoolean, SetVertexColorFromBoolean, SetShownFromBoolean, SetTimerDuration). Refactor of TargetedList_DelayedPickup: * REMOVED: important-spells filter at pickup. Will be re-added in commit #5 via SetShownFromBoolean(IsSpellImportant(spellId), true, false) at render time, so the filter never inspects the secret bool in Lua. * REMOVED: spellName / spellTexture reads at pickup. The render pipeline will fetch them via C_Spell.GetSpellName / C_Spell.GetSpellTexture and feed them straight into SetText / SetTexture sinks. * Active-cast record now stores spellId opaquely as a secret token — never inspected, only passed into C_Spell.* + sinks. * Debug log simplified to clean values only: casterUnit + channel flag. The previous tostring(spellName) call would have crashed on a secret string. Refactor of TargetedList_OnCastStop: * REMOVED: interrupter source extraction (gotcha #7). The interrupter GUID arrives in the event payload as a secret string; truth-testing it ('not interrupterGuid') is not allowed and UnitNameFromGUID(secretGuid) returns a secret string that can't be formatted in Lua. The render pipeline will display the interrupter name (if at all) by piping UnitNameFromGUID's result directly into a SetText sink at render time. * Stop event still records the wasInterrupted flag for the render pipeline to play the interrupted-flash animation.

Resource bar border (BUG DanderBot#12): - FullFrameRefresh and OnAttributeChanged called non-existent DF:PositionResourceBar(), renamed to DF:ApplyResourceBarLayout() Heal absorb bar (BUG DanderBot#5): - Add calc:SetHealAbsorbMode(1) so GetHealAbsorbs() returns the full absorb amount instead of subtracting incoming heals (mode 0 default) Performance: - Replace pcall wrappers with nil checks in all three calculator usages (2x damage absorb, 1x heal absorb) — pcall is expensive and these run per-frame every update

Replaces the scaffolded stubs with the full cast-lifecycle implementation. Debug output via DF:Debug("TARGETEDLIST", ...) is used as a stand-in for visual bars until commit DanderBot#5 lands the render pipeline — this lets in-game verification happen against the debug console without requiring the full rendering work first. All 13 correctness gotchas from the TS3 cross-reference in _Reference/targeted-spells-findings.md are handled and tagged inline: #1 0.2s delay before reading cast data (start events fire before UnitSpellTargetName / duration are populated) DanderBot#2 Cast-ID matching at both pickup (via UnitCastingInfo) and stop (via event castGuid) — prevents rapid-restart flicker DanderBot#3 UNIT_SPELLCAST_SUCCEEDED during an active channel is treated as a no-op (channel tick, not a stop) DanderBot#4 INTERRUPTED with nil interrupter or a dead caster is treated as a normal stop (no interrupter flash) DanderBot#6 Empower spellId/castGuid offset handled (same shape as regular start on current retail — documented inline) DanderBot#7 Interrupter lookup via UnitNameFromGUID / UnitClassFromGUID DanderBot#8 Uninterruptible flag from UnitCastingInfo/UnitChannelInfo is treated as secret-tainted and only ever fed to SetVertexColorFromBoolean (applied in commit DanderBot#5) DanderBot#9 UNIT_SPELLCAST_INTERRUPTIBLE / NOT_INTERRUPTIBLE overwrite the stored uninterruptible field with a clean boolean DanderBot#11 LOADING_SCREEN_DISABLED triggers a full release sweep Also adds the cast-targeting filter pipeline: * TargetedList_IsRelevantCaster filters out non-nameplate units, friendly units, and party-member nameplates * TargetedList_ReadCastData unifies UnitCastingInfo / UnitChannelInfo read paths, handling the positional offset difference for castID vs notInterruptible between the two APIs * Important-spells filter via C_Spell.IsSpellImportant * Hide-own-casts filter * Roster name cache used for O(1) target lookup Event dispatcher extended to route NAME_PLATE_UNIT_ADDED and UNIT_TARGET through the start handler so new nameplates and mid-cast target swaps are picked up. UNIT_TARGET also fires an immediate stop to drop the old tracking — the 0.2s delayed re-pickup recreates it if the new target is still a party member. Content-type filter (gotcha DanderBot#13) is intentionally deferred to commit DanderBot#5 since the existing TargetedSpells content-type detection can be reused rather than duplicated.

Live testing surfaced two more secret-tainting restrictions that collectively kill the name-matching approach: Finding A: Equality comparison on secret strings errors. Not just arithmetic — "apiCastId ~= scheduledCastId" raised "attempt to compare a secret string value". Cast-ID matching is impossible. Finding B: UnitSpellTargetName returns a SECRET string on nameplates. "targetedListRosterNames[targetName]" raised "table index is secret". The findings doc was wrong: this string is only safe as input to secret-safe sinks (FontString:SetText), never as a key or compare. Reading TS3 again with this knowledge clarifies their architecture — they DO use UnitInParty("nameplateXtarget") as their primary filter (Driver.lua:317), and we have empirical evidence (TS3 functioning post-hotfix) that the compound-vs-party comparison still works despite what the original findings doc warned. The doc's listing of this as "the highest-leverage probe to verify" is now answered: it works. Refactor: * NEW: TargetedList_CastTargetIsPartyMember(casterUnit) — uses UnitInParty(casterUnit .. "target") just like TS3. Returns a bool, not a unit token. We don't need to know WHICH party member. * REMOVED: TargetedList_RebuildRosterCache, targetedListRosterNames, DF._TargetedListRebuildRoster export, TargetedList_IsCastTargetingGroupMember, GROUP_ROSTER_UPDATE event registration, and the dispatcher branch that called the rebuild. * REMOVED: cast-ID matching from both DelayedPickup and OnCastStop. Equality compare on secret-tainted castID errors. Without it, rapid same-spell restarts may briefly show stale state. Acceptable. * REMOVED: stored targetUnit / targetName / targetClass on the active-cast record. The render pipeline (commit DanderBot#5) will fetch these via UnitSpellTargetName / UnitSpellTargetClass at render time and pipe them directly into FontString:SetText and C_ClassColor.GetClassColor — both of which are secret-safe sinks that accept secret string arguments. * REMOVED: hide-own-casts filter. The previous implementation compared targetUnit == "player". Without that field, the filter needs to be implemented at render time via SetAlphaFromBoolean on UnitIsUnit(casterUnit .. "target", "player") — "player" is in the always-allowed list and returns a secret-safe boolean. Deferred to commit DanderBot#5. The active-cast record is now down to nine fields, all either clean or only fed to secret-safe sinks. Total file diff: -63 lines net.

Live testing surfaced more secret-tainting: the spellId from the UNIT_SPELLCAST_START event payload is itself secret-tainted on nameplates, not just the values from UnitCastingInfo. So: C_Spell.IsSpellImportant(secretSpellId) returns a SECRET boolean. not <secretBool> raises 'attempt to perform boolean test on a secret boolean value'. Combined with earlier findings, the rule is: essentially everything derived from a nameplate context is secret. The only safe pattern is to read at render time and pipe directly into secret-safe sinks (SetText, SetTexture, SetAlphaFromBoolean, SetVertexColorFromBoolean, SetShownFromBoolean, SetTimerDuration). Refactor of TargetedList_DelayedPickup: * REMOVED: important-spells filter at pickup. Will be re-added in commit DanderBot#5 via SetShownFromBoolean(IsSpellImportant(spellId), true, false) at render time, so the filter never inspects the secret bool in Lua. * REMOVED: spellName / spellTexture reads at pickup. The render pipeline will fetch them via C_Spell.GetSpellName / C_Spell.GetSpellTexture and feed them straight into SetText / SetTexture sinks. * Active-cast record now stores spellId opaquely as a secret token — never inspected, only passed into C_Spell.* + sinks. * Debug log simplified to clean values only: casterUnit + channel flag. The previous tostring(spellName) call would have crashed on a secret string. Refactor of TargetedList_OnCastStop: * REMOVED: interrupter source extraction (gotcha DanderBot#7). The interrupter GUID arrives in the event payload as a secret string; truth-testing it ('not interrupterGuid') is not allowed and UnitNameFromGUID(secretGuid) returns a secret string that can't be formatted in Lua. The render pipeline will display the interrupter name (if at all) by piping UnitNameFromGUID's result directly into a SetText sink at render time. * Stop event still records the wasInterrupted flag for the render pipeline to play the interrupted-flash animation.

… left behind TargetedList_IsGateOpen hid the Targeted List from release builds while it was being written. The feature shipped -- settings toggle, GUI page, profile export -- so the gate had been `return true` with no way to close it, and its 15 call-site guards and its "dev gate closed (release build)" reason string could never fire. It was never the user-facing gate, and removing it does not change what the feature does: that gate is party.targetedListEnabled, checked in TargetedList_IsActive, and it is untouched. The comments around it had drifted the same way. The file banner still advertised the group display deleted in July -- per-member frame icons, stacking, max-icon limit, sort by cast time -- so anyone who skipped the deprecation block got a description of a feature that is gone. The header said the Targeted List "is being designed" when it ships 2000 lines below. Seven "commit #4/#5/#6" notes described already-shipped code as scaffolding awaiting implementation. And a comment claimed DF:InitTargetedList gates internally on targetedListEnabled -- it does not, and the unconditional container creation is deliberate (the mover needs an anchor), so the behaviour was right and only the stated reason was wrong. One that would have misled a reader looking for a bug: "Only process valid unit types (nameplate, boss, arena)" sits above a check that accepts nameplates and rejects boss and arena. The real reasoning is on IsValidCasterUnit twenty lines up. Also drops the write-only DF.personalTargetedSpellsContainer export (not to be confused with the Mover, which is read from Position.lua), an unused UnitGUID upvalue left from the move to token-based dedup, and a shadowed local db.

Handle secret cooldown values in aura countdown

1aa163c

DanderBot added the codex label Dec 1, 2025 — with ChatGPT Codex Connector

Merge branch 'main' into codex/add-countdown-formatting-options-for-b…

343dd84

…uffs-sk77o8

DanderBot merged commit b431b3e into main Dec 1, 2025

DanderBot deleted the codex/add-countdown-formatting-options-for-buffs-sk77o8 branch February 14, 2026 13:29

Navigation Menu

Safely handle secret values in aura countdown updates - #5

Safely handle secret values in aura countdown updates#5
DanderBot merged 2 commits into
mainfrom
codex/add-countdown-formatting-options-for-buffs-sk77o8

DanderBot commented Dec 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

DanderBot commented Dec 1, 2025

Summary

Testing

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant