ApproveParentalLinkCommandHandler silently resets InfoVault (and any unlisted permission) to false #876

Closed
opened 2026-07-27 14:30:27 +00:00 by spikerj · 2 comments
Owner

Latent bug found while designing #873. ApproveParentalLinkCommandHandler.cs:33 builds Permissions = new FeaturePermissions { Chat = true, FileUpload = true, ... } explicitly — any field not listed gets the CLR default, not the entity default. InfoVault has entity default true but ends up false after link approval. Every future permission added with a non-false default will hit the same trap.

Fix: initialize from new FeaturePermissions() and override only the intended fields (or list every field explicitly with the entity defaults), plus a pinning test.

Latent bug found while designing #873. `ApproveParentalLinkCommandHandler.cs:33` builds `Permissions = new FeaturePermissions { Chat = true, FileUpload = true, ... }` explicitly — any field not listed gets the CLR default, not the entity default. `InfoVault` has entity default `true` but ends up `false` after link approval. Every future permission added with a non-false default will hit the same trap. Fix: initialize from `new FeaturePermissions()` and override only the intended fields (or list every field explicitly with the entity defaults), plus a pinning test.
Author
Owner

Re-verified against origin/master — the bug is verbatim intact. Staying open. Recording the exact state so this isn't mistaken for fixed-by-#873:

SpikerSoft.Business/Domain/Profile/Commands/ApproveParentalLink/ApproveParentalLinkCommandHandler.cs:36-46:

childProfile.ParentalControls.Permissions = new FeaturePermissions
{
    Chat = true, FileUpload = true, VideoCall = false, PublicProfile = false,
    SocialFeatures = true, MaxDailyScreenTimeMinutes = 120,
    ExposeAgePublicly = false,
};

InfoVault is not listed, and its entity default is true (ProfileParentalSubDocuments.cs, [JsonProperty("infoVault")] public bool InfoVault { get; set; } = true;) — so link approval still silently flips it to false. The handler even carries a NOTE at :33-35 naming this ticket as open.

Important: the ExposeAgePublicly = false line at :45 was added by #873 and satisfies that ticket's item 9 only. It does not fix this one — it's another hardcoded field in the same wholesale replacement, which is the defect. Any future permission added to FeaturePermissions will be silently zeroed here too.

Remaining work: rebuild from new FeaturePermissions() and override only the fields this flow genuinely intends to set (or enumerate every field with its entity default), plus a pinning test — there is currently no test asserting unlisted permissions survive link approval, which is why this survived #873's review.

Re-verified against `origin/master` — **the bug is verbatim intact.** Staying open. Recording the exact state so this isn't mistaken for fixed-by-#873: `SpikerSoft.Business/Domain/Profile/Commands/ApproveParentalLink/ApproveParentalLinkCommandHandler.cs:36-46`: ```csharp childProfile.ParentalControls.Permissions = new FeaturePermissions { Chat = true, FileUpload = true, VideoCall = false, PublicProfile = false, SocialFeatures = true, MaxDailyScreenTimeMinutes = 120, ExposeAgePublicly = false, }; ``` `InfoVault` is not listed, and its entity default is `true` (`ProfileParentalSubDocuments.cs`, `[JsonProperty("infoVault")] public bool InfoVault { get; set; } = true;`) — so link approval still silently flips it to false. The handler even carries a NOTE at `:33-35` naming this ticket as open. **Important:** the `ExposeAgePublicly = false` line at `:45` was added by #873 and satisfies *that* ticket's item 9 only. It does not fix this one — it's another hardcoded field in the same wholesale replacement, which is the defect. Any future permission added to `FeaturePermissions` will be silently zeroed here too. Remaining work: rebuild from `new FeaturePermissions()` and override only the fields this flow genuinely intends to set (or enumerate every field with its entity default), plus a pinning test — there is currently no test asserting unlisted permissions survive link approval, which is why this survived #873's review.
Author
Owner

Migrated to spikerj/spikersoft-backend#534 as part of the umbrella-tracker breakup.

Verified 2026-08-07 against spikersoft-backend@98102023.
Status: not started — verbatim intact, and #873 added a second hardcoded field to the same wholesale replacement.

Closing here. Work now lives in the repo that holds the fix, so fixes #534 in a PR will auto-close it on merge. The umbrella tracker keeps cross-repo epics only.

— Opus 5 Agent

Migrated to **spikerj/spikersoft-backend#534** as part of the umbrella-tracker breakup. Verified 2026-08-07 against ``spikersoft-backend@98102023``. Status: not started — verbatim intact, and #873 added a second hardcoded field to the same wholesale replacement. Closing here. Work now lives in the repo that holds the fix, so `fixes #534` in a PR will auto-close it on merge. The umbrella tracker keeps cross-repo epics only. — Opus 5 Agent
Sign in to join this conversation.