Skip to content

fix(skills): prevent HAR credential leakage - #87958

Open
plcunha wants to merge 3 commits into
NousResearch:mainfrom
plcunha:fix/har-derived-api-secret-safety
Open

plcunha wants to merge 3 commits into
NousResearch:mainfrom
plcunha:fix/har-derived-api-secret-safety

Conversation

@plcunha

@plcunha plcunha commented Aug 16, 2026

Copy link
Copy Markdown

Summary

Prevent the official har-derived-api-client skill from copying captured credentials into agent/model output and reduce exposure of the raw HAR itself. This preserves the scheme/query/header fixes from closed PR #85053 as its original commit (and therefore preserves Tuomas Hietala's authorship), then extends redaction to sensitive query parameters, nested JSON request/response fields, and URL-encoded forms. Both capturers now make completed HARs owner-only on POSIX; the CDP writer also refuses symlink traversal where the platform supports O_NOFOLLOW.

This intentionally does not absorb the capture lifecycle, postData: null, form-without-text, or base64 work in #85100/#84977. Those are separate correctness fixes with overlapping files.

RED proof

With the new behavioral tests kept but the three scripts restored to current origin/main:

$ scripts/run_tests.sh tests/skills/test_har_derived_api_client_skill.py -q
9 passed, 5 failed

FAILED test_preserves_scheme_parses_url_query_and_redacts_credentials
FAILED test_redacts_query_and_structured_body_credentials
FAILED test_redacts_form_credentials_without_hiding_non_secret_token_fields
FAILED test_cdp_capture_populates_query_string
FAILED test_cdp_capture_writes_owner_only_har

On this branch:

$ scripts/run_tests.sh tests/skills/test_har_derived_api_client_skill.py -q
14 passed, 0 failed

A live local capture against JSONPlaceholder also produced mode=600, derived GET /posts/{id}, and completed browserless derivation successfully.

Code path trace

  1. Symptom: running har_to_client.py can place live query/body/response credentials into agent context; capture scripts create raw HARs readable beyond the current user under a typical 022 umask.
  2. Intermediate: the grouping loop previously printed query values and body samples verbatim, while capture output used Playwright/default open() permissions.
  3. Root cause: secret handling covered only selected request headers and documentation warnings; it did not treat structured fields or the raw HAR's filesystem permissions as part of the same credential boundary.

Widening audit

  • Request headers: affected → common authorization/key/token/secret header values are redacted (from fix(skills): harden HAR-derived API output #85053).
  • URL query parameters: affected → exact credential field names are redacted; benign fields such as token_count remain visible.
  • JSON request bodies: affected → credential keys are recursively redacted.
  • JSON response bodies: affected → nested credential keys are recursively redacted.
  • URL-encoded request bodies: affected → credential values are redacted and the sample is safely re-encoded.
  • Arbitrary unstructured bodies: cannot be classified reliably → still sampled; the skill now states this residual risk explicitly.
  • Local Playwright capture: affected → completed HAR is chmod 0600 and symlink output is rejected.
  • CDP capture: affected → owner-only os.open(..., 0600) writer; existing loose files are tightened and symlinks are rejected where O_NOFOLLOW exists.
  • Windows: POSIX permission bits do not apply; redaction remains active and documentation scopes the permission guarantee accordingly.

Verification

scripts/run_tests.sh tests/skills/test_har_derived_api_client_skill.py -q
# 14 passed

python -m ruff check optional-skills/web-development/har-derived-api-client/scripts/*.py tests/skills/test_har_derived_api_client_skill.py
# All checks passed!

python -m py_compile optional-skills/web-development/har-derived-api-client/scripts/*.py tests/skills/test_har_derived_api_client_skill.py
git diff --check
# passed

Live smoke:

python optional-skills/web-development/har-derived-api-client/scripts/har_capture.py \
  https://jsonplaceholder.typicode.com/posts/1 /tmp/har-permission-live.har --wait 1
stat -c 'mode=%a bytes=%s' /tmp/har-permission-live.har
# mode=600

python optional-skills/web-development/har-derived-api-client/scripts/har_to_client.py \
  /tmp/har-permission-live.har --max-body 100
# 1 distinct endpoint; GET https://jsonplaceholder.typicode.com/posts/{id}

Attribution

The first commit is the exact commit from closed PR #85053 by @pnaaberi / Tuomas Hietala. This branch builds on it instead of reimplementing the same fixes, preserving authorship in Git history.

@alt-glitch alt-glitch added type/security Security vulnerability or hardening tool/browser Browser automation (CDP, Playwright) tool/skills Skills system (list, view, manage) P3 Low — cosmetic, nice to have labels Aug 16, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

Review of "fix(skills): prevent HAR credential leakage". Well-layered secret hygiene for a skill whose whole workflow handles live credentials: capture outputs get owner-only permissions with symlink refusal (O_NOFOLLOW on the CDP path plus a pre-check on the Playwright path), the derivation tool redacts credential headers and structured credential fields while HONESTLY documenting that unstructured bodies can still leak, and the SKILL.md guidance shifts from "copy these headers" to "obtain equivalent values from an approved runtime secret source" without pretending the technique bypasses auth. The contributor entry is included. Suggestions:

  1. optional-skills/.../scripts/har_capture.py:51 (TOCTOU asymmetry) — the plain-capture path checks islink BEFORE launching the browser, then lets Playwright create/write the file itself; the CDP path got the stronger O_NOFOLLOW open — aligning both on an os.open-based private write would close the small check-vs-write window too.

  2. nit — consider emitting one stderr line when redaction REPLACED credential material during derive ("redacted N credential headers/fields"), so users know their HAR contained live secrets even if they skip the SKILL.md fine print.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have tool/browser Browser automation (CDP, Playwright) tool/skills Skills system (list, view, manage) type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants