Skip to content

fix: harden webhook gate, add JSON schema validation#70

Merged
tcconnally merged 1 commit into
mainfrom
feat/fix-review-issues
Jun 15, 2026
Merged

fix: harden webhook gate, add JSON schema validation#70
tcconnally merged 1 commit into
mainfrom
feat/fix-review-issues

Conversation

@tcconnally

Copy link
Copy Markdown
Collaborator

Closes #67, #69.

  • ALLOW_UNSIGNED_WEBHOOKS now gates on ENVIRONMENT=development
  • BaseAgent._validate_and_retry() validates LLM JSON responses against schemas
  • jsonschema added to dependencies

- #67: ALLOW_UNSIGNED_WEBHOOKS now requires ENVIRONMENT=development
- #69: Add _validate_and_retry() to BaseAgent with jsonschema enforcement
- Add jsonschema to pyproject.toml dependencies

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 PR Pilot Review

🔍 Reviewer (Agent 1)

The pull request significantly improves security by hardening the webhook signature check and adds valuable JSON schema validation for LLM responses. However, the implementation of schema validation could be more robust; it currently proceeds with invalid data after a validation failure, which could cause downstream errors. Additionally, the local import of jsonschema is unconventional for a required dependency.

📋 Issues Found

  • [MEDIUM] error_handling — When JSON schema validation fails with a jsonschema.ValidationError, the error is logged, but the function proceeds to return the invalid data. This can lead to unexpected KeyError or other runtime exceptions in downstream code that relies on the data conforming to the schema. (src/agents/base.py:64)
  • [LOW] style — The jsonschema library is imported locally within a try...except ImportError block, but it is defined as a core dependency in pyproject.toml. This pattern is typically used for optional dependencies and can obscure environment setup problems if the package is missing. (src/agents/base.py:61)

🔧 Fixer (Agent 2)

Fixer failed: RetryError[<Future at 0x7f8258095fd0 state=finished raised ValueError>]

🧪 Tester (Agent 3)

No tests generated.

✅ Verifier (Agent 4)

Verification skipped.

👤 Decision: ESCALATE TO HUMAN

This PR requires human review — too complex or risky for autonomous handling.

Pipeline duration: ~190.4s


Review by PR Pilot — AI-native code quality service

@pr-flyer

pr-flyer Bot commented Jun 15, 2026

Copy link
Copy Markdown

PR Pilot

Automated review pipeline could not complete: the escalator agent encountered an error (Escalator failed: Unterminated string starting at: line 14 column 21 (char 2066)). No automated judgment was made — escalating to human review.

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.

ALLOW_UNSIGNED_WEBHOOKS is a footgun — should require explicit confirmation

1 participant