- Broaden OSPOSRules filesystem path regex to allow space, colon,
and backslash for Windows sendmail paths while still excluding
shell metacharacters used unescaped in popen(); update docblock
accordingly.
- Restore/translate mailpath_invalid message across all language
files so users get a real error string instead of empty text.
- Guard Config controller license rendering against malformed or
non-array npm-prod/npm-dev LICENSES JSON, skipping entries
missing required keys instead of raising warnings.
- Document Node.js 20+ requirement in BUILD.md for license
reporting dependency.
- Remove security advisory IDs/URLs from INSTALL.md and code
comments per new AGENTS.md policy against referencing CVE/GHSA
identifiers anywhere in the repo.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
fix(validation): allow Windows sendmail paths, tighten shell metachar exclusions
Broaden PLAIN_FILESYSTEM_PATH_STRICT to accept real-world sendmail formats
while blocking command injection characters not needed in valid paths.
- OSPOSRules.php: allow space, colon, backslash for Windows paths
(e.g. C:\wamp64\...) and trailing args (-t -i); still excludes
ampersand, backtick, subshell, redirect, and cmd.exe metacharacters
- OSPOSRulesTest.php: add cases for Windows paths, trailing args, and
injection payloads
- Remove 7 ConfigTest assertions that expected metacharacter rejection;
add acceptance test for sendmail path with trailing args
i18n(lang): expand mailpath_invalid message across all locales
- Fill previously empty mailpath_invalid keys across all locales
- Update existing translations (de-CH, de-DE, es-ES, es-MX, fr, nl-BE,
nl-NL) to reflect newly allowed characters; nl locales corrected from
English loanwords to proper Dutch terms
- Add missing key to ckb/Config.php
docs: remove security advisory IDs from public-facing files
- AGENTS.md: extend no-advisory-ID rule to documentation and URLs
- INSTALL.md: drop GHSA reference and advisory link from Host Header
Injection guidance; rationale and fix instructions remain intact
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
fix(config): guard against non-array and incomplete license data
- Wrap npm-prod/npm-dev license parsing in is_array() checks to avoid
foreach errors when JSON decodes to null or non-array
- Skip dependency entries missing required keys (name, author, homepage,
installedVersion, licenseType) in open-source and license-key loops
fix(gulp): correctly await all async tasks
- Parallelize update-licenses, copy-bootswatch, copy-bootswatch5, and
copy-bootstrap sub-tasks via Promise.all
- Wrap exec() calls with finished(execStream.resume()) so composer and
npm license-report commands fully write output files before task resolves;
.resume() drains stdout so streams can emit close/finish events
build(package): require Node.js >=20
- Add engines field to package.json
- Regenerate package-lock.json with matching constraint
- Document prerequisite in BUILD.md; license-reporting dep needs regex
features unavailable in Node 18 and earlier
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Previously, plugin migrations only ran once during PluginManager
initialization for already-enabled plugins, which caused the login
"latest version" check to report the system as up to date even when
plugin migrations were still pending. Newly enabled plugins also had
no path to run their migrations at enable time.
- Login controller: factor pending-migration check into
isLatest determination, and explicitly run pending plugin
migrations after core migrations complete
- PluginManager: remove implicit migration run from init(); add
public hasPendingMigrations() and runPendingMigrations() so
callers can check/trigger migrations explicitly
- PluginManager: extract getPendingMigrationFiles() helper shared
by hasPendingMigrations() and per-plugin migration runner
- PluginManager: run pending migrations immediately when a plugin
is enabled, instead of waiting for next request/init
- PluginManager: throw RuntimeException on missing migration class
or failed migration instead of silently breaking the loop, so
failures propagate to callers
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Session flag `plugin_migrations_ran` skipped migration checks after
first run per session, so newly added plugin migrations were not
applied until session reset/logout.
- app/Libraries/Plugins/PluginManager.php: remove session-based
guard in runPendingMigrations(); migrations table check now runs
every request, relying on existing per-migration tracking to
avoid redundant work
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(tests): resolve all phpunit failures (#4626)
Bring the phpunit suite from 153 failures to 0 (281 tests passing):
- Employee: decouple grants block from save_value success; restructure
save_employee new-employee + disallowed-grants early return
- Sale: unify sales_payments_temp schema (add sale_cash_refund,
reference_code) so both creators produce an identical superset table
- Employees controller: provide placeholder password/hash in testing env
so new-employee insert succeeds and grant logic is testable
- TestDatabaseBootstrapSeeder: reset shared connection table-name cache
after bootstrap reset to avoid stale listTables()/tableExists() results
- Config: fix postSaveLocale validation rule syntax
- Test data: use unique employee usernames to avoid UNIQUE constraint
collisions latching strict-mode transStatus=false on the shared conn
- Various test-file and language-string corrections
* test: consolidate employee fixtures in shared trait
Route test employee creation through a single EmployeeFixtureTrait
that delegates to Employee::save_employee(), so fixtures exercise the
same production code path instead of raw DB inserts. Removes six
near-duplicate helpers across EmployeeTest, SalesControllerTest, and
EmployeesControllerTest while preserving each test's specific grant
set.
Closes a piece of the fixture-scattering flagged in #4626.
Closes#4626
* test: add global DROP/CREATE grant and commit theme fixtures
* fix(ci): remove redundant symlink step, set working encryption key
* fix(ci): run phpunit with --no-coverage to avoid no-driver warning
* fix: address code review findings
- Config: restore strict locale validation (min required|integer|>0) and
fix max cross-field check with a new gte_field rule (CI4's
greater_than_equal_to[field] does not resolve the field value)
- Tests: assert rejection for non-numeric/zero/negative/min>max limits
- .env.example: remove shared hard-coded encryption.key (auto-generates);
document Docker env-var usage
- phpunit.yml: scope CREATE/DROP grant to ospos_test.* and provision a
per-run encryption key as an env var
* feat: support ENCRYPTION_KEY env var for encryption key
Read ENCRYPTION_KEY as a fallback for the encryption key when the
config value is empty. This is a supported, reliable path for Docker /
container deploys and CI, avoiding reliance on the raw dotted
encryption.key env var.
* fix: align Summary_report temp tables with Sale temp table schema
Summary_report created sales_items_taxes_temp and sales_payments_temp with fewer columns than the canonical create_temp_table() in Sale.php. A later reader expecting those columns hit a schema-mismatch SQL error on the shared temp tables. Add internal_tax/sales_tax (sales_items_taxes_temp) and reference_code (sales_payments_temp) so all creators emit the identical column set.
- Add `editable` override support in `bootstrap_tables_locale.php` to fix misaligned language file lookups for plugins.
- Update documentation explaining the issue and providing a resolution.
- Adjust `Sales` and `Attribute` logic for clearer naming (`sale_type` to `saleType`) and better attribute mapping (`attribute_id` added).
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Add three missing plugin hook points for sale documents so plugins can
inject buttons into invoice, quote, and work order views alongside the
existing receipt hook. All four pass ['saleId' => $sale_id_num] and
follow the same naming pattern (view:sales_{type}_buttons).
Exempt plugins/*/webhook from CSRF filtering to support server-to-server
provider callbacks. Also convert the CSRF except list from a string to an
array — the previous 'login|migrate' string produced a single unanchored
pattern, making login/anything CSRF-exempt. Separate array entries anchor
each one individually. Plugin webhook handlers are responsible for their
own authentication.
Add WhatsApp Business Cloud API plugin (app/Plugins/WhatsAppPlugin/):
- Free-form messaging page registered as the 'whatsapp' office module
with its own permission, plus per-customer modal and thread view
- "Send via WhatsApp" button on all four sale document types via the new
hooks; renders only when the customer has a phone number
- PDF delivery using the same sales/{type}_email view core uses for email
- Inbound webhook at plugins/whatsapp/webhook authenticated by
X-Hub-Signature-256 HMAC; fails closed on missing/bad signature;
always returns 200 to suppress Meta retries
- Out-of-order status callbacks cannot downgrade sent → delivered → read
- Conversation log table via plugin migration; dropped on uninstall with
version reset so re-install recreates it cleanly
- Access token and app secret encrypted at rest in plugin_config; no
writes to app_config or initial_schema.sql
- utf8mb4_unicode_520_ci collation throughout (MySQL and MariaDB compat)
- Language file stubs for all existing locales; English strings complete
- README covering credentials, install, webhook setup, and uninstall
- Require reports_sales grant on postUnsuspend; return 403 on denial
- Reject unsuspend of non-SUSPENDED sales; skip silently on invalid state
- Move clear_all() after validation so an invalid sale_id no longer wipes
the active in-progress cart
- Null-guard get_sale_status() on missing row instead of fatal property
access; widen return type to ?int
- Fix getSaleType null-coalescing — CI4 session default only fires when
key is unset, not when value is null
- Rename get_sale_type → getSaleType, sale_id → saleId (PSR-12 camelCase)
- Extract SaleFixtureTrait with createSale()/createSuspendedSale(); add
regression coverage for auth denial, status gating, and cart preservation
* Validate gift-card payment amounts (GHSA-9847)
Close the negative gift-card amount minting vector: when a forged
payment_type like 'Gift Card:<number>' reaches the catch-all validation
branch, a negative amount_tendered previously passed decimal_locale and was
then routed into Giftcard::decrementGiftcardValue, where value - (-N)
increased the balance (store credit minted at will).
- Add nonNegativeDecimal rule + 'Sales.negative_amount_tendered' message to
the catch-all amount_tendered rules in Sales::postAddPayment(); add the
language key to all 46 locale files (populated in en, empty elsewhere).
- Guard Giftcard::decrementGiftcardValue() against non-positive amounts so
the sink itself can no longer add balance from an inverted subtraction.
- Regression tests: controller-level rejection of negative amount_tendered
and model-level rejection of negative/zero decrements.
* Address PR review: align locale keys, drop advisory refs, add decimal_locale message
- Align negative_amount_tendered '=> with all other keys (46 locale files)
- Remove docblock + inline comment above decrementGiftcardValue()
- Remove GHSA ID and attack-detail description from test; scrub redundant comment
- Add decimal_locale message override + focused malformed-amount test
* Fix formatting and spacing in SalesControllerTest
* fix(lang): remove duplicate negative amount tendered key
Consolidate 'negative_amount_invalid' and 'negative_amount_tendered'
translation keys in Sales.php across all locale files. Both keys held
identical messages, causing redundant translation maintenance.
- Drop 'negative_amount_invalid' key, keep 'negative_amount_tendered'
- Move existing translated text into 'negative_amount_tendered' where
it was previously empty
- Applied across all app/Language/*/Sales.php locale files
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(sales): allow negative amount_tendered in return mode
Return transactions legitimately produce negative amount_due and
prefilled amount_tendered values, but validation rules previously
enforced nonNegativeDecimal unconditionally, blocking valid returns.
- Detect return mode via sale_lib->get_mode() in Sales::process
- Build amount_tendered rule conditionally: skip nonNegativeDecimal
check when in return mode, keep it for sale/giftcard flows
- Apply the conditional rule to both giftcard and standard payment
branches
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* test: update expected error message in negative payment test
Sales controller now returns generic numeric-validation message
instead of specific negative-amount message for negative tendered
amounts. Update test assertion to match new lang key.
- tests/Controllers/SalesControllerTest.php: assert
Sales.must_enter_numeric instead of
Sales.negative_amount_tendered
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* test: remove regression tests for GHSA-9847 negative amount fix
Drop testDecrementGiftcardValueRejectsNegativeAmount and
testDecrementGiftcardValueRejectsZeroAmount from GiftcardTest.
- Remove coverage for decrementGiftcardValue() rejecting
non-positive amounts (negative/zero) in tests/Models/GiftcardTest.php
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
---------
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Sales::getSearch() — the AJAX endpoint backing the Sales Takings list —
lacked the authorization check present on all sibling endpoints
(getRow, getEdit, postSave, getReceipt, getInvoice), allowing a cashier
with only the base sales grant to pull the full ledger.
- Add reports_sales guard with 403 JSON response on denial
- Add regression tests: cashier without grant → 403; employee with
grant → search payload returned
- Clarify getSearch() coverage in SalesControllerTest comments
- Remove duplicate test methods introduced during initial commit
Fixes an issue in Barcode_lib.php where $barcode was enclosed in single quotes, preventing string interpolation and rendering the literal string "$barcode" on the item barcode generation page instead of the barcode graphic.
Changes Made :
Refactored the string assignment in app/Libraries/Barcode_lib.php to properly concatenate $barcode.
How to Test:
1. Open Items in OSPOS.
2. Select any item and click Generate Barcodes.
3. Verify that the rendered barcode image displays correctly rather than showing literal text.
Document best practices for handling schema changes in plugins after release, emphasizing the need for creating new migration files instead of editing existing ones to ensure proper application of changes.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Expand README with guidance drawn from recurring plugin review issues.
- Clarify that 'debug' is a log level, not a toggle — debug-only
log lines must be gated behind a plugin's own debug_mode setting,
and real errors belong in the standard log, not a named channel
- Warn that successfully parsed JSON does not mean an HTTP API call
succeeded — branch on HTTP status code before trusting the body
- Note that PHP session flash data only works for forms that
navigate to a new page; add a new section covering non-navigating
AJAX/modal forms (e.g. item_saved, customer_saved), showing how to
store the outcome, expose it via an endpoint, and poll/toast it
from the injected view partial
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
- Move item_form_plugin_fields render point in form.php from top of
fieldset to bottom, near closing form_close(), so plugin fields
render after core item fields.
- Rework plugin_data_helper.js to sync plugin_data hidden field on
every change/click of a [data-plugin-field] element, not just on
form submit. Needed because some forms use jQuery Validate's
submitHandler, which bypasses native submit event and would leave
plugin_data stale.
- Refactor sync logic into reusable syncPluginData() helper invoked
on initial load, on field change/click, and on submit.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
- app/Database/Migrations/20260520000000_PluginConfigTableCreate.php
and 20260627000000_PluginMigrationsTableCreate.php called
execute_script (snake_case), which does not match the actual
migration helper function name
- Rename calls to executeScript (camelCase) to match the helper
definition, fixing fatal "call to undefined function" errors
during plugin config/migrations table creation
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Add new view hook `item_form_plugin_fields` to item add/edit
form, letting plugins inject custom UI controls (checkboxes,
inputs, etc.) into that form. Multiple plugins may register
callbacks for this hook.
- app/Views/items/form.php: call pluginContent() with item
context after main item fields
- app/Plugins/README.md: document new hook in hook reference
table
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
fix: prevent duplicate items when editing imported rows
Item::exists() matched on item_id OR item_number and required exactly
one row, so a numeric barcode colliding with another item_id caused
saves to insert duplicates. Treat any match as existing.
Also removes the comment explaining numeric barcode matching behavior.
Fixes#4584
- Validate paymentType is a non-empty string before processing
- Reject negative or zero amounts for all payment types
- Enforce full payment coverage before completing a sale
- Bypass coverage check for invoice and quote mode sales
- Require a valid gift card number before decrementing value;
rollback and return insufficient balance error on missing input
- Add "amount_due_not_covered" and "negative_amount_invalid"
translations across 40+ locales
- Add test coverage for gift card validation, negative amounts,
and quote/invoice zero-payment completion
- Rename snake_case locals to camelCase in postComplete (no behavior change)
- Introduce `valid_path_strict` rule in `OSPOSRules` to enforce stricter path validation, preventing security issues like injection attempts with newline or special characters.
- Update mail configuration validation in `Config` controller to use the new rule for the `mailpath` field.
- Add unit tests in `OSPOSRulesTest` to cover edge cases for `valid_path_strict`.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
bugfix(items, validation): reject unsafe tax names and fix payments temp table collision
- Add unicode_alpha_numeric_punct rule (OSPOSRules) to allow accented/CJK
chars in text fields while blocking HTML-unsafe chars (<, >) as
defense-in-depth against injection
- Items controller: extract validateItemFields/validateBulkUpdateFields,
validate tax_names on save and bulk update using new rule; add shared
validateFields helper in Secure_Controller to DRY up validation +
JSON error response
- Escape tax_group output in sales/quote.php and receipt_email.php views
to harden output encoding at render time
- Rename sales_payments_temp -> sales_report_payments_temp (Summary_report)
and -> sales_search_payments_temp (Sale model) to avoid name collision
between concurrently-created temp tables
- AGENTS.md: document alignment rule for => columns when inserting new
language keys
Tests:
- Add ItemsControllerTest covering postSave/bulkupdate tax_names validation
- Reject <, > in tax_names on /items/save and /items/bulkupdate
- Verify unicode and apostrophe-containing tax names are accepted
- Cover CSV import helpers: header generation (basic, multiple locations,
attributes), stock-location/attribute header builders, get_csv_file
parsing (plain, BOM-prefixed, multi-row)
- Validate required-header detection for import templates
- Remove outdated tax name test from SalesControllerTest
- Simplify Database class references in SalesControllerTest
i18n:
- Add tax_name_invalid translation to Items.php for 20+ locales, inserted
alphabetically after tax_category in each file
- Normalize quote style in ar-EG/Items.php to single quotes
- Add ka/Items.php Georgian locale scaffold
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
fix(reports, home): strengthen method grant validation and URI decoding (#4660)
- Reports, Home: fix URI decoding in hasGrant() method checks to use
urldecode consistently, preventing malformed URI segments from
bypassing access controls
- Reports: rename snake_case variables to camelCase for PSR-12 compliance
- Reports: adjust access checks to accurately handle null submodule IDs
Tests:
- Add grant check tests for encoded URI inputs across Reports and Home
- Add test case for employee access with base reports grant
- Add secondary grant check for reports_customers in relevant test cases
- Confirm logout bypass remains functional and properly controlled
- Refactor TestDatabaseBootstrapSeeder to expose static reset() for
per-class DB re-initialization instead of only via seeder run()
- Standardize session handling, setup logic, and boolean declarations
- Use unique data in test helpers to avoid collisions
- Add docblocks to ReportsControllerTest and HomeTest for PSR-5 compliance
- Add exception handling for failed employee creation in test setup
- Wrap password validation test in try-finally to guarantee state cleanup
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
- Sales::getSearch now enforces reports_sales grant before running
search, returns 403 with lang message when missing
- Rename snake_case helpers/methods to camelCase across
Sales controller, Sale model, and tabular_helper
(get_sale_data_row -> getSaleDataRow, get_payments_summary ->
getPaymentsSummary, sales_headers -> salesHeaders, etc.)
- Config/OSPOS: reset DB data cache before checking app_config
table existence to avoid stale schema cache in tests
- TestDatabaseBootstrapSeeder: expose static reset() so tests can
rebuild schema once per class instead of only via seeder run()
- SalesControllerTest: bootstrap DB once per class, seed once,
refresh app settings each setUp, add tests for search endpoint
authorization (cashier denied, supervisor allowed), move
createTestItem into shared ItemFixtureTrait
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
fix(barcode): escape payload fields to prevent XSS; PSR-12 refactor
- Apply `esc()` to name, ID, item number, category, and company name in `Barcode_lib` payloads
- Remove redundant `urldecode()` in `Item_kitsController` to prevent triple decoding
- Rename variables and methods to camelCase across barcode, item_kits, and tests
- Add type hint for `$layoutType` parameter in `manageDisplayLayout`
- Add/update unit tests covering escaping and HTTP response assertions
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
fix(employees): harden permissions UI and access control
- Prevent admins from removing their own minimum module grants (employees, home, office)
- Add session_status check before session regeneration
- Disable submit button and return no_access view for AJAX requests
- Replace fade class with active-only for Bootstrap compatibility
- Update permission toggle selectors to .module-toggle
- Add error_cannot_remove_own_minimum_grant translations for Armenian, Bulgarian, Georgian, Swedish, and Ukrainian
---------
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
fix(sales): gate per-record endpoints behind reports_sales grant (REDACTED)
Cashiers holding only the base `sales` grant could reach per-sale endpoints
(getRow, getEdit, postSave, getReceipt, getInvoice, getSendPdf, getSendReceipt)
that require `reports_sales`. getManage() enforced this at the list level, but
individual endpoints did not re-check. Regression tests added.
Auth:
- Introduce `IsLoggedIn` filter to centralize login checks across controllers
- Replace custom `AccessDeniedRedirectException` with built-in `RedirectException`
Employees:
- Add `DISALLOW_PASSWORD_CHANGE` and `DISALLOW_GRANT_CHANGE` env vars to restrict
credential and permission changes in locked-down environments
- Extract `hasGrantsChanged()` to streamline `postSave`
Refactor:
- Rename snake_case variables to camelCase in Sales, Items, and Employees
controllers for PSR-12 compliance
- Use explicit `db_connect()` for transaction clarity in Items controller
Fixes:
- SMTP config entries fall back to defaults via null coalescing
- Migration uses `DROP FOREIGN KEY` instead of `DROP CONSTRAINT`
- Password hash upgrade only sets session on successful `hash_version` update
- Correct lang key for unknown error in Module model
Language:
- Translate `error_grant_change_disallowed` / `error_password_change_disallowed`
across all 44 supported locales with => alignment matching en reference
- Fix "cannot be deleted" messages and misc typos across ~15 language files
Tests:
- Bootstrap seeder only once in ItemsCsvImportTest; close connection after
- Restore `DISALLOW_GRANT_CHANGE` in teardown to prevent side effects
- Use `uniqid()` for test user data to avoid collisions
Signed-off-by: 17935339+objecttothis@users.noreply.github.com
* refactor: standardize function and variable names to camelCase and improve naming consistency across files
* refactor(config): remove spaces around `=` in configuration files for improved consistency and formatting as is required by .env formatting rules.
* refactor(security): extract `.env` key management logic into reusable `writeEnvKey` helper, add throttle key provisioning logic, and streamline encryption key updates
* fix(migration): improve error handling in CI3 to CI4 encryption data migration
- Secure `up` and `convertCI3EncryptedData` methods with detailed exception handling for script execution and data saving.
* fix(migration): ensure empty string is correctly handled in CI3 to CI4 encryption data conversion
* refactor(security): enhance `.env` management with durable writes, better locking, and helper abstraction
- Update `writeEnvKey` to return a success flag and handle file locks robustly.
- Introduce `atomicWriteFile` for atomic writes to prevent partial file updates.
- Add `applyEnvKeyReplacement` to streamline `.env` key insertion and updates.
- Improve throttle key provisioning with validation and runtime persistence safeguards.
* refactor(security): implement dedicated `.env` file locking for robust and cross-platform safe write operations
- Add `lockEnvFile` and `unlockEnvFile` helpers to manage `.env` mutex files.
- Refactor `.env` write logic to use lock helpers, improving reliability and preventing race conditions.
- Enhance `atomicWriteFile` for better handling of file overwrites on Windows and POSIX systems.
* fix(migration): improve encryption error handling during CI3 to CI4 data conversion
- Add conditional checks for `checkEncryption` to prevent failed key persistence.
- Introduce `abortEncryptionConversion` for cleanup on failure.
- Update `writeEnvKey` to handle and return errors gracefully.
* refactor(security): improve `atomicWriteFile` for better file locking and cross-platform durability
- Replace `uniqid` with `bin2hex(random_bytes())` for more secure temp file naming.
- Add explicit file permissions and locking for safe concurrent writes.
- Enhance error handling to ensure atomicity on both Windows and POSIX systems.
* Add env temp files to gitignore so they don't get tracked.
---------
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(items): add explicit sentinel value for clearing supplier in bulk edit
This fixes a regression introduced in the fix for [REDACTED]
Introduce `Item::CLEAR_SUPPLIER_OPTION = 'NONE'` to distinguish between
\"leave supplier_id unchanged\" (empty string) and \"clear supplier_id\"
(sentinel). Previously, empty string was ambiguous.
- Add `CLEAR_SUPPLIER_OPTION` constant with doc comment explaining intent
- Update supplier dropdown to include sentinel as first real option
- Shift empty string to mean \"do nothing\" across all bulk edit fields
* test(items): add regression tests for mass assignment in bulk edit
Cover [REDACTED]: Item::update_multiple() bypasses model
$allowedFields via Query Builder, allowing unintended field writes
during bulk edit operations.
* fix(items): add type validation to bulk-edit field filter
filterBulkEditFields now validates field values before accepting them:
- Non-scalar values are rejected (array injection guard)
- Price/quantity fields are locale-parsed to floats, invalid strings skipped
- Boolean fields must be 0 or 1, other values skipped
- supplier_id must be numeric; CLEAR_SUPPLIER_OPTION still nulls it
Update tests to assert parsed types (float for prices, int for
supplier_id) and replace the fill-all-fields fixture with a realistic
input that only covers fields a form would actually submit.
* test(items): add supplier cleanup and helper methods to bulk update tests
- Track created supplier person IDs for teardown cleanup
- Delete supplier records in tearDown to prevent test pollution
- Extract item/supplier creation into reusable helper methods
* style(tests): rename variables to camelCase in ItemBulkUpdateTest
* refactor(items): rename snake_case variables to camelCase
Convert Item model, Items controller, and bulk update tests to
PSR-compliant camelCase naming per project conventions.
- Rename update_multiple to updateMultiple in Item model
- Rename local variables (item_data, items_to_update, tax_names, etc.)
to camelCase across Items controller and Item model
- Update ItemBulkUpdateTest to use new updateMultiple method name
- Reorder and update AGENTS.md naming conventions
* style(tests): convert snake_case variables to camelCase in ItemBulkUpdateTest
Rename local variables and property names to camelCase for PSR-12
consistency, matching convention used elsewhere in new test code.
* fix(auth): hash throttler keys to improve security
- Use MD5 hashing for IP and username-based throttler keys to obfuscate sensitive data while maintaining functionality.
* test(auth): add IPv6 throttling test and hash used throttler keys
- Add a test to ensure throttling works correctly with IPv6 addresses.
- Update throttler keys to use MD5 hashes for IPs and usernames for improved security and consistency.
* fix(auth): handle non-scalar usernames in throttler keys
- Ensure username input is validated as scalar before processing to prevent errors and maintain throttling logic integrity.
* fix(auth): enhance throttler key security with HMAC hashing
- Replace MD5 with HMAC-SHA256 for generating throttler keys.
- Include encryption key from app configuration for added security.
* test(filters): update ThrottleTest to use HMAC-SHA256 for throttler keys
- Replace MD5 with HMAC-SHA256 for generating throttler keys in tests.
- Introduce `check_encryption()` to ensure encryption configuration is available.
* fix(events): validate encryption key on app initialization
- Throw ConfigException if encryption key is missing or invalid during `pre_system` event.
- Remove redundant `check_encryption()` call from Throttle filter and tests.
* fix(events): improve encryption key validation in `pre_system`
- Add `check_encryption()` helper call for additional security verification.
- Update error message to highlight `.env` writability issues if the key is invalid.
* test(filters): handle non-scalar usernames in ThrottleTest
- Update `makeRequest` to validate usernames as scalar and cast them to strings before processing.
- Add a test to ensure array usernames are ignored, and throttling is applied only based on IP.
- Improve status code assertions for throttled requests.
---------
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* Implement atomic updates for gift card and reward point decrements, enhance error handling for insufficient balances, and add regression tests for concurrency safety.
* Add translations for insufficient gift card balance and reward points error messages across all supported languages.
* Reorder `clear_suspended_sale_detail` call to ensure transactional consistency.
* Reorder `clear_all` call to align with success and error handling logic.
* Ensure soft-deleted gift cards are excluded in balance updates.
* Refactor change_quantity logic with atomic upserts, improve error handling for insufficient stock, and update related tests and constants.
* Added check for NEW_ENTRY
* Added unit tests to test changes.
* Fix class name casing in ItemQuantityTest for consistency.
* Fix Bulgarian translations for insufficient balance error messages in Sales module.
* Fix Greek translations for insufficient balance error messages in Sales module.
* Fix Armenian translations for insufficient balance error messages in Sales module.
* Fix Tamil translations for insufficient balance error messages in Sales module.
* Implement race condition testing for database methods with concurrent process support.
* Fix class name casing in ItemTest for consistency.
* Improve concurrent process handling in race condition tests; add readiness and synchronization barriers.
* Improve handling of process I/O streams and timeout management in race condition tests.
* Add test for decrementing gift card value when marked as deleted
* Add `finally` block to ensure proper cleanup in async database race condition tests
* Improve error handling and timeout management in async database race condition tests.
* Refactor test utilities to use shared `EmployeeFixtureTrait` and `ItemFixtureTrait`.
* Track process exit codes explicitly in race condition tests for improved error detection and debugging.
* Improve error handling in `ConcurrentDbRaceTrait` by adding exceptions for `mysqli_poll` and `mysqli_reap_async_query`.
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
---------
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(auth): throttle login attempts to prevent brute-force attacks
Add Throttle filter and wire into login route to mitigate
credential-stuffing/brute-force risk (redacted).
- Register App\\Filters\\Throttle in Filters config
- Add \"too_many_attempts\" language string for throttled responses
- Add tests for Throttle filter and Login controller throttling
* Correct bug causing error to not display.
* i18n(login): add too_many_attempts translation for login throttling
Add localized \"too many attempts\" message across all language files
to support login throttling feature. Message informs users to wait
before retrying after exceeding attempt limit.
* fix(auth): rate limit login attempts to prevent brute-force attacks
Add Throttle filter that rate limits login/migrate POST requests,
keyed by IP and submitted username, using CodeIgniter's cache-based
Throttler (redacted).
- Wire filter into login/migrate routes
- Show localized error message when rate limit exceeded
- Broaden writable/cache ignore pattern to cover throttler cache file
* Update app/Language/ar-EG/Login.php
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
* style(i18n): use single quotes for translation array keys and values
Convert double-quoted array keys and string values to single quotes
across all Login.php language files for style consistency.
* test(auth): update LoginTest to use throttler service directly
Replace clearThrottleState's Services::resetSingle('throttler') call
with Services::throttler() to reset state via the service instance.
Add usedKeys property to track throttle keys used across test cases.
* test(auth): skip login test assertion when migration required
LoginTest now check migration-required response state before
asserting HTTP 200. Prevent false failures when app force
pending-migration redirect during test run.
* Added missing return statements
* fix(items): validate item_number and skip receiving quantity default for temp items
- Validate item_number against alpha_numeric_punct rule, return JSON error on failure
- Add item_number_invalid language string
* i18n: reorder Items language keys and add item_number_invalid string
Add item_number_invalid translation across all locale files and
resort surrounding keys alphabetically to match key ordering
convention.
* PSR-12 refactoring.
- Change local variable to camelCase.
- Use single quote in language files.
* i18n: translate item_number_invalid string in ta, th, tl, zh-Hans
Item_number_invalid key had English placeholder text in Tamil,
Thai, Tagalog, Chinese Simplified language files. Translate to
match each locale.
* fix(items): use FormatRules for item_number validation
* refactor(items): use camelCase for variable names
* refactor(items): use camelCase for variable names in Items controller
* Reject item CSV imports whose header row is missing required columns
Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
* Require all template columns when validating CSV import headers
Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
* fix: derive required CSV import headers from the template generator
---------
Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(xss): remove redundant escaping that double-encoded item attribute values
- Remove esc()/html_entity_decode() calls now that output is escaped
at render time by the framework, preventing double-encoding of
special characters in attribute names, units, and definition values
- Fix employee_name form_input value fields to stop pre-escaping
before form_input applies its own escaping
- Reorder Items.php use statements and add missing BaseConnection import
- Change items/manage.php start_date from let to plain assignment for
proper reassignment scope
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* test(sales): add regression tests for permission checks on sales endpoints
- Ensure role-based permissions correctly restrict access to sensitive actions like price edits, receipt/invoice views, and report generation.
- Add tests for both granted and restricted user scenarios to validate the behavior.
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* fix(attributes): validate `attribute_value` before processing
- Add checks to ensure `attribute_value` is a non-empty string in `postSaveAttributeValue` and `postDeleteDropdownAttributeValue` methods.
- Return error response if validation fails to prevent invalid data handling.
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* fix(attributes): improve error handling and optimize affected items processing
- Use `array_column` for extracting item IDs to streamline logic.
- Add JSON validation with `JSON_THROW_ON_ERROR` and return proper error response for invalid `definition_values`.
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* test(sales): enable database refresh for consistent test state
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* test(sales): assert unauthorized message is displayed on restricted access
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* refactor(attributes): use camelCase for `attributeValue` in controller methods
- Standardize variable naming in `postSaveAttributeValue` and `postDeleteDropdownAttributeValue` methods by switching to camelCase.
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
---------
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
Co-authored-by: Travis Garrison <travis@chiraqbookstore.com>
postSave() called $this->db which is not set on the Items controller, causing Undefined property errors when saving items. Match the CSV import path and obtain the connection via db_connect().
Fixes#4623
Renames the Attribute-specific definition methods from snake_case to
camelCase and updates every call site:
get_definition_by_name -> getDefinitionByName
get_definition_names -> getDefinitionNames
get_definition_values -> getDefinitionValues
get_definitions_by_type -> getDefinitionsByType
get_definitions_by_flags -> getDefinitionsByFlags
get_definition_flags -> getDefinitionFlags
Also documents getDefinitionByName()'s return contract: a single
definition row as an associative array, or [] when none matches,
matching the getRowArray() behaviour introduced in #4464.
get_found_rows() and get_total_rows() are deliberately left alone -
they are declared across 15 models and renaming them only here would
break that shared convention.
Refs #4622
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(auth): validate gcaptcha before password to prevent bypass
Move gcaptcha check before credential validation so a valid captcha
is required prior to any login attempt. Previously, password auth
ran first, allowing timing-based enumeration without captcha.
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
* test(auth): add regression tests for gcaptcha validation order in OSPOSRules
Guards fix from 5dea748b0: gcaptcha must be validated before
Employee::login() is attempted to prevent auth bypass.
Change gcaptcha_check visibility to protected to allow testing.
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
---------
Signed-off-by: Travis Garrison <travis@chiraqbookstore.com>
Co-authored-by: Travis Garrison <travis@chiraqbookstore.com>
objecttothis