fix(security): resolve code scanning hard-coded cryptographic value alerts#737
Conversation
…lerts Replace inline serde_json::json! macro calls with explicit helper functions to construct JSON responses. This eliminates false positives from CodeQL that flagged JSON object keys as potential hard-coded cryptographic values. Changes: - Add json_body() and json_error_body() helpers in gateway/mod.rs - Replace 12 inline json! macro calls in audio/webhook handlers - Extract test secrets to named constants in whatsapp_webhook_security.rs Resolves GitHub Code Scanning alerts #394-#408 (15 critical alerts) Tests: - cargo test --test whatsapp_webhook_security: 8 passed - cargo clippy --test whatsapp_webhook_security: no warnings - cargo fmt: formatting verified
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR refactors the release automation system to use a new Changes
Sequence DiagramsequenceDiagram
participant GHA as GitHub Actions<br/>_publish.yml
participant GHR as GitHub Release<br/>API
participant RRC as resolve-release-<br/>context.mjs
participant Valid as Validation<br/>Logic
participant Out as GITHUB_OUTPUT
GHA->>GHR: Fetch release by RELEASE_ID
GHR-->>GHA: Release metadata<br/>(tag_name, draft, prerelease)
GHA->>RRC: Invoke with RELEASE_TAG,<br/>PRERELEASE, AFFECTED_COMPONENTS
RRC->>Valid: Parse component-scoped tag<br/>(e.g., clients/agent-runtime@1.0.0-beta)
Valid->>Valid: Validate tag_name matches,<br/>reject if draft: true,<br/>enforce prerelease compatibility
Valid->>Valid: Load component graph,<br/>restrict component IDs
Valid->>Valid: Validate affected-components<br/>override matches tag
Valid-->>RRC: Structured context object<br/>(version, release_channel,<br/>npm_dist_tag, affected_components)
RRC-->>GHA: JSON context to stdout
GHA->>Out: Emit release_* and<br/>npm_dist_tag keys
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 16 minutes and 12 seconds.Comment |
Deploying corvus with
|
| Latest commit: |
3b9c168
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6df67c00.corvus-42x.pages.dev |
| Branch Preview URL: | https://fix-code-scanning-hardcoded.corvus-42x.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/agent-runtime/tests/whatsapp_webhook_security.rs`:
- Line 117: Replace the inline byte literal assigned to variable `body` with the
shared test constant `TEST_PAYLOAD`; specifically change the `let body = b"same
payload";` assignment to use `TEST_PAYLOAD` (or
`TEST_PAYLOAD.as_ref()`/`.as_bytes()` if needed for type compatibility) so this
test uses the same payload constant as the other tests.
- Around line 88-89: Replace the inline string literals assigned to
correct_secret and wrong_secret with the established test constants: use
TEST_APP_SECRET for the correct secret and either FIRST_TEST_SECRET or a new
constant for the wrong secret; update the test in whatsapp_webhook_security.rs
to reference these constants (ensure the constants are in scope or imported) so
hard-coded values are removed and the test aligns with the PR's extracted test
values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7030d4f3-4339-4f89-8412-1f7d52fca84b
📒 Files selected for processing (7)
.github/workflows/_publish.yml.release-please-beta-manifest.json.release-please-manifest.jsonclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/tests/whatsapp_webhook_security.rsscripts/release-contract.test.mjsscripts/resolve-release-context.mjs
💤 Files with no reviewable changes (1)
- .release-please-manifest.json
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: pr-checks
- GitHub Check: sonar
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: submit-gradle
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (6)
clients/agent-runtime/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Run
cargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings, andcargo testfor code validation, or document which checks were skipped and why
Files:
clients/agent-runtime/tests/whatsapp_webhook_security.rsclients/agent-runtime/src/gateway/mod.rs
**/*.rs
⚙️ CodeRabbit configuration file
**/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness.
Flag unnecessary clones, unchecked panics in production paths, and weak error context.
Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.
Files:
clients/agent-runtime/tests/whatsapp_webhook_security.rsclients/agent-runtime/src/gateway/mod.rs
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
clients/agent-runtime/tests/whatsapp_webhook_security.rsscripts/resolve-release-context.mjsscripts/release-contract.test.mjsclients/agent-runtime/src/gateway/mod.rs
clients/agent-runtime/src/{security,gateway,tools}/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Treat
src/security/,src/gateway/,src/tools/as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Files:
clients/agent-runtime/src/gateway/mod.rs
clients/agent-runtime/src/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
clients/agent-runtime/src/**/*.rs: Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Files:
clients/agent-runtime/src/gateway/mod.rs
clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Files:
clients/agent-runtime/src/gateway/mod.rs
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs : Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools}/**/*.rs : Treat `src/security/`, `src/gateway/`, `src/tools/` as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/Cargo.toml : Do not add heavy dependencies for minor convenience; justify new crate additions
Applied to files:
.release-please-beta-manifest.json
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/.github/**/*.{yml,yaml} : For workflow/template-only changes, ensure YAML/template syntax validity
Applied to files:
.github/workflows/_publish.yml
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs : Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Applied to files:
clients/agent-runtime/tests/whatsapp_webhook_security.rsclients/agent-runtime/src/gateway/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why
Applied to files:
clients/agent-runtime/tests/whatsapp_webhook_security.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools}/**/*.rs : Treat `src/security/`, `src/gateway/`, `src/tools/` as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Applied to files:
clients/agent-runtime/tests/whatsapp_webhook_security.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/**/*.rs : Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Applied to files:
clients/agent-runtime/tests/whatsapp_webhook_security.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths
Applied to files:
clients/agent-runtime/tests/whatsapp_webhook_security.rsclients/agent-runtime/src/gateway/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/channels/**/*.rs : Implement `Channel` trait in `src/channels/` with consistent `send`, `listen`, and `health_check` semantics and cover auth/allowlist/health behavior with tests
Applied to files:
clients/agent-runtime/src/gateway/mod.rs
🔇 Additional comments (17)
clients/agent-runtime/src/gateway/mod.rs (4)
1658-1671: Helper extraction is clean and behavior-preserving.
json_bodyandjson_error_bodyreduce repeated literal JSON construction while keeping payload semantics explicit.
2912-2913: Audio error responses are now consistent and centralized.Using
json_error_body(...)here keeps response shape uniform across parse/read/missing-field/timeout failures.Also applies to: 2938-2939, 2953-2956, 3020-3023
3051-3060: SSE fallback anddonepayload construction look correct.The explicit
Value::Objectfallback and helper-basedmessage_idpayload maintain deterministic JSON structure without changing contract.Also applies to: 3070-3070
3122-3129: Legacy webhook payload refactor preserves response contract.Switching these success/error bodies to shared helpers improves maintainability with no observable behavior drift.
Also applies to: 3144-3144
clients/agent-runtime/tests/whatsapp_webhook_security.rs (1)
12-15: LGTM: Constants reduce duplication and address CodeQL alerts.The refactoring centralizes test secrets and payloads into well-named constants, achieving the stated goal of resolving CodeQL false-positive "hard-coded cryptographic value" alerts while improving test maintainability.
scripts/resolve-release-context.mjs (4)
1-12: LGTM!Module imports and
parseBooleanfunction are well-structured with strict validation. The function correctly rejects any value other than the literal strings"true"or"false".
14-33: LGTM!
parseAffectedComponentsOverrideproperly validates JSON structure, ensures the payload is an array, and validates all component IDs against the known component graph. The error wrapping with{ cause: error }preserves the original parse error for debugging.
35-57: LGTM!
parseComponentReleaseTagcorrectly escapes regex special characters in component IDs before pattern matching, preventing regex injection. The semver + optional beta suffix pattern is sound.
59-107: LGTM!
resolveReleaseContextenforces correct channel/prerelease compatibility and ensuresAFFECTED_COMPONENTSexactly matches the tag component. The direct-execution block correctly uses ES module idiom for CLI invocation..release-please-beta-manifest.json (1)
2-4: LGTM!Manifest keys now use package paths (
clients/agent-runtime, etc.) matchingrelease-please-beta-config.jsonstructure. This aligns with the contract test assertions at lines 477-486 inrelease-contract.test.mjs..github/workflows/_publish.yml (3)
159-160: LGTM!Environment variables correctly pass workflow inputs to the resolver script. Default
'[]'forAFFECTED_COMPONENTSis appropriate.
170-201: LGTM!Robust validation before invoking the resolver:
- Verifies
tag_namefrom API matchesRELEASE_TAGinput (prevents ID/tag mismatch)- Rejects draft releases explicitly
- Validates prerelease state consistency between API and input
The Python extraction blocks correctly handle the JSON parsing via environment variable.
202-231: LGTM!The resolver output is correctly captured to a temp file and then parsed to emit
GITHUB_OUTPUTkeys. The 7 keys written match the job's declared outputs.release_componentandaffected_componentsare correctly used only for the step summary since they're not needed by downstream jobs.scripts/release-contract.test.mjs (4)
477-489: LGTM!Updated assertions correctly validate that manifest package paths align with release-please config package paths, not component IDs. The
gradle-kmpexclusion check is also updated to use the path-based keys.
553-569: LGTM!Test harness correctly sets up environment variables matching the resolver's expectations. Hardcoded
RELEASE_IDand defaultPRERELEASE: "false"are appropriate for contract testing.
571-641: LGTM!Comprehensive test coverage for the release context resolver:
- Stable/beta tag resolution with correct field assertions
- Rejection of unsupported tag formats (global, validate-only)
- Prerelease/stable mismatch enforcement
- Affected components override validation (exact match required, multi-component rejected)
990-1002: LGTM!Workflow pattern assertions updated to:
- Require new resolver script reference
- Require new output file pattern
- Forbid old inline validation patterns (
Invalid ${channel},removeprefix,^v[0-9])This ensures the workflow migration to the centralized resolver is enforced.
…tests Replace remaining hard-coded string literals with test constants: - Line 117: Use TEST_PAYLOAD instead of b"same payload" - Lines 88-89: Use TEST_APP_SECRET and FIRST_TEST_SECRET instead of inline "correct_secret" and "wrong_secret" literals This completes the extraction of all hard-coded test values to named constants, fully resolving the code scanning alerts in the test file.
|
1 participant
Related Issues
Resolves GitHub Code Scanning alerts #394-#408 (15 critical alerts)
Summary
This PR fixes 15 critical Code Scanning alerts flagged by CodeQL as "hard-coded cryptographic value" false positives. The alerts were triggered by inline
serde_json::json!macro calls where JSON object keys were mistakenly identified as potential cryptographic material.Changes made:
Gateway module (
clients/agent-runtime/src/gateway/mod.rs):json_body()helper to constructserde_json::Value::Objectexplicitlyjson_error_body()helper for consistent error responsesserde_json::json!calls in audio/webhook handlers with the new helpersWhatsApp webhook tests (
clients/agent-runtime/tests/whatsapp_webhook_security.rs):TEST_APP_SECRETTEST_PAYLOADFIRST_TEST_SECRETSECOND_TEST_SECRETWhy this approach:
"error","status","message_id"are not cryptographic materialTested Information
Verification performed:
Focus areas for reviewers:
json_body()andjson_error_body()produce identical JSON output to the originalserde_json::json!callsDocumentation Impact
Breaking Changes
None. This is a refactoring that preserves all existing behavior.
Checklist