[agents] favicons enhancements - #1126
Conversation
Introduce a new API route to retrieve favicons for source documents via sources/{sourceId}/favicon.ico. Implement GetActualSourceFavicon handler and GetFaviconUrlAsync helper using HtmlAgilityPack for HTML parsing. Add HtmlAgilityPack NuGet dependency. Update Angular ChatCitationsComponent to use Google's favicon service with backend fallback. Backend handler redirects to detected favicon URL or /favicon.ico for local sources.
Added necessary using directives to OutOfScopeResponder.cs. Updated HandleAsync to be async, awaiting context.AddEventAsync to log AgentResponseUpdateEvent before returning results. Replaced ValueTask.FromResult with awaited event addition.
Expanded `IntentClassifier` and `PurposeResponder` prompts to include internal documentation about Indice and its products (e.g., IAM), as well as banking institution documentation. Clarified assistant's knowledge scope for improved accuracy.
- Changed source favicon route to /sources/{sourceId}/favicon
- Added /favicons endpoint to fetch favicon by domain
- Return SVG globe icon if favicon not found, no redirect
- Added GetFaviconFor handler for domain-based favicon retrieval
- FaviconHelper now returns null if favicon is missing
- Added GlobeSvg as fallback image
- Improved error handling and response types
Updated VersionSuffix from beta14 to beta15 in the .csproj files for Indice.Features.Agents.Core, Indice.Features.Agents.Server, and Indice.Features.Agents.UI. No other changes were made.
There was a problem hiding this comment.
Pull request overview
This PR enhances favicon handling for Agents by switching the UI to use Google’s favicon service for more reliable client-side resolution, while also adding server-side favicon resolution helpers and endpoints for fidelity testing/fallback scenarios. It also includes a small workflow UX improvement (emitting response updates for out-of-scope replies) and bumps Agents package versions.
Changes:
- Update chat citations UI to resolve favicons via Google’s favicon service with
/favicon.icofallback. - Add server-side favicon resolution helper + two anonymous endpoints for resolving/redirecting to favicons.
- Emit
AgentResponseUpdateEventfor out-of-scope responses; update prompt text; bumpbeta14→beta15.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Indice.Features.Agents.UI/Indice.Features.Agents.UI.csproj | Bumps Agents UI package suffix to beta15. |
| src/Indice.Features.Agents.Server/Services/FaviconHelper.cs | Adds HTML-based favicon discovery helper (server-side resolver). |
| src/Indice.Features.Agents.Server/Indice.Features.Agents.Server.csproj | Adds HtmlAgilityPack dependency and bumps suffix to beta15. |
| src/Indice.Features.Agents.Server/Endpoints/SourcesHandlers.cs | Adds favicon endpoints/handlers and globe SVG fallback. |
| src/Indice.Features.Agents.Server/Endpoints/SourcesApi.cs | Maps the new favicon endpoints (sources/{sourceId}/favicon, favicons). |
| src/Indice.Features.Agents.Core/Workflows/Steps/OutOfScopeResponder.cs | Emits response update events for out-of-scope replies (consistent with other steps). |
| src/Indice.Features.Agents.Core/Indice.Features.Agents.Core.csproj | Bumps Agents Core package suffix to beta15. |
| src/Indice.Features.Agents.Core/AgentsConstants.cs | Updates prompt wording for intent/purpose defaults. |
| src/Indice.Features.Agents.App/src/app/features/chat/chat-citations.component.ts | Uses Google favicon service for citation source origin favicons. |
Comments suppressed due to low confidence (2)
src/Indice.Features.Agents.Server/Endpoints/SourcesHandlers.cs:63
new UriBuilder(domain)will throw for common inputs likeexample.com(no scheme), which will surface as a 500. This endpoint is also an SSRF vector because it triggers server-side fetches for user-supplied domains; at minimum, normalize/validate the host and reject IP-literals/loopback, and avoid permanent redirects.
var uriBuilder = new UriBuilder(domain) {
Scheme = "https", // Ensure HTTPS scheme
Port = 443
};
src/Indice.Features.Agents.Server/Services/FaviconHelper.cs:45
- The XML doc says this falls back to
/favicon.ico, but when no<link rel=...icon...>is found it currently returnsnullinstead. This makes callers treat “no icon tags” as a hard failure and skip the conventional favicon location.
}
return null;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
src/Indice.Features.Agents.Server/Endpoints/SourcesHandlers.cs:9
- There are several unused
usingdirectives added here (System.Runtime.Intrinsics.Arm,Microsoft.AspNetCore.Rewrite,SixLabors.ImageSharp.Drawing). They aren’t referenced in this file and add noise (and can produce build warnings depending on settings).
using System.Runtime.Intrinsics.Arm;
using System.Security.Claims;
using Indice.Features.Agents.Core.Services;
using Indice.Features.Agents.Server.Services;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.HttpResults;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Rewrite;
using SixLabors.ImageSharp.Drawing;
src/Indice.Features.Agents.Server/Endpoints/SourcesHandlers.cs:53
GetActualSourceFaviconignoresdocument.IsPrivateandcurrentUser(unlikeGetActualSource). This makes it possible for anonymous callers to probe existence of private sources and trigger outbound HTTP requests/redirects based on them. Consider returning 401 for private docs when the user isn’t authenticated, and avoid a permanent redirect since favicon targets can change.
public static async Task<Results<ContentHttpResult, NotFound, RedirectHttpResult>> GetActualSourceFavicon(Guid sourceId, ClaimsPrincipal currentUser, IDocumentsService documentsService, IHttpClientFactory httpClientFactory, CancellationToken cancellationToken) {
var document = await documentsService.FindBySourceAsync(sourceId.ToString(), includeData: false, cancellationToken: cancellationToken);
if (document is null) {
return TypedResults.NotFound();
}
if (document.Source.StartsWith("local://", StringComparison.OrdinalIgnoreCase)) {
return TypedResults.Redirect("/favicon.ico"); // Favicon retrieval is not supported for local sources
}
var httpClient = httpClientFactory.CreateClient("favicon");
var faviconUrl = await httpClient.GetFaviconUrlAsync(document.Source, cancellationToken: cancellationToken);
if (string.IsNullOrEmpty(faviconUrl)) {
return TypedResults.Content(content: GlobeSvg, contentType: "image/svg+xml", contentEncoding: System.Text.Encoding.UTF8);
}
// Implementation for retrieving the favicon of the actual source document
return TypedResults.Redirect(faviconUrl, permanent: true);
}
src/Indice.Features.Agents.Server/Services/FaviconHelper.cs:28
- The XML docs say this method “fetches the page” and “falls back to /favicon.ico”, but the implementation currently fetches only the site root (
UriPartial.Authority) and returnsnullwhen no icon is found. Also, theHttpResponseMessageshould be disposed. Either adjust the docs/contract or align the implementation with the described behavior.
/// <summary>
/// Retrieves the favicon URL for a given page URL. It first attempts to fetch the page and look for any <link> elements that specify an icon.
/// If none are found, it falls back to the default /favicon.ico path.
/// </summary>
/// <param name="httpClient">The HttpClient instance used to fetch the page.</param>
/// <param name="pageUrl">The URL of the page for which to retrieve the favicon.</param>
/// <param name="cancellationToken">A cancellation token to cancel the operation.</param>
/// <returns>The URL of the favicon.</returns>
public static async Task<string?> GetFaviconUrlAsync(this HttpClient httpClient, string pageUrl, CancellationToken cancellationToken = default) {
ArgumentException.ThrowIfNullOrWhiteSpace(pageUrl);
var baseUri = new Uri(new Uri(pageUrl).GetLeftPart(UriPartial.Authority));
var response = await httpClient.GetAsync(baseUri, cancellationToken);
if (!response.IsSuccessStatusCode) {
return null; // Favicon retrieval failed, redirect to default favicon
}
// Load the page
HtmlDocument doc = new HtmlDocument();
doc.LoadHtml(await response.Content.ReadAsStringAsync(cancellationToken));
src/Indice.Features.Agents.Core/AgentsConstants.cs:38
- The updated
IntentClassifierprompt text has a duplicated line (“internal documentation about Indice…” appears twice) and inconsistent capitalization/wording (“Its”, missing punctuation). This can reduce prompt clarity for the classifier.
questions based on its context, which is currently comprised of
- internal documentation about Indice and Its products for example IAM
- Banking Institution internal documentation about their banking services
internal documentation about Indice and random general facts about the world.
src/Indice.Features.Agents.Server/Endpoints/SourcesHandlers.cs:71
new UriBuilder(domain)will throw for common inputs likeexample.com(no scheme) and currently this handler doesn’t catch/validate, so a malformeddomainquery can cause a 500. Consider parsing withUri.TryCreate, normalizing to HTTPS, and rejecting loopback/unknown hosts before making outbound requests.
public static async Task<Results<ContentHttpResult, RedirectHttpResult>> GetFaviconFor([FromQuery] string? domain, IHttpClientFactory httpClientFactory, CancellationToken cancellationToken) {
if (string.IsNullOrWhiteSpace(domain) ||
domain.Contains("localhost", StringComparison.OrdinalIgnoreCase)) {
return TypedResults.Content(content: GlobeSvg, contentType: "image/svg+xml", contentEncoding: System.Text.Encoding.UTF8);
}
var uriBuilder = new UriBuilder(domain) {
Scheme = "https", // Ensure HTTPS scheme
Port = 443
};
var httpClient = httpClientFactory.CreateClient("favicon");
var faviconUrl = await httpClient.GetFaviconUrlAsync(uriBuilder.ToString(), cancellationToken: cancellationToken);
if (faviconUrl == null) {
return TypedResults.Content(content: GlobeSvg, contentType: "image/svg+xml", contentEncoding: System.Text.Encoding.UTF8);
}
// Implementation for retrieving the favicon of the actual source document
return TypedResults.Redirect(faviconUrl, permanent: true);
}
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Refined the IntentClassifier string in AgentsConstants.cs by removing a duplicate mention of "internal documentation about Indice" and clarifying the list of context items. Now, "Banking Institution internal documentation about their banking services" and "Random general facts about the world" are listed as distinct items.
…dice.Platform into fix/agents/favicons
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (8)
src/Indice.Features.Agents.Server/Services/FaviconHelper.cs:48
- The XML summary says the method falls back to "/favicon.ico" when no is found, but the implementation returns null. This forces callers into their own fallback behavior and diverges from the documented contract.
return null;
src/Indice.Features.Agents.Server/Endpoints/SourcesHandlers.cs:36
- GetActualSourceFavicon retrieves SourceDocument (including IsPrivate) and accepts currentUser, but it never enforces privacy. As written, unauthenticated callers can probe private sourceIds and trigger server-side requests to the stored Source URL.
public static async Task<Results<ContentHttpResult, NotFound, RedirectHttpResult>> GetActualSourceFavicon(Guid sourceId, ClaimsPrincipal currentUser, IDocumentsService documentsService, IHttpClientFactory httpClientFactory, CancellationToken cancellationToken) {
var document = await documentsService.FindBySourceAsync(sourceId.ToString(), includeData: false, cancellationToken: cancellationToken);
if (document is null) {
return TypedResults.NotFound();
src/Indice.Features.Agents.Server/Endpoints/SourcesHandlers.cs:49
- These favicon URLs can change over time; returning a permanent (301) redirect risks clients caching a stale favicon indefinitely. Prefer a temporary redirect and rely on server-side caching/output caching if needed.
return TypedResults.Redirect(faviconUrl, permanent: true);
src/Indice.Features.Agents.Server/Endpoints/SourcesHandlers.cs:67
- As with GetActualSourceFavicon, a permanent (301) redirect here can cause long-lived stale caching of favicons on the client side. Use a temporary redirect and keep caching concerns on the server/CDN layer.
return TypedResults.Redirect(faviconUrl, permanent: true);
src/Indice.Features.Agents.Core/AgentsConstants.cs:52
- This line reads ungrammatically ("regarding the Indice...") and is slightly redundant. Since this is a user-facing capability prompt, tightening the wording helps produce more consistent responses.
regarding the Indice and its products, for example IAM, Indice's identity provider.
src/Indice.Features.Agents.Server/Services/FaviconHelper.cs:25
- This helper parses the HTML to discover favicon tags, but it currently fetches only the site authority (baseUri) instead of the provided pageUrl. That makes the result incorrect for sites whose favicon is declared only on specific pages (and also contradicts the method summary).
This issue also appears on line 48 of the same file.
using var response = await httpClient.GetAsync(baseUri, cancellationToken);
src/Indice.Features.Agents.Core/AgentsConstants.cs:38
- The updated IntentClassifier prompt has a duplicated/contradictory line and inconsistent capitalization ("Its"). This reduces clarity for the model and may hurt intent classification consistency.
This issue also appears on line 52 of the same file.
questions based on its context, which is currently comprised of
- internal documentation about Indice and Its products for example IAM
- Banking Institution internal documentation about their banking services
- Random general facts about the world.
src/Indice.Features.Agents.App/src/app/features/chat/chat-citations.component.ts:115
- The JSDoc says the method always returns a string, but the implementation can return null. Also, hostname should be URL-encoded before interpolation into the Google service URL.
* Get favicon URL from a domain using Google's favicon service
* @param {string} domain - The domain name or full URL
* @returns {string} - Direct favicon URL
*/
getFaviconUrl(domain: string) : string | null {
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
src/Indice.Features.Agents.Server/Endpoints/SourcesHandlers.cs:56
- GetFaviconFor accepts arbitrary user input and then performs server-side HTTP fetching (via GetFaviconUrlAsync). The current validation only blocks strings containing "localhost", which still allows SSRF against internal/private hosts (including direct IPs like 10.0.0.0/8, 169.254.0.0/16, etc.). Restrict input to a host (or https origin) and reject loopback/private IP literals before any outbound request.
public static async Task<Results<ContentHttpResult, RedirectHttpResult>> GetFaviconFor([FromQuery] string? domain, IHttpClientFactory httpClientFactory, CancellationToken cancellationToken) {
if (string.IsNullOrWhiteSpace(domain) ||
domain.Contains("localhost", StringComparison.OrdinalIgnoreCase)) {
return TypedResults.Content(content: GlobeSvg, contentType: "image/svg+xml", contentEncoding: System.Text.Encoding.UTF8);
}
src/Indice.Features.Agents.Server/Endpoints/SourcesHandlers.cs:36
- GetActualSourceFavicon doesn’t enforce the same privacy rule as GetActualSource: a private document can be probed anonymously (and may disclose source info via redirects). Add an IsPrivate/authenticated check and return 401 for unauthenticated callers.
public static async Task<Results<ContentHttpResult, NotFound, RedirectHttpResult>> GetActualSourceFavicon(Guid sourceId, ClaimsPrincipal currentUser, IDocumentsService documentsService, IHttpClientFactory httpClientFactory, CancellationToken cancellationToken) {
var document = await documentsService.FindBySourceAsync(sourceId.ToString(), includeData: false, cancellationToken: cancellationToken);
if (document is null) {
return TypedResults.NotFound();
src/Indice.Features.Agents.Server/Services/FaviconHelper.cs:48
- GetFaviconUrlAsync's XML doc says it falls back to the default /favicon.ico when no is found, but the method currently returns null in that case. This will cause callers to emit the globe fallback even when a standard /favicon.ico exists.
return null;
src/Indice.Features.Agents.Core/AgentsConstants.cs:38
- Minor grammar/capitalization issues in the updated prompt text ("Its" -> "its", inconsistent capitalization in bullets). Since this is prompt content, these details can affect model behavior.
questions based on its context, which is currently comprised of
- internal documentation about Indice and Its products for example IAM
- Banking Institution internal documentation about their banking services
- Random general facts about the world.
src/Indice.Features.Agents.App/src/app/features/chat/chat-citations.component.ts:112
- The Google favicon service URL should URL-encode the hostname to avoid producing an invalid request for edge cases (IDN/punycode, unexpected characters).
// Extract hostname if a full URL is provided
const hostname = new URL(domain).hostname;
return `https://www.google.com/s2/favicons?sz=64&domain=${hostname}`;
} catch (error) {
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (7)
src/Indice.Features.Agents.Server/Endpoints/SourcesHandlers.cs:41
GetActualSourceFaviconbypasses the privacy check used byGetActualSource. As a result, anonymous callers can probe for the existence of private documents and get redirected to (or infer) their source host.
public static async Task<Results<ContentHttpResult, NotFound, RedirectHttpResult>> GetActualSourceFavicon(Guid sourceId, ClaimsPrincipal currentUser, IDocumentsService documentsService, IHttpClientFactory httpClientFactory, CancellationToken cancellationToken) {
var document = await documentsService.FindBySourceAsync(sourceId.ToString(), includeData: false, cancellationToken: cancellationToken);
if (document is null) {
return TypedResults.NotFound();
}
if (document.Source.StartsWith("local://", StringComparison.OrdinalIgnoreCase)) {
return TypedResults.Redirect("/favicon.ico"); // Favicon retrieval is not supported for local sources
}
src/Indice.Features.Agents.Server/Endpoints/SourcesHandlers.cs:49
- Using a permanent redirect (301) for a resolved favicon URL can cause clients/CDNs to cache the redirect for a long time, even if the target favicon changes. A temporary redirect is usually safer for this kind of derived resource.
return TypedResults.Redirect(faviconUrl, permanent: true);
src/Indice.Features.Agents.Server/Endpoints/SourcesHandlers.cs:71
- Using a permanent redirect (301) for a resolved favicon URL can cause clients/CDNs to cache the redirect for a long time, even if the target favicon changes. A temporary redirect is usually safer for this kind of derived resource.
return TypedResults.Redirect(faviconUrl, permanent: true);
src/Indice.Features.Agents.Server/Endpoints/SourcesHandlers.cs:61
GetFaviconForperforms server-side HTTP requests to a user-supplied host. The current validation only blocks literal IP hosts andlocalhost, but it does not prevent DNS rebinding / domains resolving to private or loopback IPs, which is a typical SSRF vector.
var input = domain.Contains("://", StringComparison.Ordinal) ? domain : $"https://{domain}";
if (!Uri.TryCreate(input, UriKind.Absolute, out var uri) || string.IsNullOrWhiteSpace(uri.Host) ||
uri.Host.Contains("localhost", StringComparison.OrdinalIgnoreCase) ||
System.Net.IPAddress.TryParse(uri.Host, out _)) {
return TypedResults.Content(content: GlobeSvg, contentType: "image/svg+xml", contentEncoding: System.Text.Encoding.UTF8);
src/Indice.Features.Agents.Server/Services/FaviconHelper.cs:48
GetFaviconUrlAsyncclaims to fall back to/favicon.ico, but on a successful page fetch with no matching<link rel=...icon...>it returnsnull(and it also fetches only the site authority, not the providedpageUrl). This breaks the documented contract and reduces fidelity for page-specific favicons.
var baseUri = new Uri(pageUri.GetLeftPart(UriPartial.Authority));
using var response = await httpClient.GetAsync(baseUri, cancellationToken);
if (!response.IsSuccessStatusCode) {
return new Uri(baseUri, "/favicon.ico").ToString();
}
src/Indice.Features.Agents.App/src/app/features/chat/chat-citations.component.ts:115
- The hostname is interpolated into a query string without URL-encoding. Even though it usually contains safe characters, encoding avoids malformed URLs for IDNs/punycode and is the correct way to construct query parameters.
getFaviconUrl(domain: string) : string | null {
try {
// Extract hostname if a full URL is provided
const hostname = new URL(domain).hostname;
return `https://www.google.com/s2/favicons?sz=64&domain=${hostname}`;
} catch (error) {
return null;
}
}
src/Indice.Features.Agents.Core/AgentsConstants.cs:38
- Minor grammar/capitalization issues in the prompt text (e.g., "Its" → "its", inconsistent capitalization) can leak into model behavior and reduce prompt clarity.
questions based on its context, which is currently comprised of
- internal documentation about Indice and Its products for example IAM
- Banking Institution internal documentation about their banking services
- Random general facts about the world.
Switched favicon resolution on the browser side to google api. Also created a source favicon resolver and an accompanying endpoint to test the fidelity of resolving the favicons ourselves. This was not so easy as it sounds but we got an alternative in case we need it.