test+docs: follow-ups to #108 — FME transport coverage, redaction toggle test, env-var docs#113
Open
thisrohangupta wants to merge 2 commits intophase1-p1-p7-p8from
Open
test+docs: follow-ups to #108 — FME transport coverage, redaction toggle test, env-var docs#113thisrohangupta wants to merge 2 commits intophase1-p1-p7-p8from
thisrohangupta wants to merge 2 commits intophase1-p1-p7-p8from
Conversation
Closes follow-up gaps identified in #108 review: - FME transport: assert that product='fme' requests omit the Harness-Account header and the accountIdentifier query param so Harness-specific scope metadata never leaks to the Split.io API. Adds a positive control that non-fme requests still inject accountIdentifier as before. - HARNESS_LOG_UNSAFE_BODIES toggle: integration test that the flag actually flows from config through HarnessClient to the debug log call sites — by default sensitive fields are redacted in both request and response bodies; when set to true the raw body is logged. These complement the unit tests in tests/utils/redact.test.ts which exercise the redaction utility in isolation; the new tests close the loop on its wiring inside HarnessClient. Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
Both env vars were introduced in #108 but missed the docs update: - HARNESS_FME_BASE_URL: override for self-managed/staging FME (Split.io) backends; defaults to https://api.split.io and is subject to the same HTTPS guard as HARNESS_BASE_URL. - HARNESS_LOG_UNSAFE_BODIES: debug-only escape hatch to disable sensitive-field redaction in request/response body logs. Defaults to false. Documented as 'local debugging only — never in shared environments' to discourage misuse. Updates .env.example, the env-var table in README.md, the HTTPS Enforcement section (now mentions FME), and adds a troubleshooting row for the FME HTTPS startup error. Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #108. Closes the must-fix gaps from review.
What's here
1. FME transport test coverage (
tests/client/harness-client.test.ts)PR #108 added
product === "fme"branches inharness-client.tsthat suppress both theHarness-Accountheader and theaccountIdentifierquery param (so Harness-specific scope metadata never leaks to the Split.io API). Those branches had no assertions, meaning a regression to either would silently start leaking account scope.Three new tests:
omits accountIdentifier query param for product='fme'— also asserts noaccountID(log-service variant) is set.omits Harness-Account header for product='fme' (Split.io API)still injects accountIdentifier for non-fme product (default Harness)— positive control so we don't accidentally regress the default path.2.
HARNESS_LOG_UNSAFE_BODIESintegration testPR #108's
tests/utils/redact.test.tsexercises the redaction utility in isolation. The newdebug log redaction (HARNESS_LOG_UNSAFE_BODIES toggle)describe block closes the loop on the wiring insideHarnessClient:HARNESS_LOG_UNSAFE_BODIES=truelogs raw body — verifies the escape hatch actually bypasses redaction.Spies on
console.error(logger output goes to stderr viaconsole.error) and filters for theRequest body/Response bodylog entries.3. Documentation (
.env.example,README.md)Both env vars introduced in #108 were missing from docs:
HARNESS_FME_BASE_URL— added to.env.exampleand the env-var table; HTTPS Enforcement section now mentions both URLs; new troubleshooting row for the FME HTTPS startup error.HARNESS_LOG_UNSAFE_BODIES— added to.env.exampleand the env-var table with a "local debugging only — never in shared environments" warning to discourage misuse.Test plan
pnpm typecheck— cleanpnpm test— 1091/1091 (was 1085 in fix: P1/P7/P8 — version unification, FME config wiring, debug log redaction #108; +6 new)Merge order
Merge #108 first, then this. Branch is based on
phase1-p1-p7-p8so it'll auto-target main once #108 lands.