Skip to content

security: Tier 1 hardening (wrapper integrity, token redaction, log sanitization)#39

Open
MarketDataApp wants to merge 1 commit into
mainfrom
security/tier1-hardening
Open

security: Tier 1 hardening (wrapper integrity, token redaction, log sanitization)#39
MarketDataApp wants to merge 1 commit into
mainfrom
security/tier1-hardening

Conversation

@MarketDataApp

Copy link
Copy Markdown
Owner

Tier 1 security hardening

Applies the Tier 1 findings from a security review scoped to SECURITY.md (the five in-scope axes: credential handling, TLS, request injection, deserialization safety, supply-chain). Tier 1 = behavior-compatible for consumers, so per policy these are applied directly; a reviewer sign-off is still wanted here.

Changes

1. Gradle build integrity (gradle-wrapper.properties, pull-request.yml, publish.yml)

  • Pinned the distribution checksum via distributionSha256Sum (verified against services.gradle.org for 9.6.1) so a tampered/MITM'd distribution zip is rejected on every ./gradlew run.
  • Added gradle/actions/wrapper-validation to the PR gate and the publish gate so a tampered gradle-wrapper.jar can't execute — importantly, in the signed publish build that holds the GPG key + Central creds.
  • Fixed a misleading comment: setup-gradle does not validate the wrapper jar hash by itself (the old comment claimed it did).

2. Configuration.toString() redacts the API key (Configuration.java)

  • A record's generated toString() prints every component verbatim, so any future in-package LOGGER.info("config=" + config) would have leaked the raw token. Now redacts apiKey via Tokens.redact; non-secret fields stay visible for diagnostics.

3. Sanitize untrusted API-response strings before logging (LogSafe.java + 6 call sites)

  • errmsg and malformed date/timestamp cells are attacker-influenced (untrusted response body). Copied verbatim into ParseError messages that consumers log, embedded CR/LF can forge log lines and ESC can spoof terminals. New LogSafe.sanitize collapses ISO control chars and caps length; applied in ParallelArrays, the three envelope-error deserializers, and the three MarketDataDates parse paths.

Deliberately excluded

  • SHA-pinning of GitHub Actions — every action here is first-party (actions/*, gradle/actions, codecov, dependabot); the value is low and Dependabot + reputation suffice. Discussed and dropped.

Verification

  • ./gradlew build green: unit tests, Spotless, and JaCoCo coverage verification all pass.
  • New tests: LogSafeTest (control-char/ANSI/truncation) and a Configuration.toString redaction test.

Not in this PR — Tier 2 (need maintainer approval per policy)

Tracked separately (compatibility-affecting): http:// base-URL cleartext-token allowance, symbol path-segment ..// traversal, unbounded response-body size cap, and Redirect.NEVER.

…on, log sanitization)

Addresses the Tier 1 findings from the SECURITY.md-scoped review:

- Build integrity: pin the Gradle distribution checksum (distributionSha256Sum)
  and add gradle/actions/wrapper-validation to the PR and publish workflows so a
  tampered gradle-wrapper.jar cannot execute in CI (fixes the misleading
  "validates the wrapper jar hash" comment, which setup-gradle does not do).
- Configuration.toString() redacts the API key via Tokens.redact instead of the
  record's default verbatim rendering (latent token leak if ever logged).
- Sanitize untrusted API-response strings (errmsg, malformed date/timestamp
  cells) before embedding them in exception/log messages, preventing CR/LF
  log-forging and ANSI-escape terminal spoofing (new LogSafe helper).

SHA-pinning of first-party actions was intentionally excluded (low value for
GitHub/Gradle-owned actions; Dependabot + reputation suffice).

Tests added for LogSafe and the Configuration.toString redaction.
@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.28%. Comparing base (52efbc7) to head (e112c9b).

Additional details and impacted files
@@            Coverage Diff            @@
##               main      #39   +/-   ##
=========================================
  Coverage     98.27%   98.28%           
- Complexity     1043     1049    +6     
=========================================
  Files           123      124    +1     
  Lines          2606     2617   +11     
  Branches        301      304    +3     
=========================================
+ Hits           2561     2572   +11     
  Partials         45       45           
Files with missing lines Coverage Δ
...rc/main/java/com/marketdata/sdk/Configuration.java 100.00% <100.00%> (ø)
src/main/java/com/marketdata/sdk/LogSafe.java 100.00% <100.00%> (ø)
.../main/java/com/marketdata/sdk/MarketDataDates.java 93.33% <100.00%> (+0.15%) ⬆️
...marketdata/sdk/OptionsExpirationsDeserializer.java 94.73% <100.00%> (ø)
.../com/marketdata/sdk/OptionsLookupDeserializer.java 100.00% <100.00%> (ø)
...c/main/java/com/marketdata/sdk/ParallelArrays.java 96.26% <100.00%> (ø)
...java/com/marketdata/sdk/StockNewsDeserializer.java 95.83% <100.00%> (ø)

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 52efbc7...e112c9b. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant