JonKazama-Hellion/HellionChat
1
0
Fork 0
Code Issues 1 Pull Requests 3 Actions Releases 49 Activity

fix(privacy): the screenshot guard was reading a field that gets wiped on purpose

Browse Source 
This morning's fix hung on TellTarget, and TellTarget is routing state that the
codebase clears deliberately. StripTellBindingOnPromote sets IsTempTab false,
empties TellTarget, and keeps the name -- so a promoted tell tab is called
"Player@World" permanently while carrying neither marker, and falls through both
possible checks. That state survives restarts. A pinned tab whose binding did not
survive a save is the same hole with a different cause; the auto-tell service
logs that case as expected and repairs around it.

The flag is set where the name is built from a partner and is not cleared by
promotion. Renaming clears it, because at that point the user typed it.

Config v26 carries it backwards for tabs that already exist: anything still
holding a tell binding or the temp flag got its name from a partner. Tabs
promoted before this version cannot be recovered -- nothing in the stored data
says where their name came from -- and renaming one has the same effect anyway.

Two more things the header was giving away. Its icon for an auto-tell tab is
derived from the partner and stable across sessions, which is three bits of
linkable information on a picture meant to be shareable; the message path
re-salts its name hashes on every load precisely to avoid that, so screenshot
mode now falls back to a plain envelope. And a world name that is not ASCII --
the CN and KR clients have those, and we ship translations for both -- was being
drawn in the meta face, which carries ASCII and a middle dot. It would have come
out as question marks, the same defect the split was built to prevent.

Plus two that are not privacy: the header had no FontsReady gate, alone among
the drawing components, so its band height and baseline offset were wrong in
exactly the frames this cycle made more common. And a long tab name ran past the
band and got cut mid-glyph at the window edge; it fits now, the way the honorific
header already did it.
This commit is contained in:
JonKazama-Hellion
2026-08-19 12:00:16 +02:00
parent 0399b68d8c
commit 8fea9113b9
8 changed files with 138 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
HellionChat/AutoTellTabsService.cs
+1
View File
@@ -394,6 +394,7 @@ internal sealed class AutoTellTabsService : IDisposable
return new Tab
{
Name = FormatTabName(playerName, worldRowId),
NameCameFromPartner = true,
IsTempTab = true,
AllSenderMessages = true,
TellTarget = new TellTarget(playerName, worldRowId, 0, TellReason.Direct),
HellionChat/Configuration.cs
+8 -1
View File
@@ -35,7 +35,7 @@ public class ConfigKeyBind
[Serializable]
public class Configuration : IPluginConfiguration
{
internal const int LatestVersion = 25;
internal const int LatestVersion = 26;
public int Version { get; set; } = LatestVersion;
@@ -409,6 +409,13 @@ public class Tab
public bool AllSenderMessages;
public TellTarget TellTarget = TellTarget.Empty();
// Set once, where the name is built from a conversation partner. Never
// cleared by promotion, unlike IsTempTab and TellTarget -- both of those are
// routing state and are deliberately wiped when a tab is promoted, while the
// name they produced stays. Screenshot mode reads this, so it has to outlive
// every path that keeps the name but drops the binding.
public bool NameCameFromPartner;
// Per-tab notification sound for messages arriving in an inactive tab.
public bool EnableNotificationSound;
public uint NotificationSoundId = 1;
HellionChat/Plugin.cs
+36 -8
View File
@@ -322,13 +322,41 @@ public sealed class Plugin : IAsyncDalamudPlugin
);
}
// v25 carries no migration step. The schema gate above only refuses
// anything under 16, and Json.NET drops keys it does not recognise on
// load, so the fields v1.12.0 deleted simply stop being written on the
// next save. The bump is documentation, and it has to be consistent:
// the constant and this stamp are two separate places, and changing
// only one gives a config that re-stamps itself on every start.
Config.Version = 25;
// v25 carried no migration step; the bump was documentation.
//
// v26 does. NameCameFromPartner is what screenshot mode reads to decide
// whether a tab name is a person, and a config written before it existed
// has it false on every tab -- including pinned tell tabs, which survive
// reloads and are named "Player@World". Anything still carrying a tell
// binding or the temp flag got its name from a partner, so the flag is
// set from those two.
//
// Tabs promoted before this version are past saving: promotion clears
// both markers and keeps the name, so nothing in the stored data says
// where that name came from. Renaming one clears the flag anyway, which
// is the same outcome the user gets by editing it.
if (Config.Version < 26)
{
var carried = 0;
foreach (var tab in Config.Tabs)
{
if (tab.NameCameFromPartner || (!tab.IsTempTab && tab.TellTarget?.IsSet() != true))
continue;
tab.NameCameFromPartner = true;
carried++;
}
if (carried > 0)
{
Log.Information(
$"Marked {carried} tab(s) as partner-named during the v26 migration, so "
+ "screenshot mode hides them in the channel header."
);
}
}
Config.Version = 26;
// Unpinned TempTabs are session-only and dropped on every load. Pinned
// TempTabs survive reload — Jin's tester feedback (v1.4.7).
@@ -491,7 +519,7 @@ public sealed class Plugin : IAsyncDalamudPlugin
new SelfTests.SettingsWindowOpenStep(this),
new SelfTests.OnOpenMainUiRoutesMainWindowStep(this),
new SelfTests.TypingIpcStateStep(this),
new SelfTests.ConfigMigrationV25Step(this),
new SelfTests.ConfigMigrationV26Step(this),
new SelfTests.DbGateWiringStep(this),
new SelfTests.ChannelPopoutBindStep(this),
new SelfTests.HoverStateFootprintStep(),
HellionChat/SelfTests/ConfigMigrationV25Step.cs → HellionChat/SelfTests/ConfigMigrationV26Step.cs
+5 -5
View File
@@ -8,20 +8,20 @@ namespace HellionChat.SelfTests;
// below must carry valid values here. This probe never rewrites config; the
// migrations themselves are load-time and verified by the prepared-config smoke
// in the plan.
internal sealed class ConfigMigrationV25Step : ISelfTestStep
internal sealed class ConfigMigrationV26Step : ISelfTestStep
{
public ConfigMigrationV25Step(Plugin plugin)
public ConfigMigrationV26Step(Plugin plugin)
{
_ = plugin;
}
public string Name => "Hellion Chat - Config v25 migration";
public string Name => "Hellion Chat - Config v26 migration";
public SelfTestStepResult RunStep()
{
if (Plugin.Config.Version != 25)
if (Plugin.Config.Version != 26)
{
ImGui.Text($"Config.Version is {Plugin.Config.Version}, expected 25");
ImGui.Text($"Config.Version is {Plugin.Config.Version}, expected 26");
return SelfTestStepResult.Fail;
}
HellionChat/Ui/Components/MessageList.cs
+20 -8
View File
@@ -436,7 +436,7 @@ internal sealed class MessageList
// change height after its first measurement, so last frame's value is
// this frame's value. That is what lets the surface go down before
// the text instead of needing a draw-channel detour.
DrawRowSurface(heights[i]);
DrawRowSurface(msg, heights[i]);
drawRow(msg);
if (frozen)
@@ -461,24 +461,30 @@ internal sealed class MessageList
// which is why it takes two rectangles: AddRectFilledMultiColor has no
// rounding parameter, so the rounded base goes down first and the gradient
// sits inside it.
private void DrawRowSurface(float height)
private void DrawRowSurface(Message message, float height)
{
if (height <= 0f)
return;
var top = ImGui.GetCursorScreenPos();
PaintRowSurface(ImGui.GetWindowDrawList(), top, height, direct: true);
PaintRowSurface(ImGui.GetWindowDrawList(), message, top, height, direct: true);
}
private void FillRowSurface(Vector2 top, float height)
private void FillRowSurface(Message message, Vector2 top, float height)
{
if (height <= 0f)
return;
PaintRowSurface(ImGui.GetWindowDrawList(), top, height, direct: false);
PaintRowSurface(ImGui.GetWindowDrawList(), message, top, height, direct: false);
}
private void PaintRowSurface(ImDrawListPtr dl, Vector2 top, float height, bool direct)
private void PaintRowSurface(
ImDrawListPtr dl,
Message message,
Vector2 top,
float height,
bool direct
)
{
var scale = StyleEngine.Metrics.Scale;
var width = ImGui.GetContentRegionAvail().X;
@@ -489,7 +495,13 @@ internal sealed class MessageList
var max = top + new Vector2(width, height);
var hovered = ImGui.IsWindowHovered() && ImGui.IsMouseHoveringRect(min, max);
var key = (uint)HashCode.Combine(top.Y, height);
// Keyed on the message, not on where it happens to sit. A screen
// coordinate changes every frame while scrolling, so each row would get a
// fresh entry starting at zero and the highlight would never fade in at
// all. It would also let a row in the main window and one in a pop-out
// share an entry whenever their y and height matched.
var key = (uint)message.Id.GetHashCode();
var amount = StyleEngine.HoverState.Query(key, hovered);
if (amount <= 0.01f)
return;
@@ -537,7 +549,7 @@ internal sealed class MessageList
var after = ImGui.GetCursorPosY();
var height = after - before;
FillRowSurface(top, height);
FillRowSurface(msg, top, height);
msg.Height[tabId] = height;
msg.IsVisible[tabId] = ImGui.IsItemVisible();
HellionChat/Ui/Components/Settings/TabEditor.cs
+2
View File
@@ -132,6 +132,8 @@ internal sealed class TabEditor
if (ImGui.InputText(Language.Options_Tabs_Name, ref name, 512))
{
tab.Name = name;
// The user typed this one, so it is no longer a partner's name.
tab.NameCameFromPartner = false;
MarkDirty();
}
HellionChat/Ui/StyleEngine/Widgets/ChannelHeader.cs
+43 -8
View File
@@ -54,6 +54,14 @@ internal static class ChannelHeader
float reservedBelow
)
{
// Every other drawing component gates on this. Without it the band is
// measured against whatever face happens to be active, and a handle that
// is not ready pushes nothing at all -- silently -- so the height, the
// drop-out rule and the baseline offset would all be wrong for those
// frames. This cycle made rebuilds more frequent, not less.
if (!fonts.FontsReady)
return;
var scale = Metrics.Scale;
var width = ImGui.GetContentRegionAvail().X;
var height = Height;
@@ -65,14 +73,28 @@ internal static class ChannelHeader
var body = BodyFace(fonts);
var meta = MetaFace(fonts);
var icon = Components.Sidebar.ResolveTabIcon(tab);
var track = TrackRaw * scale;
var detailTrack = DetailTrackRaw * scale;
// Screenshot mode hides the name of a tell tab, because that name IS the
// conversation partner: AutoTellTabsService builds it as "Player@World".
// Drawing it in tracked caps above a log whose messages are anonymised
// would give away in the header exactly what the log is hiding.
var namesAPartner = tab.TellTarget?.IsSet() == true;
// Screenshot mode hides a name that came from a conversation partner:
// AutoTellTabsService builds those as "Player@World", and drawing one in
// tracked caps above a log whose messages are anonymised would give away
// in the header exactly what the log is hiding.
//
// Reading TellTarget or IsTempTab here would miss the case that matters
// most. StripTellBindingOnPromote clears both and keeps the name, so a
// promoted tell tab is called "Player@World" for good while carrying
// neither marker. The flag is set where the name is built and survives
// that.
var namesAPartner = tab.NameCameFromPartner;
// The tab icon for an auto-tell tab is derived from the partner and is
// stable across sessions, so it is three bits of linkable information on
// a picture meant to be shareable. The message path guards against
// exactly that by re-salting its name hashes on every plugin load.
var icon =
Plugin.Config.ScreenshotMode && namesAPartner
? Dalamud.Interface.FontAwesomeIcon.Envelope
: Components.Sidebar.ResolveTabIcon(tab);
var showName =
mode is ChannelHeaderMode.Full && !(Plugin.Config.ScreenshotMode && namesAPartner);
@@ -83,12 +105,26 @@ internal static class ChannelHeader
using (fonts.FontAwesome.Push())
iconSize = ImGui.CalcTextSize(icon.ToIconString());
var inset = InsetRaw * scale;
var iconRun = iconSize.X + IconGapRaw * scale;
var nameRun = 0f;
if (showName)
{
// Tab names are free user input, and an auto-tell name is
// "Firstname Lastname@World". Left alone it runs past the band and
// gets cut mid-glyph at the window edge; the drop-out rule only
// handles the two runs overlapping, not one of them overflowing.
var room = width - inset * 2f - iconRun;
using (body.Push())
{
if (DrawListExtensions.MeasureTrackedText(name, track) > room)
name = StringUtil.TruncateToFitWidth(name, room);
nameRun = DrawListExtensions.MeasureTrackedText(name, track);
nameRun += iconSize.X + IconGapRaw * scale;
}
nameRun += iconRun;
}
// Measured under the face that will draw it, which is the whole point of
@@ -110,7 +146,6 @@ internal static class ChannelHeader
restRun = DrawListExtensions.MeasureTrackedText(rest, detailTrack);
var detailRun = whereRun + restRun;
var inset = InsetRaw * scale;
var plan = ChannelHeaderLayout.Plan(
showName ? ChannelHeaderMode.Full : ChannelHeaderMode.DetailOnly,
HellionChat/Ui/StyleEngine/Widgets/ChannelHeaderDetail.cs
+23 -1
View File
@@ -11,6 +11,12 @@ namespace HellionChat.Ui.StyleEngine.Widgets;
// meta face they would come out as rows of question marks. So the caller draws
// WhereIsTranslated text in the body face and the rest in the meta face.
//
// WhereIsTranslated is really "this needs the body face". Two things set it: the
// stand-in for an unknown world, which is translated into 25 languages, and a
// world name that is not plain ASCII. The CN and KR clients have those, and the
// plugin ships zh-Hans, zh-Hant and ko -- so "world names are Latin everywhere"
// would have been true for four regions and wrong for two.
//
// The clock follows the same Use24HourClock setting the message timestamps do.
// Two clock formats in one window, with the header sitting directly above a
// column of timestamps, would be a defect rather than a preference. Culture is
@@ -46,6 +52,22 @@ internal static class ChannelHeaderDetail
return new ChannelHeaderDetailParts(string.Empty, clock, false);
var missing = string.IsNullOrWhiteSpace(world);
return new ChannelHeaderDetailParts(missing ? fallback : world!, clock, missing);
if (missing)
return new ChannelHeaderDetailParts(fallback, clock, true);
return new ChannelHeaderDetailParts(world!, clock, !IsPlainAscii(world!));
}
// The meta face carries ASCII plus a middle dot and nothing else. Eight
// characters to scan, once per frame, against a row of question marks.
private static bool IsPlainAscii(string text)
{
foreach (var c in text)
{
if (c is < ' ' or > '~')
return false;
}
return true;
}
}