Refine html embedding

[iceljc]

PR Type

Enhancement

---

Description

• Add HtmlStyle property with default centering styles

• Add AppendTokenName property for token URL parameters

• Enhance embedding data model for menu customization

---

Diagram Walkthrough

flowchart LR
  A["EmbeddingData Model"] --> B["HtmlStyle Property"]
  A --> C["AppendTokenName Property"]
  B --> D["Default: justify-content center"]
  C --> E["Token URL Parameter Support"]

Loading

File Walkthrough

	Relevant files
Enhancement	PluginMenuDef.cs Add HTML styling and token parameter support                          src/Infrastructure/BotSharp.Abstraction/Plugins/Models/PluginMenuDef.cs Added HtmlStyle property with default flexbox centering styles Added AppendTokenName property for appending bearer tokens to URLs Both properties are JSON-ignored when null Enhanced EmbeddingData class for improved menu customization +12/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🔴	Token in URL Description: AppendTokenName encourages appending a bearer token to the URL query string (e.g., url?tokenName={bearerToken}), which can leak credentials via browser history, server/access logs, Referer headers, analytics, and proxies, enabling token theft and account/session compromise. PluginMenuDef.cs [74-78] Referred Code /// <summary> /// Token name to append after url, e.g., url?tokenName={bearerToken} /// </summary> [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] public string? AppendTokenName { get; set; }
⚪	CSS injection risk Description: HtmlStyle is a free-form string likely to be injected into rendered HTML/CSS; if any part of it is derived from untrusted input, it can enable CSS/style injection (and in some renderers, broader DOM injection), so ensure strict allowlisting/encoding before embedding into markup. PluginMenuDef.cs [56-60] Referred Code /// <summary> /// Html element style /// </summary> [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] public string? HtmlStyle { get; set; } = "justify-content: center; width: 100%; height: 100%;";
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Token-in-URL risk: The new AppendTokenName field implies appending bearer tokens into URLs (query parameters), which may expose sensitive tokens via logs/referrers/history unless downstream code validates and avoids placing secrets in URLs. Referred Code /// <summary> /// Token name to append after url, e.g., url?tokenName={bearerToken} /// </summary> [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] public string? AppendTokenName { get; set; } Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Avoid embedding raw CSS in models Replace the HtmlStyle property, which contains a raw CSS string, with an abstract identifier like a class name. This change will decouple the backend data model from frontend presentation logic. Examples: src/Infrastructure/BotSharp.Abstraction/Plugins/Models/PluginMenuDef.cs [60] public string? HtmlStyle { get; set; } = "justify-content: center; width: 100%; height: 100%;"; Solution Walkthrough: Before: public class EmbeddingData { // ... other properties /// <summary> /// Html element style /// </summary> [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] public string? HtmlStyle { get; set; } = "justify-content: center; width: 100%; height: 100%;"; // ... other properties } After: public class EmbeddingData { // ... other properties /// <summary> /// A style identifier or class name for the html element /// </summary> [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] public string? StyleClass { get; set; } = "default-centered-style"; // ... other properties } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a design flaw where the HtmlStyle property embeds presentation logic (raw CSS) into a data model, violating the separation of concerns principle.	Medium
Possible issue	Compute URL with bearer token Add a helper method GetEmbeddableUrl to correctly construct the final URL by appending the bearer token, handling existing query parameters, and performing URL encoding. src/Infrastructure/BotSharp.Abstraction/Plugins/Models/PluginMenuDef.cs [77-78] -[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] -public string? AppendTokenName { get; set; } +[JsonIgnore] +public string GetEmbeddableUrl(string bearerToken) +{ + if (string.IsNullOrEmpty(Url)) + return Url; + if (string.IsNullOrEmpty(AppendTokenName)) + return Url; + var separator = Url.Contains('?') ? "&" : "?"; + return $"{Url}{separator}{WebUtility.UrlEncode(AppendTokenName)}={WebUtility.UrlEncode(bearerToken)}"; +} Apply / Chat Suggestion importance[1-10]: 7 __ Why: This suggestion enhances the new AppendTokenName feature by adding a robust helper method that encapsulates the URL construction logic, including handling existing query parameters and URL encoding, which improves usability and prevents potential errors.	Medium
	Remove default style to prevent side-effects Remove the default value from the HtmlStyle property to prevent unintended side effects on existing data during deserialization. src/Infrastructure/BotSharp.Abstraction/Plugins/Models/PluginMenuDef.cs [56-60] /// <summary> /// Html element style /// </summary> [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] -public string? HtmlStyle { get; set; } = "justify-content: center; width: 100%; height: 100%;"; +public string? HtmlStyle { get; set; } Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies a potential backward compatibility issue where deserializing old data would apply the new default style, which could cause unintended UI changes. Removing the default value from the data model is a safer approach.	Low
Learned best practice	Normalize empty strings to null Prevent empty/whitespace values from flowing through by normalizing to null, reducing malformed URLs like ?={token} and tightening input handling. src/Infrastructure/BotSharp.Abstraction/Plugins/Models/PluginMenuDef.cs [77-78] +private string? _appendTokenName; + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] -public string? AppendTokenName { get; set; } +public string? AppendTokenName +{ + get => _appendTokenName; + set => _appendTokenName = string.IsNullOrWhiteSpace(value) ? null : value; +} Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Use defensive coding for user-provided string fields by validating/normalizing to prevent incorrect behavior or injection-prone concatenation.	Low
More