Skip to content

Release version 0.1.3.2#501

Merged
randhirinsta merged 5 commits into
mainfrom
develop
May 13, 2026
Merged

Release version 0.1.3.2#501
randhirinsta merged 5 commits into
mainfrom
develop

Conversation

@randhirinsta
Copy link
Copy Markdown
Collaborator

  • Security: Migration files on the destination site now use unguessable hashed filenames for stronger privacy.
  • Improved: Each push migration automatically cleans up leftover artifact files at start for a cleaner destination site.

dev5 and others added 5 commits May 11, 2026 07:30
…nloadable by URL

Every push migration writes db.sql, iwp-db-received.sql, iwp-push-log.txt
to ABSPATH and db-<timestamp>.sql to wp-content. All four are static names
in the docroot, so anyone who knows the destination domain can GET them
directly. The encrypted credentials sidecar migrate-push-db-<N>.txt had
only 5 hex chars of filename entropy.

- Suffix four files with sha256(migrate_key)[0..16] (64 bits of entropy):
  - ABSPATH/db.sql                 -> db-<key_hash>.sql
  - ABSPATH/iwp-db-received.sql    -> iwp-db-received-<key_hash>.sql
  - ABSPATH/iwp-push-log.txt       -> iwp-push-log-<key_hash>.txt
  - WP_CONTENT_DIR/db-<ts>.sql     -> db-<key_hash>-<ts>.sql
  Wire format header X-File-Relative-Path: db.sql is unchanged — the
  destination remaps internally, so older source plugins interop with
  no coordinated upgrade.

- migrate-push-db-<5chars>.txt naming is intentionally NOT changed:
  source and destination plugins compute it identically, and changing it
  would break mixed-version pushes. Content is AES-256-CBC encrypted with
  the full 160-bit migrate_key so filename entropy is not load-bearing.

- New helpers in includes/functions-pull-push.php (procedural so iwp-dest
  can call them without bootstrapping WordPress):
  - iwp_get_migration_key_hash()
  - iwp_get_migration_file_paths()
  - iwp_delete_migration_files()
  - iwp_cleanup_stale_migration_files() — self-healing orphan sweep with
    a 6h mtime threshold and current-migration exclusion, so concurrent
    migrations can never race.

- iwp-dest invokes the orphan sweep inside the $_POST['check'] block (the
  first call of every push), and passes the hash to iwp_backup_wp_database().

- InstaWP_Tools::clean_iwp_files_dir() extended with broad globs
  (iwp-*.sql, iwp-*.txt, db*.sql in both ABSPATH and wp-content) so it
  catches BOTH legacy plain names AND new hashed names — in-flight
  migrations during the plugin upgrade are still cleaned up cleanly.

- No register_shutdown_function / no chmod 0600. Shutdown handlers
  would race with successful mid-flight chunk writes; tightening perms
  to 0600 can prevent the WP-context cleanup from deleting files
  written by iwp-dest on suEXEC/suPHP hosts. Cleanup is layered:
  primary API call on success, orphan sweep on the next push, plus the
  unguessable filename in between.

- Tooltip for instawp_keep_db_sql_after_migration no longer references
  the literal db.sql filename since the on-disk name is now hashed.

Verified with helper unit tests (deterministic hash, distinct keys,
idempotent delete, sweep skips fresh files, sweep keeps within-threshold,
sweep deletes stale, exclude_hash protects current migration, broad globs
match both legacy and hashed names).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…anup

The encrypted credentials sidecar migrate-push-db-<5chars>.txt and the
legacy catch-all instawp-sql-* were only being deleted by the API-driven
clean_iwp_files_dir(). On a failed migration that never reached that
endpoint, the sidecar lingered indefinitely.

Adds both patterns to iwp_cleanup_stale_migration_files so the next push
migration's startup sweep reclaims them. Still bounded by the same 6h
mtime threshold so a concurrent in-flight migration cannot race. The
sidecar's naming scheme uses a 5-char raw-key prefix rather than the
sha256 hash, so it can't participate in the exclude_hash protection —
mtime is the sole guard, which is fine in practice because the sidecar
is uploaded by the source at migration start (mtime ~= request start).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bump Stable tag to 0.1.3.2. Customer-facing changelog summarizes the
push-migration security and cleanup work shipped in this version.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Matches the Stable tag and changelog entry in readme.txt.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fix: hash push-migration artifacts on destination so they are not downloadable by URL
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 13, 2026

Claude finished @randhirinsta's task —— View job


Pull Request Review Complete

Context loaded:

  • CLAUDE.md (project guidelines)
  • .claude/randhirinsta.claude (developer preferences)

Review Todo List:

  • Load developer context and project guidelines
  • Analyze changed files for architectural principles compliance
  • Review security improvements and hashed filename implementation
  • Check cleanup functionality for artifact files
  • Assess code quality, performance, and best practices
  • Verify backward compatibility
  • Provide comprehensive feedback and recommendations

Overall Assessment: EXCELLENT

This PR implements a critical security hardening feature with excellent engineering practices. The changes demonstrate strong adherence to architectural principles and security best practices.

🔒 Security Improvements

Critical Security Enhancement

  • Hashed Filenames: Migration files now use unguessable hashed filenames instead of predictable ones
    • Old: db.sql, iwp-push-log.txt → New: db-{hash}.sql, iwp-push-log-{hash}.txt
    • Impact: Prevents unauthorized access to migration dumps via URL guessing on staging domains
    • Hash: 64-bit entropy (16 hex chars) from SHA-256 of migration key - cryptographically strong

Backward Compatibility

  • Wire protocol unchanged (X-File-Relative-Path: db.sql) - older source plugins continue working
  • Graceful fallback to legacy naming when no hash provided

🧹 Automatic Cleanup System

Self-Healing Architecture

  • Proactive Cleanup: iwp_cleanup_stale_migration_files() runs at migration start
  • Broad Pattern Matching: Catches both legacy and hashed filenames
  • Race Condition Protection: 6-hour staleness threshold prevents interference with active migrations
  • Comprehensive Coverage: Includes encrypted credential sidecars (migrate-push-db-*.txt)

🏗️ Architectural Excellence

STRONG ADHERENCE TO CORE PRINCIPLES

1. Single Source of Truth (SSOT)

  • iwp_get_migration_file_paths() - centralized filename management
  • All components use same hash derivation logic
  • Eliminates filename inconsistencies

2. DRY (Don't Repeat Yourself)

  • iwp_get_migration_key_hash() - shared hash generation
  • iwp_delete_migration_files() - reusable cleanup logic
  • Consolidated glob patterns in clean_iwp_files_dir()

3. Separation of Concerns (SoC)

  • Hash generation → iwp_get_migration_key_hash()
  • Path management → iwp_get_migration_file_paths()
  • Cleanup logic → iwp_cleanup_stale_migration_files()
  • Each function has single, clear responsibility

4. Proper Abstraction

  • Clean APIs with documented parameters
  • Idempotent operations (safe to call multiple times)
  • Host-specific considerations (suEXEC/suPHP permissions)

📋 Code Quality Assessment

Strengths

  • Excellent Documentation: Clear PHPDoc with security rationale
  • Error Handling: Proper use of @unlink() for permission issues
  • Performance: Efficient glob patterns, bounded cleanup operations
  • Security: Hash timing attack protection via deterministic generation

WordPress Standards Compliance

  • Proper function existence checks (if ( ! function_exists() ))
  • Correct escaping and validation patterns
  • WordPress coding standards followed

🔧 Implementation Details

iwp-dest/index.php Changes

// Lines 28-32: Per-migration filename suffix generation
$migration_key_hash = iwp_get_migration_key_hash( $migrate_key );
$log_file_path = $root_dir_path . DIRECTORY_SEPARATOR . 'iwp-push-log-' . $migration_key_hash . '.txt';
$received_db_path = $root_dir_path . DIRECTORY_SEPARATOR . 'iwp-db-received-' . $migration_key_hash . '.sql';

// Lines 127-134: Secure db.sql handling
$file_save_path = $root_dir_path . DIRECTORY_SEPARATOR . 'db-' . $migration_key_hash . '.sql';

functions-pull-push.php Enhancements

  • Hash Generation: 64-bit entropy from SHA-256
  • Path Management: Centralized via iwp_get_migration_file_paths()
  • Cleanup System: Comprehensive stale file removal

class-instawp-tools.php Improvements

  • Updated clean_iwp_files_dir() with glob patterns
  • Handles both legacy and hashed filenames
  • Respects user's "Keep DB SQL File" setting

🚀 Performance & Reliability

  • Bounded Operations: 6-hour staleness threshold prevents runaway cleanup
  • Efficient Patterns: Single glob calls vs. individual file checks
  • Memory Conscious: Processes files individually, not in bulk arrays
  • Host Compatibility: Considers different PHP/web server configurations

🔍 Minor Observations

Suggestions for Future Enhancement

  1. Monitoring: Consider logging cleanup statistics for operational visibility
  2. Configuration: The 6-hour threshold could be configurable for extreme edge cases
  3. Metrics: Track cleanup effectiveness in migration logs

Code Style Notes

  • Excellent use of strict comparison (===, !==)
  • Proper array casting with (array) glob()
  • Good variable naming conventions

🎯 Recommendation: APPROVE

This is exemplary work that significantly enhances security while maintaining backward compatibility. The implementation demonstrates:

  • Security First: Addresses real vulnerability (URL-guessable migration files)
  • Operational Excellence: Self-healing cleanup system
  • Engineering Quality: Clean abstractions, proper separation of concerns
  • Production Ready: Comprehensive edge case handling

The PR is ready for merge with confidence.


@randhirinsta randhirinsta merged commit b2f55e3 into main May 13, 2026
3 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