You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stormcrawler.apache.org by "Riddhish1 (via GitHub)" <gi...@apache.org> on 2026/09/13 13:30:23 UTC

[PR] #2102 Redact proxy credentials from worker logs [stormcrawler]

Riddhish1 opened a new pull request, #2151:
URL: https://github.com/apache/stormcrawler/pull/2151

   # Redact proxy credentials from worker logs
   
   ## The problem
   
   `SCProxy.toString()` included the proxy username and password when credentials were configured:
   
   ```text
   http://user:password@proxy.example.com:8080
   ```
   
   `HttpProtocol` passed this value to `LOG.debug()` for every proxied fetch. When DEBUG logging was enabled, proxy credentials were therefore written to worker logs and could be shipped to central log aggregation.
   
   These credentials may belong to paid third-party proxy services and should not be exposed to everyone with access to worker logs.
   
   ## What this PR changes
   
   ### Redact credentials in `SCProxy.toString()`
   
   Authenticated proxies are now rendered as:
   
   ```text
   http://user:***@proxy.example.com:8080
   ```
   
   Unauthenticated proxies continue to render normally:
   
   ```text
   http://proxy.example.com:8080
   ```
   
   Proxy credentials remain available through `getUsername()` and `getPassword()` for the actual authentication flow. Equality and hashing are also unchanged because they operate directly on the proxy fields.
   
   ### Remove proxy stringification from OkHttp logging
   
   The OkHttp protocol now logs only the proxy endpoint:
   
   ```text
   fetching with proxy <url> - proxy.example.com:8080
   ```
   
   This prevents the logging path from depending on the proxy's rendered representation.
   
   ## Compatibility impact
   
   The rendered value of `SCProxy.toString()` is no longer a credential-bearing connection string. This is intentional because `toString()` may be used implicitly by logging and diagnostics.
   
   No production code in this repository requires the full connection string from `toString()`. Proxy construction and authentication continue to use the individual proxy getters.
   
   ## Tests
   
   Updated the existing proxy tests to verify the redacted representation for authenticated proxies while preserving the existing endpoint format for unauthenticated proxies.
   
   The tests also continue to verify that:
   
   - Proxy usernames and passwords are parsed correctly.
   - Proxy authentication fields remain available through their getters.
   - Proxy equality and hash codes still include credentials.
   - Single-proxy manager configuration continues to work.
   - Metadata proxy overrides continue to work.
   - Multi-proxy rotation and lookup behavior are unchanged.
   - OkHttp proxy handling remains thread-safe.
   
   ## Verification
   
   ```text
   mvn -pl core test \
     -Dtest=SCProxyTest,SingleProxyManagerTest,MultiProxyManagerTest,HttpProtocolProxyConcurrencyTest
   ```
   
   Result:
   
   ```text
   Tests run: 42, Failures: 0, Errors: 0, Skipped: 0
   BUILD SUCCESS
   ```
   
   Also verified:
   
   ```text
   git diff --check
   ```
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@stormcrawler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] #2102 Redact proxy credentials from worker logs [stormcrawler]

Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 commented on PR #2151:
URL: https://github.com/apache/stormcrawler/pull/2151#issuecomment-5676772878

   thx @Riddhish1 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@stormcrawler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] #2102 Redact proxy credentials from worker logs [stormcrawler]

Posted by "Riddhish1 (via GitHub)" <gi...@apache.org>.
Riddhish1 commented on PR #2151:
URL: https://github.com/apache/stormcrawler/pull/2151#issuecomment-5676395936

   > LGTM. The String constructor still includes the rejected connection string in its exception; could we omit it there too, since it may contain credentials?
   > 
   > _Findings flagged by Claude_
   
   
   Addressed


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@stormcrawler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] #2102 Redact proxy credentials from worker logs [stormcrawler]

Posted by "rzo1 (via GitHub)" <gi...@apache.org>.
rzo1 merged PR #2151:
URL: https://github.com/apache/stormcrawler/pull/2151


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@stormcrawler.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org