Resolve wallet security review issues

[Sidzeppelin95]

Motivation

• Remove brittle circular coupling between the wallet manager and security engine by extracting shared types and helpers into a single module.
• Ensure recovery locks are actually enforced and represented with timezone-aware UTC datetimes to avoid naive-datetime bugs.
• Replace ad-hoc device-id substring heuristics with explicit connection metadata for more robust trust scoring and to handle all private IP ranges.
• Centralize runtime configuration (URLs and flags) to a single PiOSConfig surface while preserving legacy module-level aliases for backwards compatibility.

Description

• Add pishield/backend/security_shared.py providing Wallet, SecurityUtils.hash_passphrase, utc_now(), and ConnectionMetadata so wallet and security services share types without importing each other.
• Update pishield/backend/wallet_manager.py to use the shared Wallet type, call SecurityUtils.hash_passphrase, enforce recovery_locked_until > utc_now() (short-circuiting authentication when locked), and accept an injected RecoveryResponseHandler instead of importing the engine.
• Update pishield/backend/security_engine.py to introduce PiTrustAnalyzer that consumes ConnectionMetadata (explicit uses_vpn / uses_tor flags and ip_address), use ipaddress.ip_address(...).is_private for private-network heuristics, add SecurityEvent and security_events_db, and implement PiSecurityEngine.trigger_response to set a timezone-aware UTC recovery_locked_until using PiOSConfig.RECOVERY_LOCK_HOURS.
• Centralize configuration in pishield/backend/config.py by adding PiOSConfig and providing clear legacy aliases (PI_APP_URL, PI_APP_NAME, etc.) for Flask compatibility.
• Fix pishield/backend/app.py indentation and formatting so imports and endpoints import/format correctly and the Flask test client runs.

Testing

• Ran byte-compile across the package with python -m compileall -q pishield (succeeded).
• Executed targeted runtime assertions verifying timezone-aware created_at, lock enforcement behavior, revoked-passphrase handling, and PiTrustAnalyzer.calculate_risk_score behavior (all assertions passed).
• Exercised Flask endpoints with the test client (/health, /api/config, and invalid rotate-passphrase input) and checks passed.
• Ran pytest which completed but reported "no tests ran" (no collected tests).
• Ran git diff --check (no whitespace/merge errors reported).

---

Codex Task

Summary by Sourcery

Strengthen the sandbox wallet security architecture with shared security primitives, enforced UTC recovery locks, explicit connection metadata, centralized configuration, and expanded validation.

New Features:

• Add sandbox wallet authentication with recovery-lock enforcement and a dedicated authentication endpoint.
• Add advisory trust scoring based on explicit VPN, Tor, and IP connection metadata.
• Add security-event recording and analyst review workflow for recovery-related signals.

Bug Fixes:

• Prevent authentication with active, revoked, or invalid credentials while a recovery lock is active.
• Normalize wallet timestamps and recovery locks to timezone-aware UTC datetimes.
• Handle private IPv4 and IPv6 network detection consistently and remove device-ID network heuristics.

Enhancements:

• Extract shared wallet, hashing, timestamp, and connection types to remove coupling between wallet and security services.
• Centralize runtime settings in PiOSConfig while retaining legacy configuration aliases.
• Wire wallet recovery through an injectable response handler and improve Flask endpoint validation and formatting.

Documentation:

• Document the sandbox security architecture, authentication behavior, advisory risk scoring, and validation commands.

Tests:

• Add integration and unit coverage for Flask endpoints, wallet locks, credential handling, configuration aliases, trust scoring, and security workflows.

Chores:

• Add pytest configuration and test suite structure.

[sourcery-ai]

Reviewer's Guide

This PR decouples wallet and security services through shared primitives and an injected response interface, enforces timezone-aware recovery locks, replaces device-string trust heuristics with explicit connection metadata, adds security-event review support, centralizes configuration with compatibility aliases, and fixes Flask application formatting.

Sequence diagram for recovery-lock enforcement

sequenceDiagram
    participant Client
    participant PiWalletManager
    participant Wallet
    participant PiSecurityEngine

    Client->>PiWalletManager: authenticate(username, entered_passphrase)
    PiWalletManager->>Wallet: read recovery_locked_until
    alt recovery lock is active
        PiWalletManager-->>Client: false
    else lock expired or absent
        PiWalletManager->>PiWalletManager: SecurityUtils.hash_passphrase(entered_passphrase)
        alt revoked passphrase detected
            PiWalletManager->>PiSecurityEngine: trigger_response(wallet)
            PiSecurityEngine->>Wallet: set recovery_locked_until
            PiWalletManager-->>Client: false
        else active passphrase
            PiWalletManager-->>Client: true
        end
    end

Loading

File-Level Changes

Change	Details	Files
Extract shared wallet, hashing, timestamp, and connection metadata primitives to remove wallet/security-engine coupling.	Added a shared timezone-aware UTC clock and wallet model. Centralized passphrase hashing and introduced explicit VPN, Tor, and IP metadata. Updated wallet and security modules to consume the shared interfaces.	pishield/backend/security_shared.py pishield/backend/wallet_manager.py pishield/backend/security_engine.py
Enforce recovery locks and connect revoked-passphrase detection to an injectable security response handler.	Short-circuited authentication while a wallet lock is active. Triggered recovery responses for revoked passphrases without importing the security engine. Set recovery lock expiry using configurable hours and timezone-aware UTC datetimes.	pishield/backend/wallet_manager.py pishield/backend/security_engine.py
Replace device-ID heuristics with explicit connection risk analysis and add in-memory security review state.	Added bounded trust scoring for VPN, Tor, and all IP ranges recognized as private by the standard library. Added security events and analyst review/status transitions. Added a response coordinator that applies recovery locks.	pishield/backend/security_engine.py pishield/backend/security_shared.py
Centralize runtime settings while preserving existing Flask configuration imports.	Added the PiOSConfig configuration surface for URLs, feature flags, and security thresholds. Mapped legacy module-level constants to the centralized values. Loaded environment configuration consistently.	pishield/backend/config.py
Repair Flask application formatting and endpoint structure so the application imports and test client operate correctly.	Corrected indentation and malformed formatting across routes and application startup. Preserved health, configuration, validation, and wallet-rotation endpoint behavior. Retained validation for malformed optional Pi auth identifiers.	pishield/backend/app.py

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 2 issues

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="pishield/backend/app.py" line_range="19" />
<code_context>
- 
+CORS(app)
+
 security_engine = SecurityEngine()
 FRONTEND_DIR = Path(__file__).resolve().parents[1] / "frontend"

</code_context>
<issue_to_address>
**issue (broader_impact):** The Flask application instantiates `SecurityEngine`, not `PiSecurityEngine`, and never wires a `PiWalletManager` with a recovery response handler. Consequently, revoked-passphrase authentication through the new wallet manager uses the default `None` handler and returns `False` without setting `recovery_locked_until`, so the new recovery-lock enforcement is not active in the application path.

**Triggers:** When authentication is performed through the application’s configured security components.

**Suggested fix:** Instantiate `PiSecurityEngine` and inject it into the wallet manager, or make the application’s existing `SecurityEngine` implement the response-handler contract and connect it to wallet authentication.
</issue_to_address>

### Comment 2
<location path="pishield/backend/wallet_manager.py" line_range="42" />
<code_context>
+
+        # A recovery lock applies to every authentication attempt, including a
+        # revoked passphrase, until its timezone-aware UTC expiry has passed.
+        if wallet.recovery_locked_until and wallet.recovery_locked_until > utc_now():
+            return False
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The comparison raises `TypeError` when a wallet contains a naive `recovery_locked_until`, because `utc_now()` is timezone-aware. The `Wallet` dataclass accepts naive datetimes without validation, so callers constructing or restoring wallets with legacy naive timestamps cannot authenticate at all.

**Triggers:** When a wallet is created or restored with a naive recovery-lock datetime.

**Suggested fix:** Normalize incoming datetimes to UTC or reject naive values at the `Wallet` boundary before comparing them.
</issue_to_address>

Sourcery assessment

Needs a human reviewer. 2 findings to address first, and the change adds wallet authentication and recovery-lock behavior, including passphrase hashing and a 24-hour lock triggered by revoked credentials. A defect could deny legitimate wallet access or create an authentication weakness; the state is currently in memory and a restart or rollback clears it, but any incorrect access decision can affect users while deployed.

Blocking findings: pishield/backend/app.py:19, pishield/backend/wallet_manager.py:42

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook