Skip to content

fix: default-config tier from 2026-06-10 audit#254

Merged
pratyush618 merged 4 commits into
masterfrom
fix/audit-default-tier
Jun 13, 2026
Merged

fix: default-config tier from 2026-06-10 audit#254
pratyush618 merged 4 commits into
masterfrom
fix/audit-default-tier

Conversation

@pratyush618

@pratyush618 pratyush618 commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator

Fixes the four findings from the 2026-06-10 codebase audit that affect a default deployment (thread pool, SQLite, dashboard). One focused commit per finding.

#15 — soft-gate reschedules burned retry budget (scheduler)

Rate-limit / circuit-breaker / concurrency-cap / channel-backpressure gates called storage.retry(), which increments retry_count for a job that never executed — exhausting its budget and dead-lettering it on the first real failure. Backpressure fires under ordinary load, so this is default-reachable.

New reschedule(id, next_at) Storage method = retry() minus the retry_count increment, wired across the trait, diesel_common macro, Redis backend, impl_storage!, and delegate!. Poller gate paths route to it; result_handler keeps retry() for genuine post-execution failures. Helper renamed rollback_claim_and_reschedule.

#4 — webhook HMAC secrets leaked via GET /api/settings (dashboard)

Webhook subscriptions (plaintext signing secret) and delivery logs live under webhooks: keys in the same store as /api/settings, which only redacted auth:. Any authenticated viewer could read them. Added webhooks: to _PROTECTED_PREFIXES.

#5 — notes type drift crashed the job detail page (dashboard)

JobResult.to_dict emitted the parsed notes dict, but the API contract is notes: string | null and the client JSON.parse-s it — crashing the Overview tab for any job with notes. to_dict now emits the raw canonical JSON string; the notes property stays a dict for Python callers.

#43 — NaN/Infinity notes produced invalid JSON

Non-finite floats passed validation and json.dumps (default allow_nan=True) emitted bare Infinity/NaN tokens that break the jobs API response. Rejected in _validate_value (math.isfinite) plus allow_nan=False as a safety net.

Verification

  • cargo test --workspace (84 sqlite + new contract test_reschedule), clippy clean, postgres/redis feature builds compile
  • full Python suite: 1070 passed, 4 skipped; ruff + mypy clean
  • new tests: test_reschedule_preserves_retry_count, test_reschedule, test_get_settings_hides_webhook_keys, test_to_dict_emits_notes_as_json_string, test_non_finite_floats_rejected

Summary by CodeRabbit

Release Notes

  • New Features

    • Jobs experiencing temporary failures (rate limiting, circuit breaker rejection, concurrency limits) now reschedule without consuming retry budget.
    • Dashboard settings now protect webhook configurations from generic endpoint access.
  • Improvements

    • Strengthened validation for job notes—rejects non-finite floating-point values (NaN, Infinity).
    • Fixed job result serialization to use canonical JSON format for notes field.

Soft-gate reschedules (rate limit, circuit breaker, concurrency cap, channel backpressure) called storage.retry(), which increments retry_count for a job that never executed — exhausting its retry budget and dead-lettering it on the first real failure. Add a reschedule() Storage method that resets to Pending without incrementing, and route the gate paths to it; retry() stays for genuine post-execution failures.
Webhook subscriptions store plaintext HMAC secrets (and delivery logs store request payloads) under webhooks: keys in the same store as /api/settings, which only redacted auth:. Any viewer could read them. Add webhooks: to the protected prefixes.
NaN/Infinity passed notes validation and json.dumps (allow_nan default) emitted bare Infinity/NaN tokens — invalid JSON that breaks the dashboard jobs API. Reject non-finite floats in _validate_value and pass allow_nan=False.
JobResult.to_dict emitted the parsed notes dict, but the dashboard API contract is notes: string | null and the client JSON.parse-s it — crashing the job detail page. Emit the raw canonical JSON string instead.
@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8bbfacd1-b1bd-41f3-9730-7d4d615a8228

📥 Commits

Reviewing files that changed from the base of the PR and between 4c2e7b7 and 473f0af.

📒 Files selected for processing (12)
  • crates/taskito-core/src/scheduler/poller.rs
  • crates/taskito-core/src/storage/diesel_common/jobs.rs
  • crates/taskito-core/src/storage/mod.rs
  • crates/taskito-core/src/storage/redis_backend/jobs/state.rs
  • crates/taskito-core/src/storage/sqlite/tests.rs
  • crates/taskito-core/src/storage/traits.rs
  • crates/taskito-core/tests/rust/storage_tests.rs
  • py_src/taskito/dashboard/handlers/settings.py
  • py_src/taskito/notes.py
  • py_src/taskito/result.py
  • tests/core/test_notes.py
  • tests/dashboard/test_dashboard_settings.py

📝 Walkthrough

Walkthrough

This PR introduces a new reschedule operation for soft-gate job failures that preserves retry budget, upgrades the scheduler to use reschedule instead of retry for rate limiting and concurrency failures, hardens notes validation against non-finite floats, fixes result serialization to emit canonical notes JSON strings, and protects webhook settings from generic API endpoints.

Changes

Job Rescheduling and Soft-Gate Handling

Layer / File(s) Summary
Storage reschedule abstraction
crates/taskito-core/src/storage/traits.rs, crates/taskito-core/src/storage/diesel_common/jobs.rs, crates/taskito-core/src/storage/redis_backend/jobs/state.rs, crates/taskito-core/src/storage/mod.rs, crates/taskito-core/tests/rust/storage_tests.rs, crates/taskito-core/src/storage/sqlite/tests.rs
Defines the reschedule(&self, id, next_scheduled_at) trait method across all backends (Diesel/SQLite, Redis, forwarding wiring) to requeue a job to Pending at a future timestamp without decrementing retry_count. Integration and unit tests verify the operation preserves retry budget and updates scheduled time.
Scheduler soft-gate rescheduling
crates/taskito-core/src/scheduler/poller.rs
Replaces retry with reschedule for pre-claim soft gate failures (queue rate limiting, circuit breaker rejection, task rate limiting) and post-claim rollbacks (concurrency exceeded, channel full/closed). New rollback_claim_and_reschedule helper clears execution claim then reschedules instead of retrying.

Notes Validation and Serialization

Layer / File(s) Summary
Non-finite float validation and JSON encoding
py_src/taskito/notes.py, tests/core/test_notes.py
Imports math and rejects non-finite floats (NaN, Infinity) in note validation before encoding; json.dumps uses allow_nan=False to prevent invalid JSON tokens. Tests verify rejection of non-finite values both at top level and nested in structures.
Result notes serialization fix
py_src/taskito/result.py, tests/core/test_notes.py
JobResult.to_dict() now emits notes as the raw canonical JSON string stored in the job (not the re-parsed dict), preserving the `string

Settings Access Control

Layer / File(s) Summary
Webhook settings protection
py_src/taskito/dashboard/handlers/settings.py, tests/dashboard/test_dashboard_settings.py
Expands _PROTECTED_PREFIXES to include webhooks: alongside auth:, hiding webhook configuration and secrets from generic settings list/get/set/delete endpoints. New test verifies webhook keys are filtered from /api/settings snapshots and direct fetch returns 404.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • ByteVeda/taskito#217: Both PRs modify the scheduler's dispatch/rollback behavior in crates/taskito-core/src/scheduler/poller.rs, specifically around soft gate handling and retry vs. reschedule decisions for concurrency caps and backpressure scenarios.

Suggested labels

rust, storage, scheduler, python, tests

Poem

🐰 A reschedule hops forth, no retry in sight,
Soft gates let jobs bounce back, their budget kept tight.
Notes validated clean of infinity's curse,
Settings guard webhooks from endpoints perverse.
In queues and in dicts, the truth now rings clear!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 64.52% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title describes a generic audit fix but does not clearly specify which audit findings are addressed or their scope. Consider revising to be more specific, such as: 'fix: reschedule without retry budget, webhook secret redaction, notes serialization, non-finite float validation' to better summarize the four distinct findings addressed.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/audit-default-tier

Comment @coderabbitai help to get the list of available commands and usage tips.

@pratyush618
pratyush618 merged commit ac4abd8 into master Jun 13, 2026
22 checks passed
@pratyush618
pratyush618 deleted the fix/audit-default-tier branch June 13, 2026 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant