Skip to content

fix: warn on invalid auto-approve-risk header; make config required in write tools#207

Open
sunilgattupalle wants to merge 1 commit into
mainfrom
fix/auto-approve-risk-review-fixes
Open

fix: warn on invalid auto-approve-risk header; make config required in write tools#207
sunilgattupalle wants to merge 1 commit into
mainfrom
fix/auto-approve-risk-review-fixes

Conversation

@sunilgattupalle

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to #204 — two review nits that were in a commit on the PR branch but didn't make it into the merge.

  • session-headers.ts: log a warn when X-Harness-Auto-Approve-Risk contains an unrecognized value (e.g. "read", typos) instead of silently falling back to the deployment default. Prevents confusing silent behavior where a misconfigured client gets a different security posture than intended.
  • Write tool registrars (harness_create, harness_update, harness_delete, harness_execute): change config?: Configconfig: Config. The parameter is always passed by registerAllTools — making it optional was a type lie that would let future callers omit it and silently use the process-global auto-approve default instead of the session value.

Test plan

  • pnpm typecheck passes
  • pnpm test passes
  • Sending an invalid X-Harness-Auto-Approve-Risk: read header in an HTTP session logs a warning and uses the deployment default

🤖 Generated with Claude Code

…d in write tool registrars

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

AI-Session-Id: e99a0003-1e04-4235-896a-f22400d45734
AI-Tool: claude-code
AI-Model: unknown
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

2 participants