Skip to content

Fix security findings from gem audit#1

Merged
ardavis merged 6 commits into
masterfrom
fizzy-849-security-findings-activeitem-gem
May 22, 2026
Merged

Fix security findings from gem audit#1
ardavis merged 6 commits into
masterfrom
fizzy-849-security-findings-activeitem-gem

Conversation

@ardavis
Copy link
Copy Markdown
Contributor

@ardavis ardavis commented May 21, 2026

Summary

Addresses the code-level security findings from the activeitem gem audit (Fizzy #849).

Critical fixes

  • Cursor deserializationdecode_cursor now validates that decoded JSON is a flat hash with alphanumeric keys and string/numeric values only. Prevents partition traversal via crafted Base64 payloads.
  • Model loader arbitrary require — Removed file-system require from safe_constantize_model. Now validates class name format and uses safe_constantize only.

Medium fixes

  • Mass assignment protectionassign_attributes now filters to declared attribute_names only, ignoring unknown keys that could invoke arbitrary setters.
  • Backoff jitter — Added randomized jitter (0.5 + rand * 0.5 multiplier) to exponential backoff in batch_get_item and batch_write_item retry loops.

Low fixes

  • Object.const_get in composed_of — Replaced with safe_constantize to prevent traversal of the full Ruby constant hierarchy.

Not addressed in this PR (require repo/infra changes)

  • Branch protection on master (GitHub settings)
  • CI pipeline setup (separate card)
  • Gem signing
  • Thread safety of shared DynamoDB client (needs architectural discussion)
  • Uniqueness validation TOCTOU (DynamoDB limitation — documented)

Testing

  • 14 new security specs covering cursor validation, model loader, and assign_attributes filtering
  • All 66 specs pass

Fizzy: https://app.fizzy.do/6098707/cards/849

ardavis added 6 commits May 21, 2026 13:34
- Validate pagination cursors (reject nested objects, arrays, invalid keys)
- Remove arbitrary file require from model_loader (use safe_constantize)
- Filter assign_attributes to declared model attributes only
- Add jitter to exponential backoff in batch operations
- Replace Object.const_get with safe_constantize in composed_of
- Add security specs covering all fixes
- Bump version to 0.0.2
Mass assignment filtering was redundant — attribute_names derives from
setter methods (attr_accessor), so respond_to?(setter) already prevents
setting undeclared attributes. DynamoDB models intentionally remain
flexible about which attributes they accept.

Also fixes spec setup to use DynamoDB local helper from merged master.
- Add parallel_tests gem dependency
- Make table names worker-aware via TEST_ENV_NUMBER env var
- Each parallel worker gets isolated DynamoDB tables (test{N}-dev-*)
- Configure ActiveItem with worker-aware prefix in before(:suite)
- All 124 specs pass in both serial and parallel (16 processes)
- truncate_table: rescue ResourceNotFoundException (prevents cascading
  failures when tables don't exist)
- delete_table: also rescue NetworkingError for clean shutdown
- Add verify_connectivity! check in before(:suite) — aborts early with
  a clear error message if DynamoDB Local isn't running
activeitem_spec.rb sets table_prefix='myapp' and environment='test'
without restoring them. When parallel_tests groups this file with
another spec in the same worker process, subsequent specs get the
wrong config and can't find their tables.

Added after hook to restore worker-aware config.
@ardavis ardavis merged commit 39c2415 into master May 22, 2026
2 checks passed
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.

1 participant