# Security Policy This document describes the security controls actually implemented in this codebase, distinguishes them from documented-but-not-yet-built controls, and gives a maintainer or Claude Code session enough context to extend them correctly. It is written against the current code, not aspirationally — every claim below points at the mechanism that backs it. This is a multi-school / multi-tenant platform. Nothing below should be read as specific to one school; a school is a configured tenant. ## Reporting a vulnerability There is one maintainer on this project (see git history / repository owner). Report a suspected vulnerability directly to them rather than as a public GitHub issue, given this is a live system handling real student and financial data. ## Implemented controls ### SQL injection All application database access goes through parameterized query helpers (`sql()`, `sql_fetch()`, `sql_fetch_all()` in `erp/modules/base.php`, backed by `mysqli_prepare()` + `mysqli_stmt_bind_param()`). Raw string-concatenated SQL is disallowed by convention across `erp/modules/` and `erp/schools//`. `erp/url_shortner/` is a standalone tool with its own database (`shpsramp_url`) and historically used raw `mysqli_query()` with escaped string interpolation; it was migrated to parameterized `mysqli_prepare()` calls directly (not the shared `sql()` helpers, which are wired to a different connection global). ### CSRF `erp/modules/csrf.php` provides `csrf_token()` / `csrf_field()` / `csrf_valid()` / a fail-closed `csrf_require()`-style guard — one token per PHP session (not per-form), verified with `hash_equals()`. Applied to state-changing POST forms across the app; a state-changing form added without CSRF protection is a regression, not a stylistic choice. Every page now uses these helpers. The last hand-rolled implementation (`admin/timetable/auto_substitution.php`, which minted its own token and compared it inline) was consolidated, so `$_SESSION['csrf_token']` is written and read in exactly one file. ### Transaction integrity Money-handling code opens transactions through `mysqli_begin_transaction()` only; the earlier `sql("START TRANSACTION")` form is gone, which matters because that form routed through `mysqli_prepare()` and through `maintenance_write_blocked()`, whose allowlist is SELECT|SHOW|DESCRIBE|EXPLAIN -- so a transaction could be silently refused and the writes that followed would run unprotected with a ROLLBACK that did nothing. Every call site checks the return value and aborts before writing anything if the transaction cannot be opened. The three that did not -- school fee entry, transport fee entry and navigation-permission saving -- were the remaining cases. ### App portal sessions The four app portals recognise a returning visitor in one of two ways, selected by `security.app_session_mode`: - `legacy` (the current default) -- four cookies, where `user_verify` is `sha256(staff_id|adm . login_phone . login_password)`. **This value is the credential.** Its only secret input is a 4-digit PIN, so a captured cookie yields the PIN in 10,000 hashes, and a known staff_id or adm plus a known phone number can be brute-forced against the server, which applies no throttling to cookie verification. It cannot be revoked; it expires only at the end of the academic session. - `dual` -- every login also issues an opaque session and sets the `app_session` cookie, while the legacy cookie is still honoured. Nobody is signed out. Intended to run for at least a week so regular users collect a session. - `opaque` -- the legacy cookie stops being honoured. Anyone without a session signs in once more. An opaque session is `bin2hex(random_bytes(32))`; only its SHA-256 is stored, in `app_session`, so a database read yields nothing replayable. Sessions carry issue/last-seen/expiry timestamps, IP and device, and can be revoked individually, per actor, or per phone number. Expiry slides 30 days from last use, refreshed at most once a day. Switching profiles re-derives the target profile server-side from the current phone number rather than trusting the posted identifier, so a switch can only reach a profile that shares the signed-in phone number. ### Location context When `security.gps_location_required` is `active`, `location_gate_enforce()` returns HTTP 428 until the request carries a usable position fix. The fix is **server-issued**. `erp/assets/location-gate.js` and `activity-location.js` POST the browser Geolocation reading to `erp/schools//location_fix.php`, which requires a valid CSRF token, range-validates latitude, longitude and accuracy, and stores the values with a capture timestamp in the PHP session. `activity_location()` reads only that session entry and rejects a fix older than `ACTIVITY_LOCATION_MAX_AGE_SECONDS` (1800). Until 2026-09, the fix travelled in an unsigned `erp_devloc` cookie, so any coordinate in the audit trail was user-supplied data rather than evidence: setting the cookie by hand satisfied the gate. That cookie is no longer read anywhere. `activity_location()` also reports why a fix is absent -- `captured`, `expired`, `unavailable` (gate on, nothing usable) or `not_requested` (gate off) -- written to `activity_log.al_location_status`. There is no IP geolocation fallback anywhere, deliberately: an IP-derived position is not evidence of where a person was. ### Fail-closed protective checks Controls that cannot complete now deny rather than permit: - `maintenance_write_blocked()` returns `true` when the maintenance-state lookup throws, so a failure of the check blocks writes instead of allowing them. `$GLOBALS['_maintenance_bypass']` is evaluated before the check and remains the break-glass. Consequence worth knowing: if `system_maintenance_state` becomes unreadable while the rest of the database is healthy, the ERP goes read-only until it is restored or the bypass is set. - The payment router's per-IP throttle counter treats a failed count query as fully throttled rather than as zero, so a database problem no longer lifts the rate limit. - Navigation authorization — see that section above. ### Transactions All nine transaction sites use `mysqli_begin_transaction()` / `mysqli_commit()` / `mysqli_rollback()` and check that the transaction opened before writing. Two earlier idioms are gone: `sql("START TRANSACTION")`, which routed a transaction-control statement through `mysqli_prepare()` and through `maintenance_write_blocked()` (whose allowlist is `SELECT|SHOW|DESCRIBE|EXPLAIN`, so it was classified as a write and refused during maintenance mode), and `mysqli_query($con, "START TRANSACTION")`. ### Login throttling `erp/modules/login_throttle.php` locks an account (keyed by `ld.login_phone`) for 600 seconds after 5 failed attempts, tracked via `ld_failed_attempts` / `ld_locked_until`. This exists specifically because 4-digit numeric PINs are an accepted product decision that would otherwise make every account brute-forceable in well under a minute — throttling is the mitigation, not the PIN length itself (that is out of scope for this module to change). ### OTP-gated actions Some high-risk admin actions (e.g. the letterhead module) are gated behind a WhatsApp OTP check (`erp/schools//verification/otp_core.php`). **The gate must be enforced server-side on every page load/refresh**, not once, and never solely via a client-supplied parameter — a prior audit found and fixed exactly this class of bug. Treat it as a pattern to check for whenever touching an OTP-gated flow, not a closed issue. ### Admin IP gating `erp/modules/admin_ip_gate.php` restricts the Main Admin Portal (`admin/index.php`) to a database-backed IP whitelist (`admin_ip_whitelist`, managed from `app/admin/whitelisted_ips.php`), with a hardcoded break-glass fallback list consulted **only** if the whitelist table/query itself fails. It deliberately does not trust `CF-Connecting-IP`/`X-Forwarded-For` unconditionally the way the app's general-purpose `UserInfo::get_ip()` helper does (that helper is fine for cosmetic "who did this" logging elsewhere, not as a security gate). ### Webhook signature verification Payment/payout webhooks verify an HMAC-SHA256 signature via `hash_equals()` before trusting the payload: - Razorpay (`payment_gateway/razorpay/x_webhook_order_paid.php`): `X-Razorpay-Signature` header, verified against the raw payload body. - Cashfree (`automated/webhook/cashfree.php`): `x-webhook-signature` + `x-webhook-timestamp` headers, verified against `timestamp + raw_body`. A missing signature check on a payout webhook was a prior confirmed bug — a webhook handler added without this check is not done yet. - Zwitch (`automated/webhook/zwitch.php`): same pattern. Cron endpoints reachable by URL (`erp/schools//automated/cron/*.php`) require a shared secret (`config('automated', 'cron_secret')`), checked via `hash_equals()`, **except when running as CLI** (`php_sapi_name() === 'cli'`) — the actual production trigger, per `docs/cron.md`. Every dispatched cron file re-checks this itself (not only the master dispatcher), so it's also safe if hit directly by its own URL, not just via the dispatcher. ### Secrets management - The one real config file (`erp/config.local.php`, gitignored) holds only DB credentials, `BASE_DOMAIN`, `BASE_PATH`, and `CONFIG_ENCRYPTION_KEY`. `erp/config.php` loads it and is the single config entry point; it is tracked in Git and must stay secret-free. - Everything else (API keys, gateway credentials, cron secret, branding) lives in the `app_config` table, read via `config($group, $key)`. Rows can be flagged `ac_is_secret` (masked in `app_config_audit_log` history and anywhere a value is displayed) and/or `ac_is_encrypted` (AES-256-GCM via `config_encrypt()`/`config_decrypt()`, keyed by `CONFIG_ENCRYPTION_KEY`). - **A new secret must be added to `app_config`, never hardcoded in a PHP file.** ### File upload validation `uploadFile()` (`erp/modules/base.php`) fails closed on an empty/missing extension allowlist instead of silently permitting any extension (including `.php`) — a prior audit found six homework-upload call sites passing an empty allowlist, which let any authenticated teacher/ principal/admin upload an executable file disguised as "homework." Any new upload call site must pass an explicit, non-empty allowlist. ### Storage isolation Production storage lives entirely outside this repository and outside `erp/`, per tenant, accessed only through `get_storage_path($module)` — see `docs/storage.md` for the full rationale (including a prior incident caused by hardcoded paths). A new file-handling code path that constructs a storage path any other way is a regression. ### Tenant isolation Each tenant (`erp/schools//`) has its own database connection and `$school_info`; shared code in `erp/modules/` must not hardcode a tenant's identity, branding, or data. A prior audit found and fixed hardcoded tenant identity in shared modules (`security: stop hardcoding SHPS identity in shared, tenant-agnostic modules`) — treat this as an ongoing discipline to check for in new shared code, not a closed issue. ### Navigation authorization `require_navigation_access()` in `erp/modules/nav_access.php` resolves the current script to a `navigation` row and denies the request when the signed-in user's effective permission map does not grant it. Fail-closed status, stated precisely because the two halves differ: - **The exception path fails closed.** `erp/schools/SHPS/admin/secure.php` previously caught every `Throwable` from the check and rendered the page anyway. It now calls `nav_access_deny()`, which records an `access_denied` activity entry carrying the deny reason (`navigation_check_failed` vs `not_permitted`) so a genuine denial can be told apart from a misconfiguration, and returns 403. - **The missing-row path still fails OPEN.** When the current script has no `navigation` row, `require_navigation_access()` returns without checking anything, so an unregistered admin page carries no authorization check at all. This is a known gap, deliberately not yet inverted — see Known gaps below. Two page-level privilege checks that previously granted the privilege when the `navigation` row was missing or the lookup threw now deny in both cases: `paymode_user_may_correct()` in `erp/schools/SHPS/admin/finance/receipt_lookup.php`, and `$can_correct` in `erp/schools/SHPS/admin/staff/attendance_punch.php`. Both write to `error_log` naming which condition denied. ### Session/write-blocking safety `erp/modules/maintenance_lock.php` provides a global write-block (`maintenance_engage()`/`maintenance_release()`), enforced centrally inside `sql_execute()` — not in `secure.php` — specifically because cron jobs and payment-gateway webhooks reach `sql_execute()` without ever including either `secure.php`; a gate placed there would silently miss them. Used for rare, high-risk operations (e.g. an admission-number change) that need the whole ERP to briefly stop mutating data. ### Audit logging - `admin_action_audit` — admin action history. - `admin_auth_log` — admin authentication events. - `app_config_audit_log` — every `app_config` value change, storing **masked** old/new values (via `mask_secret()`), never plaintext secrets. - `sql_log` — every database query app-wide (pruned periodically). Bound parameter values are logged only for queries whose text contains no sensitive column name; for any query mentioning a credential-bearing column the entire params array is replaced with a redaction marker (`sql_log_params()` in `erp/modules/base.php`, matching against `ACTIVITY_SENSITIVE_KEY_FRAGMENTS`). - `api_activity_log` — every outbound/inbound/webhook HTTP call. Outbound request payloads, response bodies and URL query strings pass through `api_log_redact()` / `api_log_redact_url()`, which reuse `activity_sanitize()` so any key matching the shared sensitive-fragment list is stored as `[redacted]`. Request *headers* have never been logged, so bearer tokens and PhonePe `X-VERIFY` checksums do not reach this table. - `cron_job_log` — every dispatched cron job's start/end/status/output. **Never log a credential, cron secret, session token, or other sensitive auth material in plaintext** — mask it first, following the existing `mask_secret()` pattern. ## Documented policy (not a code mechanism) - Schema changes are never applied automatically or silently — see `docs/database/README.md`. This is a process control, not a technical one; nothing in code currently prevents an ad hoc schema change outside that process. - Git safety rules (no force-push to `main`, no `--no-verify`, review diffs before committing) are documented in `CLAUDE.md` and enforced by discipline, not tooling. ## Known gaps / not yet implemented Stated plainly rather than omitted: - **No automated dependency/vulnerability scanning** (e.g. no Composer-based tooling, since dependencies are vendored — see `THIRD_PARTY_NOTICES.md` for what's bundled and its version). - **No formal incident-response runbook beyond `docs/runbooks/`**, which covers operational failure modes, not a security-breach response plan specifically. - **No automated security regression testing** — see `docs/testing.md` for current test coverage (minimal, and honestly stated as such). - **No rate limiting at the network/WAF layer** — throttling that exists (login, OTP) is application-level only. - **`verification/otp_core.php` keeps its own CSRF token** under `$_SESSION['otp_csrf']`, separate from `$_SESSION['csrf_token']`. It is a complete and correct implementation, not a hand-rolled one-off, but it means two token systems coexist. - **CI does not run a static-analysis security scanner** — see `.github/workflows/php-check.yml`, which currently validates PHP syntax only. - **`require_navigation_access()` fails open on a missing `navigation` row, by default.** An admin page with no row in that table is reachable by any authenticated admin user regardless of their permission map. Flipping the default to fail-closed outright is blocked on an audit of which of the 142 admin pages that include `secure.php` actually have rows — a question only the production `navigation` table can answer, not the repository. Two mitigations exist without that audit: every fail-open hit now logs a specific, greppable `error_log()` line (`require_navigation_access: no navigation row for "..."`), building the exact list of pages missing a row from real traffic instead of guesswork; and `app_config` `security.nav_access_fail_closed` (default off) flips the behavior to deny once the maintainer has used that log to confirm every legitimate page carries a row. - **`erp/schools/SHPS/app/admin/receipt_lookup.php` performs payment-mode correction without a navigation-permission check**, unlike its admin counterpart. It is still gated by `user_type === 'admin'`, CSRF and an OTP context, so this is a coarser check rather than an absent one. - **Historical `sql_log` rows written before this redaction existed still contain plaintext login PINs** paired with the phone number they belong to. Redaction stops new rows; it does not clean old ones. The purge statement is written but unapplied — see `database/migrations/015_purge_credential_rows_from_sql_log.sql`. - **`login_credentials.login_password` is still stored and compared in plaintext.** Redacting the log narrows exposure; it does not change how the credential itself is held. - **A short URL's target is logged in full** by `shortURL()`. Nested credentials are redacted when the wrapped value carries a sensitive parameter name, but a token under an unrecognised name would survive. ## Incident response expectations There is one maintainer and no on-call rotation. If a security incident is suspected in a live session: 1. Do not attempt a silent fix that could destroy evidence (e.g. don't truncate a table that shows unauthorized writes). 2. Flag it explicitly rather than proceeding with unrelated work. 3. Prefer disabling/blocking the specific vector (e.g. revoking a compromised `app_config` credential) over a broad, unreviewed change. 4. See `docs/runbooks/` for the closest matching operational runbook if the incident overlaps a documented failure mode (e.g. a webhook endpoint behaving unexpectedly).