Skip to content

Redact query/fragment from URLs sent to model and logs#45

Open
taarikashenafi wants to merge 4 commits into
implement-copilot-quality-altfrom
redact-urls-alt-text-quality
Open

Redact query/fragment from URLs sent to model and logs#45
taarikashenafi wants to merge 4 commits into
implement-copilot-quality-altfrom
redact-urls-alt-text-quality

Conversation

@taarikashenafi

Copy link
Copy Markdown
Contributor

Follow-up to #38.
Builds on the bot's href redaction catch and extends it: pulls the href plus the two console.error image-URL logs in evaluate() (which leak the same querystring data into CI logs) into one reusable redactUrl() helper. Keeps the original link/button wording, the rewording in the bot's inline suggestion is a separate concern.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Not ready to approve

Current logging/model context can still leak sensitive query/fragment data (e.g., page URL not redacted and raw Error objects may still contain full URLs), and the new redaction paths in alt-text-quality lack targeted test coverage.

Pull request overview

This PR introduces a shared URL-redaction helper to prevent URL query strings/fragments (e.g., signed CDN tokens) from being sent to the model context or written into CI logs, and applies it to the alt-text-quality rule’s link context and error logging.

Changes:

  • Add redactUrl() utility to strip query strings and fragments with a safe fallback for non-parseable URLs.
  • Update alt-text-quality to redact link href values and the image URL interpolated into console.error messages.
  • Add unit tests for redactUrl().
File summaries
File Description
src/utils/redact-url.ts Adds the reusable URL redaction helper.
src/rules/alt-text-quality.ts Uses redactUrl() for link href context and error-log URL interpolation.
tests/unit/redact-url.test.ts Adds unit coverage for redactUrl() behavior.

Copilot's findings

  • Files reviewed: 3/3 changed files
  • Comments generated: 4

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 105 to 108
} catch (err) {
console.error(`[alt-text-quality] failed to load ${resolved}:`, err)
console.error(`[alt-text-quality] failed to load ${redactUrl(resolved)}:`, err)
continue
}
Comment on lines 119 to 122
} catch (err) {
console.error(`[alt-text-quality] judge failed for ${resolved}:`, err)
console.error(`[alt-text-quality] judge failed for ${redactUrl(resolved)}:`, err)
continue
}
Comment on lines 49 to 52
if (image.sectionHeading) parts.push(`Nearest heading above the image: ${JSON.stringify(image.sectionHeading)}`)
parts.push(`Image HTML: ${sanitizeImageHtml(image.outerHTML)}`)
if (image.inLink) parts.push(`The image is inside a link with href="${image.inLink.href}".`)
if (image.inLink) parts.push(`The image is inside a link with href="${redactUrl(image.inLink.href)}".`)
if (image.inButton) parts.push('The image is inside a button (or role="button" element).')
Comment on lines 47 to +51
const parts: string[] = [`Page URL: ${pageUrl}`]
if (image.pageTitle) parts.push(`Page title: ${JSON.stringify(image.pageTitle)}`)
if (image.sectionHeading) parts.push(`Nearest heading above the image: ${JSON.stringify(image.sectionHeading)}`)
parts.push(`Image HTML: ${sanitizeImageHtml(image.outerHTML)}`)
if (image.inLink) parts.push(`The image is inside a link with href="${image.inLink.href}".`)
if (image.inLink) parts.push(`The image is inside a link with href="${redactUrl(image.inLink.href)}".`)
@kzhou314 kzhou314 force-pushed the implement-copilot-quality-alt branch from 9b5b167 to 5ae4bc1 Compare June 24, 2026 22:11
taarikashenafi and others added 4 commits June 24, 2026 17:39
This function helps in sanitizing URLs by removing sensitive query strings and fragments.
Updated error logging to redact URLs for improved privacy.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants