Skip to content

fix(sdk): redact sensitive info from ZipDeploy logs#3448

Open
jviau wants to merge 2 commits into
mainfrom
u/jviau/sdk-redact-zipdeploy-log-urls
Open

fix(sdk): redact sensitive info from ZipDeploy logs#3448
jviau wants to merge 2 commits into
mainfrom
u/jviau/sdk-redact-zipdeploy-log-urls

Conversation

@jviau

@jviau jviau commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Issue describing the changes in this PR

resolves #3398

Follow-up to the review question on #3387 (discussion): "Could this potentially log sensitive information? Do we need to sanitize this URL or ensure we're not logging query strings?"

The concern is valid. Publish/deployment URLs can carry secrets:

  • Credentials embedded as user info (https://user:password@host).
  • Tokens in the query string (e.g. a SAS ?sig=...), notably for blob-container / pre-signed scenarios.

These were being written verbatim to build logs (frequently captured by CI systems and shared). I also confirmed that new Uri(baseUri, relativePath) preserves the base URI's user info, so the resolved deployment URI logged on success/failure would leak embedded credentials even though the query is replaced.

Fix: Added UriLogExtensions.ToLogSafeString() (Uri and string overloads) that retains only scheme, host, optional port, and path — stripping user info, query, and fragment (best-effort for values that don't parse as an absolute URI, and for relative URIs). Applied it to every URL log site:

  • ZipDeploy.cs: Deploy_BeginPublish, Deploy_CompletedSuccess, Deploy_CompletedFailure, Deploy_Failed, Deploy_InvalidPublishUrl, Deploy_InsecurePublishUrl.
  • DeploymentClient.cs: Deploy_AsyncDeployment, Deploy_PollFailure (server-provided Location header).

Pull request checklist

  • My changes do not require documentation changes
    • Otherwise: Documentation issue linked to PR
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • I have added all required tests (Unit tests, E2E tests)

Additional information

  • New UriLogExtensionsTests unit tests cover user-info/query/fragment stripping, non-default vs default ports, http scheme, relative URIs, null/empty, and best-effort redaction of non-absolute strings.
  • Added a regression test in ZipDeployTaskTests proving credentials and query in PublishUrl are redacted from the logged deployment URL.
  • Updated two existing ZipDeployTaskTests expectations (the logged URL no longer includes the ?isAsync=true query).
  • Test secret placeholders use PLACEHOLDER to avoid secret scanning.
  • Builds clean on netstandard2.0 and net472; full Azure.Functions.Sdk.Tests suite passes on net472 and net10.0.

Publish and deployment URLs can carry secrets - credentials embedded as
user info (https://user:pass@host) or tokens in the query string (e.g. a
SAS ?sig=...). These were written verbatim to build logs, which are often
captured by CI systems and shared.

Add UriLogExtensions.ToLogSafeString() (Uri and string overloads) that
retains only scheme, host, optional port, and path, stripping user info,
query, and fragment. Apply it to every URL log site in ZipDeploy and
DeploymentClient (begin/success/failure/invalid/insecure publish messages
and the async deployment location/poll-failure messages).
Copilot AI review requested due to automatic review settings July 1, 2026 23:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a security/logging concern in the Azure Functions .NET SDK ZipDeploy publishing flow by ensuring deployment URLs written to build logs are redacted to avoid leaking credentials (user info) and secrets in query strings/fragments.

Changes:

  • Introduces UriLogExtensions.ToLogSafeString() to strip user info, query, and fragment from URLs before logging.
  • Applies redaction to ZipDeploy and async-deployment polling log sites (including server-provided Location header logging).
  • Adds unit/regression tests and updates existing test expectations; adds a release note entry.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/Azure.Functions.Sdk.Tests/UriLogExtensionsTests.cs New unit tests validating URL redaction behavior for Uri and string overloads.
test/Azure.Functions.Sdk.Tests/Tasks/Publish/ZipDeployTaskTests.cs Updates log expectations to match redacted URLs; adds regression coverage for credential/query redaction.
src/Azure.Functions.Sdk/ZipDeploy/DeploymentClient.cs Redacts Location header values before logging async deployment/polling messages.
src/Azure.Functions.Sdk/UriLogExtensions.cs Adds new log-safe URL redaction extensions used by ZipDeploy-related logging.
src/Azure.Functions.Sdk/Tasks/Publish/ZipDeploy.cs Uses ToLogSafeString() when logging publish/deployment URLs (begin/success/failure/validation).
src/Azure.Functions.Sdk/release_notes.md Adds a release note entry for redacting sensitive info from ZipDeploy logs.

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

Comment thread src/Azure.Functions.Sdk/UriLogExtensions.cs Outdated
RedactRawUrl treated any '@' as a user-info delimiter, so values without a '//' authority (e.g. mailto:user@example.com, relative path api/@me) and '@' characters in the path were incorrectly rewritten. Only strip user-info when a '//' authority marker is present and the '@' falls within the authority region (before the first path '/').
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.

[SDK] Evaluate ZipDeploy logs

3 participants