Implement patching for Affinity.CloudServices to block analytics to api.canva.com#13
Implement patching for Affinity.CloudServices to block analytics to api.canva.com#13PGSCOM wants to merge 3 commits into
Conversation
…enhance error handling for Persona assembly #11
There was a problem hiding this comment.
Pull request overview
This PR enhances the Affinity patcher to block analytics by adding support for patching the Affinity.CloudServices.dll assembly in addition to the main Serif.Interop.Persona.dll assembly. The changes improve robustness by gracefully handling missing assemblies with warnings instead of throwing exceptions, and introduce new utility methods for patching void-returning methods.
Key changes:
- Added
PatchCloudServicesAssemblymethod to neutralize analytics/telemetry methods in the CloudServices assembly - Introduced
PatchWithRetandPatchWithRetVerboseutility methods for patching void-returning methods - Refactored assembly patching flow to handle optional assemblies with warnings instead of fatal errors
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| AffinityPatcher/Utils/Patcher.cs | Added two new utility methods (PatchWithRet and PatchWithRetVerbose) to support patching void-returning methods with a simple return instruction |
| AffinityPatcher/Program.cs | Refactored PatchAffinity to handle missing assemblies gracefully; added PatchCloudServicesAssembly to scan for and neutralize analytics-related methods in the CloudServices assembly using name-based heuristics |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var methodsToNeutralize = type.Methods.Where(m => | ||
| m.HasBody && | ||
| (m.Name.StartsWith("Upload") || | ||
| m.Name.StartsWith("Send") || | ||
| m.Name.StartsWith("Track") || | ||
| m.Name.StartsWith("Init") || | ||
| m.Name.Contains("Event"))); |
There was a problem hiding this comment.
The methods identified for neutralization use name-based heuristics (StartsWith/Contains) which could potentially match unrelated methods. For example, methods named "SendMessage" or "InitializeConfiguration" might be patched even if they're not analytics-related. Consider adding additional validation, such as checking method signatures, parameter types, or verifying that the method actually contains analytics-related code patterns before patching.
| var analyticsTypes = module.GetTypes().Where(t => | ||
| t.FullName.Contains("Analytics") || | ||
| t.FullName.Contains("Telemetry") || | ||
| t.FullName.Contains("Snowplow")); | ||
|
|
||
| foreach (var type in analyticsTypes) | ||
| { | ||
| // Search for methods that appear to send or initialize data | ||
| var methodsToNeutralize = type.Methods.Where(m => | ||
| m.HasBody && | ||
| (m.Name.StartsWith("Upload") || | ||
| m.Name.StartsWith("Send") || | ||
| m.Name.StartsWith("Track") || | ||
| m.Name.StartsWith("Init") || | ||
| m.Name.Contains("Event"))); | ||
|
|
||
| foreach (var method in methodsToNeutralize) | ||
| { | ||
| // If it returns void, use a simple Ret | ||
| if (method.ReturnType.ElementType == ElementType.Void) | ||
| { | ||
| Patcher.PatchWithRetVerbose(method.FullName, method.Body, verbose); | ||
| patchedList.Add(method.FullName); | ||
| } | ||
| // If it returns bool, return false (0) | ||
| else if (method.ReturnType.ElementType == ElementType.Boolean) | ||
| { | ||
| Patcher.PatchWithLdcRetVerbose(method.FullName, method.Body, 0, verbose); | ||
| patchedList.Add(method.FullName); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The type-filtering logic searches for types containing "Analytics", "Telemetry", or "Snowplow". However, the code doesn't check if any matching types were actually found before iterating. If no analytics types exist in the assembly, the patching completes silently without any indication that nothing was patched. Consider adding a check after line 239 to log whether any analytics types were found, especially in verbose mode.
| foreach (var type in analyticsTypes) | ||
| { | ||
| // Search for methods that appear to send or initialize data | ||
| var methodsToNeutralize = type.Methods.Where(m => | ||
| m.HasBody && | ||
| (m.Name.StartsWith("Upload") || | ||
| m.Name.StartsWith("Send") || | ||
| m.Name.StartsWith("Track") || | ||
| m.Name.StartsWith("Init") || | ||
| m.Name.Contains("Event"))); | ||
|
|
||
| foreach (var method in methodsToNeutralize) | ||
| { | ||
| // If it returns void, use a simple Ret | ||
| if (method.ReturnType.ElementType == ElementType.Void) | ||
| { | ||
| Patcher.PatchWithRetVerbose(method.FullName, method.Body, verbose); | ||
| patchedList.Add(method.FullName); | ||
| } | ||
| // If it returns bool, return false (0) | ||
| else if (method.ReturnType.ElementType == ElementType.Boolean) | ||
| { | ||
| Patcher.PatchWithLdcRetVerbose(method.FullName, method.Body, 0, verbose); | ||
| patchedList.Add(method.FullName); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This foreach loop immediately maps its iteration variable to another variable - consider mapping the sequence explicitly using '.Select(...)'.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Have you checked the actual IL of the ACS assembly? The LLM generated code is extremely vague about its target. |
|
I´m sorry, I´m starting in C# and in this case I commited without understanding the code. |
This pull request enhances the Affinity patching process by introducing targeted analytics-blocking patches for the
Affinity.CloudServices.dllassembly, in addition to improving feedback and flexibility when required assemblies are missing. The changes also add new utility methods to streamline patching methods that returnvoid.Affinity patching improvements:
Serif.Interop.Persona.dll(the main assembly) and, if present,Affinity.CloudServices.dllto block analytics-related functionality. IfAffinity.CloudServices.dllis not found, a warning is displayed and the process continues, providing better compatibility with different Affinity versions.PatchCloudServicesAssembly, which scans for analytics/telemetry-related types and methods inAffinity.CloudServices.dll, and neutralizes methods that send, track, or initialize analytics data by replacing their bodies with immediate returns.Patcher utility enhancements:
PatchWithRetandPatchWithRetVerboseutility methods inPatcher.csto simplify replacing method bodies with a singleretinstruction for methods that returnvoid, with optional verbose logging for improved traceability.