Validate WhatsApp Graph API config - #635
Conversation
Greptile SummaryThis PR hardens the WhatsApp Graph API target by adding structured validation of
Confidence Score: 4/5Safe to merge; both changed files are self-contained validation logic with no external API surface changes. The two findings are non-blocking quality items. The validation additions are correct and well-scoped. One finding is that the GRAPH_API_VERSION_RE regex is more permissive than the error message describes (accepts v25, error says v25.0), which could confuse operators providing a bare major version. The other is a missing test for the newly-guarded blank verifyTokenKey path. Neither affects runtime correctness of the happy path or the validated error paths covered by tests. packages/targets/chat-whatsapp/src/index.ts — the version regex/error-message mismatch; packages/targets/chat-whatsapp/src/index.test.ts — missing blank-verifyTokenKey test case Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[build / ship called] --> B[templateManifest]
B --> C[validateBaseConfig]
C --> D{phoneNumberId URL-safe?}
D -- No --> E[throw: must be a URL-safe Graph API id]
D -- Yes --> F{wabaId URL-safe?}
F -- No --> E
F -- Yes --> G{graphApiVersion matches vN.N?}
G -- No --> H[throw: must look like v25.0]
G -- Yes --> I{graphApiBaseUrl valid HTTPS URL?}
I -- No --> J[throw: must use HTTPS]
I -- Yes --> K{webhookUrl valid HTTPS URL?}
K -- No --> L[throw: webhookUrl must use HTTPS]
K -- Yes --> M[manifest built]
M --> N{dryRun?}
N -- Yes --> O[return dry-run result]
N -- No --> P{tokenKey blank?}
P -- Yes --> Q[throw: chat-whatsapp requires tokenKey]
P -- No --> R[requireSecret from vault]
R --> S[callGraph - submit templates]
S --> T{subscribeApp?}
T -- Yes --> U{verifyTokenKey set?}
U -- Yes --> V{verifyTokenKey blank?}
V -- Yes --> W[throw: chat-whatsapp requires verifyTokenKey]
V -- No --> X[requireSecret for verifyTokenKey]
X --> Y[callGraph - subscribe WABA webhooks]
U -- No --> Y
T -- No --> Z[return ship result with manifest.phoneNumberId / wabaId]
Y --> Z
Reviews (1): Last reviewed commit: "Validate WhatsApp Graph API config" | Re-trigger Greptile |
| const TEMPLATE_NAME_RE = /^[a-z0-9_]+$/; | ||
| const LANGUAGE_RE = /^[a-z]{2,3}(_[A-Z]{2})?$/; | ||
| const GRAPH_ID_RE = /^[A-Za-z0-9_-]+$/; | ||
| const GRAPH_API_VERSION_RE = /^v\d+(?:\.\d+)?$/; |
There was a problem hiding this comment.
The regex accepts a version without a minor segment (e.g.
v25), but the error message tells the user it "must look like v25.0". If the intent is to require the MAJOR.MINOR form that Meta Graph API versions use, the ? on the minor group should be dropped. If bare integers like v25 are intentionally allowed, the error message should reflect that.
| const GRAPH_API_VERSION_RE = /^v\d+(?:\.\d+)?$/; | |
| const GRAPH_API_VERSION_RE = /^v\d+\.\d+$/; |
| it('rejects blank secret key names before Graph API calls', async () => { | ||
| await expect(adapter.ship(fakeShipContext({ | ||
| dryRun: false, | ||
| secret: makeVault({ WHATSAPP_BUSINESS_TOKEN: 'mock-token' }), | ||
| }) as any, { | ||
| ...baseConfig, | ||
| tokenKey: ' ', | ||
| })).rejects.toThrow('chat-whatsapp requires tokenKey'); | ||
| }); |
There was a problem hiding this comment.
Missing test for blank
verifyTokenKey
This PR changed the verifyTokenKey path to wrap it with requireText before passing to requireSecret, but no test exercises a blank verifyTokenKey value. Since ' ' is truthy, the if (config.verifyTokenKey) guard lets it through and requireText is the only safeguard — a regression there would be silent. A test mirroring the tokenKey blank case (with a non-dry-run context and verifyTokenKey: ' ') would pin this behaviour.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
11 similar comments
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
|
🤖 Auto-rebase: The branch was rebased successfully locally but could not be pushed to the fork. Please enable 'Allow edits from maintainers' in the PR settings, or rebase manually: |
Fixes #634.
Changes:
Validation: