fix(privacy): let the channel grid actually decide what is stored
IsAllowedForStorage applied the unknown-channel failsafe to known channels too. Untick Say in the grid, leave "Save unknown channel types" on, and Say kept being written -- and that failsafe is on by default. So a config that never met the wizard ran with the filter enabled, an empty list and the failsafe on, which stored everything while the filter's own description promised "only messages from allowed channels are written to the database". The grid was inert for exactly the users who had not been walked through the wizard. The rule now reads: on the list, or unknown and the failsafe allows it. A known channel the user did not pick stays out. That correction alone would turn "stores everything" into "stores nothing" for those same configs, so two things move with it: - Config v24 switches the filter off where it was on with nothing picked. Same behaviour as before, stated where the user can see it, and one line in the log saying so. A config that does have picks keeps them and starts honouring them, which is the point of the change. - A fresh config seeds the list from PrivacyFirstWhitelist instead of starting empty. Privacy by Default was already the documented intent; it just relied on the hole to stay usable. The rule lives in its own type now. Configuration implements a Dalamud interface, and the build suite cannot load Dalamud.dll -- the runtime resolves the declaring type before reaching the method body, so even a static call on it fails. Fifteen cases pin the truth table and the migration condition; the self-test checks the running config is not in the state the migration exists to undo.
This commit is contained in:
@@ -35,7 +35,7 @@ public class ConfigKeyBind
|
||||
[Serializable]
|
||||
public class Configuration : IPluginConfiguration
|
||||
{
|
||||
internal const int LatestVersion = 23;
|
||||
internal const int LatestVersion = 24;
|
||||
|
||||
public int Version { get; set; } = LatestVersion;
|
||||
|
||||
@@ -59,8 +59,15 @@ public class Configuration : IPluginConfiguration
|
||||
// Privacy by Default master switch. Set false to restore upstream behaviour.
|
||||
public bool PrivacyFilterEnabled = true;
|
||||
|
||||
// Empty set means the migration has not run yet — see Plugin.cs v6→v7.
|
||||
public HashSet<ChatType> PrivacyPersistChannels = [];
|
||||
// Privacy by Default (DSGVO Art. 25): a config that never met the wizard
|
||||
// records the player's own conversations and nothing else. Before v1.12.0
|
||||
// this started empty, which was harmless only because the failsafe below
|
||||
// overrode it and stored everything anyway. With the corrected rule an empty
|
||||
// list means an empty database, so the default has to state the intent.
|
||||
public HashSet<ChatType> PrivacyPersistChannels =
|
||||
[
|
||||
.. Privacy.PrivacyDefaults.PrivacyFirstWhitelist,
|
||||
];
|
||||
|
||||
// Failsafe for ChatTypes added by future FFXIV patches. New configs default
|
||||
// to the failsafe via PrivacyDefaults; existing configs keep their saved
|
||||
@@ -84,17 +91,15 @@ public class Configuration : IPluginConfiguration
|
||||
// same set from the draw thread. A HashSet.Contains racing an Add that
|
||||
// resizes buckets can return the wrong answer -- and this answer decides
|
||||
// whether a message is persisted. Lock kept tight, this is a hot path.
|
||||
bool persists;
|
||||
bool listed;
|
||||
lock (Plugin.Instance.ConfigMapsLock)
|
||||
persists = PrivacyPersistChannels.Contains(type);
|
||||
if (persists)
|
||||
return true;
|
||||
listed = PrivacyPersistChannels.Contains(type);
|
||||
|
||||
var known = Enum.IsDefined(typeof(ChatType), type);
|
||||
|
||||
// F3.2: log first occurrence of a ChatType the running build doesn't
|
||||
// recognise — i.e. one a future FFXIV patch may have added. Known
|
||||
// types the user opted out of are routed through the failsafe
|
||||
// silently, like before.
|
||||
if (!Enum.IsDefined(typeof(ChatType), type) && _warnedUnknownChannels.Add(type))
|
||||
// recognise — i.e. one a future FFXIV patch may have added.
|
||||
if (!known && !listed && _warnedUnknownChannels.Add(type))
|
||||
{
|
||||
Plugin.LogProxy.Warning(
|
||||
"PrivacyFilter: unrecognised ChatType {Type} — falling back to PrivacyPersistUnknownChannels={Persist}.",
|
||||
@@ -103,7 +108,7 @@ public class Configuration : IPluginConfiguration
|
||||
);
|
||||
}
|
||||
|
||||
return PrivacyPersistUnknownChannels;
|
||||
return Privacy.StorageRule.Allows(listed, known, PrivacyPersistUnknownChannels);
|
||||
}
|
||||
|
||||
// Retention master switch defaults to false — plugin will not delete
|
||||
|
||||
+28
-2
@@ -283,7 +283,33 @@ public sealed class Plugin : IAsyncDalamudPlugin
|
||||
{
|
||||
Config.MainWindowLayoutMode = MainWindowLayoutMode.TopTabs;
|
||||
}
|
||||
Config.Version = 23;
|
||||
|
||||
// v24 migration: the privacy filter used to route a known but unticked
|
||||
// channel through the unknown-type failsafe, so the channel grid was
|
||||
// inert whenever that failsafe was on. Corrected in v1.12.0. A config
|
||||
// that never picked a channel was storing everything through that hole,
|
||||
// and the corrected rule would store nothing at all -- so the intent is
|
||||
// carried forward as a filter that is honestly switched off.
|
||||
if (
|
||||
Config.Version < 24
|
||||
&& Privacy.StorageRule.ShouldDisableFilterOnV24(
|
||||
Config.PrivacyFilterEnabled,
|
||||
Config.PrivacyPersistUnknownChannels,
|
||||
Config.PrivacyPersistChannels.Count
|
||||
)
|
||||
)
|
||||
{
|
||||
Config.PrivacyFilterEnabled = false;
|
||||
// Log, not LogProxy: this runs in Phase-0 and the proxy is only
|
||||
// resolved from the container further down.
|
||||
Log.Information(
|
||||
"Privacy filter switched off during the v24 migration: it was on with no channels "
|
||||
+ "picked, which stored everything through the unknown-channel failsafe. Pick "
|
||||
+ "channels in Settings to switch it back on."
|
||||
);
|
||||
}
|
||||
|
||||
Config.Version = 24;
|
||||
|
||||
// Unpinned TempTabs are session-only and dropped on every load. Pinned
|
||||
// TempTabs survive reload — Jin's tester feedback (v1.4.7).
|
||||
@@ -444,7 +470,7 @@ public sealed class Plugin : IAsyncDalamudPlugin
|
||||
new SelfTests.SettingsWindowOpenStep(this),
|
||||
new SelfTests.OnOpenMainUiRoutesMainWindowStep(this),
|
||||
new SelfTests.TypingIpcStateStep(this),
|
||||
new SelfTests.ConfigMigrationV23Step(this),
|
||||
new SelfTests.ConfigMigrationV24Step(this),
|
||||
new SelfTests.ChannelPopoutBindStep(this),
|
||||
new SelfTests.HoverStateFootprintStep(),
|
||||
new SelfTests.HonorificHeaderRenderStep(this),
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
namespace HellionChat.Privacy;
|
||||
|
||||
// The rule that decides whether a message is written to disk, as plain logic.
|
||||
//
|
||||
// It lives apart from Configuration because Configuration implements a Dalamud
|
||||
// interface, and the build suite cannot load Dalamud.dll -- the runtime resolves
|
||||
// the declaring type before it ever reaches the method body, so even a static
|
||||
// call on it fails. This is the single most consequential branch in the plugin,
|
||||
// and it belongs where it can be pinned.
|
||||
internal static class StorageRule
|
||||
{
|
||||
// v1.12.0 corrected the last term. A known channel the user had unticked
|
||||
// used to fall through to the unknown-type failsafe, so the channel grid did
|
||||
// nothing at all whenever that failsafe was on. It is on by default, so the
|
||||
// filter stored everything while its own description promised "only messages
|
||||
// from allowed channels are written to the database".
|
||||
internal static bool Allows(bool listed, bool knownType, bool persistUnknownTypes) =>
|
||||
listed || (!knownType && persistUnknownTypes);
|
||||
|
||||
// Carry-over for configs written before that correction. Where the failsafe
|
||||
// made the list irrelevant and no channel was ever picked, the old rule
|
||||
// stored everything; the corrected rule would store nothing. Switching the
|
||||
// filter off keeps the behaviour and states it where the user can see it.
|
||||
//
|
||||
// A config that does have picks keeps them and starts honouring them, which
|
||||
// is the point of the change.
|
||||
internal static bool ShouldDisableFilterOnV24(
|
||||
bool filterEnabled,
|
||||
bool persistUnknownTypes,
|
||||
int listedCount
|
||||
) => filterEnabled && persistUnknownTypes && listedCount == 0;
|
||||
}
|
||||
+26
-10
@@ -3,25 +3,41 @@ using Dalamud.Plugin.SelfTest;
|
||||
|
||||
namespace HellionChat.SelfTests;
|
||||
|
||||
// Pins the post-migration shape of the v23 config. By /xlperf time the schema
|
||||
// gate has already stamped Config.Version = 23 and run the SidebarTabView→
|
||||
// TopTabs migration, so MainWindowLayoutMode must carry a valid value here.
|
||||
// This probe never rewrites config; the actual migration (false → TopTabs) is
|
||||
// load-time and verified by the prepared-config smoke in the plan.
|
||||
internal sealed class ConfigMigrationV23Step : ISelfTestStep
|
||||
// Pins the post-migration shape of the config. By /xlperf time the schema gate
|
||||
// has already stamped Config.Version and run both migrations, so the fields
|
||||
// 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 ConfigMigrationV24Step : ISelfTestStep
|
||||
{
|
||||
public ConfigMigrationV23Step(Plugin plugin)
|
||||
public ConfigMigrationV24Step(Plugin plugin)
|
||||
{
|
||||
_ = plugin;
|
||||
}
|
||||
|
||||
public string Name => "Hellion Chat - Config v23 migration";
|
||||
public string Name => "Hellion Chat - Config v24 migration";
|
||||
|
||||
public SelfTestStepResult RunStep()
|
||||
{
|
||||
if (Plugin.Config.Version != 23)
|
||||
if (Plugin.Config.Version != 24)
|
||||
{
|
||||
ImGui.Text($"Config.Version is {Plugin.Config.Version}, expected 23");
|
||||
ImGui.Text($"Config.Version is {Plugin.Config.Version}, expected 24");
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
// The state the v24 migration exists to prevent: filter on, failsafe on,
|
||||
// nothing picked. Under the corrected rule that combination stores no
|
||||
// messages at all, so reaching /xlperf in it means the migration did not
|
||||
// run.
|
||||
if (
|
||||
Privacy.StorageRule.ShouldDisableFilterOnV24(
|
||||
Plugin.Config.PrivacyFilterEnabled,
|
||||
Plugin.Config.PrivacyPersistUnknownChannels,
|
||||
Plugin.Config.PrivacyPersistChannels.Count
|
||||
)
|
||||
)
|
||||
{
|
||||
ImGui.Text("Privacy filter is on with no channels picked — v24 migration did not run");
|
||||
return SelfTestStepResult.Fail;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user