mirror of
https://github.com/opensourcepos/opensourcepos.git
synced 2026-10-28 19:18:58 -04:00
master
257
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 objecttothisandClaude Sonnet 5
|
600417bfdf |
refactor: extract PSR-12 renames and independent bugfixes from plugin system PR (#4784)
build/refactor/fix: tighten tooling, rename accessors to camelCase, and correct model bugs * build: tighten gitignore, lockfile, and script config - gitignore /build and /.php-cs-fixer.cache - allow core-js@3.50.0 postinstall script in package.json * refactor: rename snake_case data accessors to camelCase Rename get_info, get_multiple_info, get_sale_items, get_sale_payments, and save_customer across Customer, Employee, Item, Sale, and Supplier models to PSR-12 camelCase, updating every call site in controllers, libraries, and views. * test: update calls to match renamed camelCase accessors Update HomeTest, ItemsControllerTest, and ItemTest to call getInfo()/getMultipleInfo() instead of the old snake_case model methods. * fix: correct independent bugs in Item, Item_taxes, and Attribute models - Item::delete()/delete_list(): let $this->db->transStatus() alone decide success instead of accumulating a bitwise-AND over intermediate results, which could report failure (and trigger error toasts) even when the delete had actually completed - Item_taxes: correct $table property from 'item_taxes' to 'items_taxes' - Attribute::getDefinitionNames(): pass an array to QueryBuilder->whereNotIn(), which requires one Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
objecttothisandClaude Sonnet 4.6
|
74e825ba6f |
feat(stock-locations): add default location and sort order support (#4643)
Refactor Stock_location model to camelCase naming and introduce
is_default and sort_order fields with full migration, UI, and
item-sync support. Multiple follow-up fixes consolidated.
Database
- Add is_default (TINYINT, backfilled to oldest non-deleted row) and
sort_order (nullable unsigned TINYINT unique, dense 0..N-1) columns
via new migration; MySQL 5.7-compatible backfill using user variables
instead of ROW_NUMBER(); down() drops both columns
Model
- Rename show_locations → showLocations (camelCase convention)
- Add getDefaultLocationId(), setDefaultLocation(), saveSortOrder();
validate location existence before setting as default
- delete() compacts sort_order via two-pass CASE update to avoid
collisions after prior reorders
- Atomic sort_order assignment via explicit SQL COUNT query
Controller / Config
- Modularize save logic into sanitizeSubmittedLocations(),
deleteRemovedLocations(), saveSubmittedLocations(),
saveLocationOrderAndDefault(); wrap in single transaction
- Support stock_location_new inputs for new-row creation
- Resolve new-* ID placeholders to real IDs post-save
- Abort loop on save failure; default missing POST arrays to []
- Validate that at least one location array is non-empty
Views
- Add Default column with center-aligned radio buttons; disable for
unsaved rows; use attr() instead of data() for location IDs
- Escape dynamic location IDs against XSS in all affected partials
- form_count_details.php: fall back to default location when current
is undefined; guard quantity input similarly
- Add aria-label to default radio inputs
Libraries / Items
- Widen Item_lib::get_item_location() and get_stock_destination() to
nullable return types
- Guard null location_id in Items::getRow() and getGenerateBarcodes()
- Item::get_multiple_info(): accept ?int $location_id; short-circuit
with where('item_id', -1) instead of always-false where('1','0')
i18n
- Add stock_location_default, stock_location_default_header,
stock_location_name_header across all locale files
Tests
- Stock_locationTest: getDefaultLocationId, setDefaultLocation,
saveSortOrder, delete compaction, saveValue; rename to
testMethod_Scenario convention
- Add ConfigControllerTest / StockLocationsControllerTest with
AdminAuthTrait covering CRUD, reorder, default assignment, and
malformed/empty/blank payload validation
- DatabaseBootstrapTrait-based setup for ItemTest and Stock_locationTest
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
||
|
jekkosandcoderabbitai[bot]
|
42025e7817 |
fix(items): allow empty tax rows on item save (#4774)
* fix(items): allow empty tax rows on item save - Remove 'required' from tax_names.* rule; use permit_empty instead - Add permit_empty before nonNegativeDecimal for tax_percents.* - Add empty-string guard in nonNegativeDecimal() rule --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> |
||
|
jekkosandobjecttothis
|
625bb3536a |
fix(taxes): reject negative tax rates and percentages (#4730)
* fix(taxes): reject negative tax rates and percentages
Reject negative tax values across all tax entry points so they can
never be persisted and produce inverted sales totals:
- Items::postSave / postBulkUpdate: validate tax_percents.* as
nonNegativeDecimal (was unvalidated, persisted raw).
- Items::save_tax_data (CSV import): fail the row on a negative
Tax N Percent instead of silently dropping the tax.
- Taxes::postSave: reject negative tax_rate before saving.
- Config::postSaveTax: reject negative default tax rates.
Adds non-negative regression tests for Items, Taxes and Config, plus
boundary tests proving a zero tax rate/percent is still accepted. New
language strings for the rejection messages.
* test: adopt test{Method}_{Purpose} naming convention for new tests
Align the negative-tax test method names with the convention established
in ConfigTest (testPostSave_RejectsNegativeTaxPercent, etc.), per review.
* fix(config): add language entries to all locales, extract TaxFixtureTrait
* fix(i18n): place tax_rate_non_negative in alphabetical order
Move the new tax_rate_non_negative entry to its correct alphabetical
position (after tax_rate_error_adding_updating) in all locale files.
---------
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
|
||
|
enricodelarosa
|
73708ee4f7 |
feat(reports): add detailed item sales report (#4538)
Add a new detailed item sales report that shows all items sold per row in an exportable format, including sale transaction information. The existing detailed transaction sales report lacked per-item row display and export support, and the items summary report did not classify by sale. - Wire sale and discount filters for the report - Validate detailed item sales route filters - Add English fallback translations for other languages - Use blank quotes for other language translations - Normalize language key placement - Use PSR-style names throughout the report |
||
|
Rayan Abdul CaderandCursor Agent
|
7ea3f21365 |
Return early from sortAndFilterCart() when no items are printable (#4713)
* Return early from sort_and_filter_cart() when no items are printable (#4685) * Use camelCase naming for cart sorting helper --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> |
||
|
jekkosandobjecttothis
|
1cf9cb9120 |
fix(appconfig): refresh cached settings once and avoid stale same-request reads (#4732)
* fix(appconfig): refresh cached settings once and avoid stale same-request reads Addresses issue #3693, which has two related problems: 1. Repeated cache refreshes. Appconfig::batch_save() called save() per key, and save() unconditionally deleted + reloaded + re-cached the settings after every single write. A settings page with dozens of keys therefore refreshed the cache dozens of times in one request. batch_save() now persists all keys in the transaction and refreshes the cached settings exactly once after it commits successfully. save() keeps refreshing so direct single saves still work. 2. Stale same-request reads. Controllers copy config(OSPOS::class)->settings at construction time, so after a save in the same request the local copy is out of date. Config::postSaveInvoice() read the stale invoice_enable / default_register_mode values and sent the user to the wrong screen (e.g. opening a new sale instead of the invoice form after saving an enabled invoice). It now reads the freshly refreshed settings. Adds tests/Models/AppconfigTest.php covering batch_save() and save() refreshing the cached settings after a successful write. * test(3693): adopt test{Method}_{Purpose} naming convention Align AppconfigTest method names with the convention from #4730 (testBatchSave_PersistsAllKeysAndRefreshesCachedSettings, etc.), per review. * style(config): remove inline comments per review feedback --------- Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com> |
||
|
jekkosandobjecttothis
|
f90adab493 |
fix(taxes): reject invalid characters in tax code, category, and jurisdiction names (#4733)
* fix(taxes): reject invalid characters in tax code, category, and jurisdiction names Apply the same unicode_alpha_numeric_punct validation already used for item tax names to the three tax save endpoints, so stored-XSS payloads (<, >) are rejected with a validation error instead of being saved. Mirrors the Items fix and adds a TaxesControllerTest regression suite. * fix(taxes): permit slash and CJK in tax names, add regression tests The unicode_alpha_numeric_punct guard rejected '/' (char type Po), which blocks legitimate slash-separated tax names such as 'GST/HST' and 'VAT/GST' from being saved. Add '/' to the allowed punctuation set and document it. Also add regression tests proving acceptance of a slash name (GST/HST) and a CJK name (消費税) across the tax save endpoints. Addresses CodeRabbit review on #4679. * test(4679): adopt test{Method}_{Purpose} naming convention Align test method names with the convention from #4730 (testPostSaveTaxCodes_RejectsMaliciousName, etc.), per review. * fix(taxes): allow parentheses in names, make name optional Address CodeRabbit review on #4733: - Permit parentheses in tax names (e.g. "VAT (20%)"); slash was already allowed, so both "GST/HST" and "VAT (20%)" now pass while < and > are still rejected. - Drop the rule from tax_code_name, jurisdiction_name and tax_category so a code with a blank name can be saved, matching the form (the form does not require the name). - Add regression tests for the parentheses and blank-name cases. --------- Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com> |
||
|
objecttothisandOllama
|
1b8ee2e3c1 |
feat(items): optimize search, attribute filtering, and sort for items view (#4652)
feat(items): optimize search, attribute filtering, and sort for items view (#4652) BREAKING CHANGE: none ## Search & Query Optimization - Split item search into two-phase query: Phase A resolves qualifying IDs, Phase B joins scoped display tables for better performance and readability - Sanitize `definition_ids` via `array_map` to prevent injection vulnerabilities - Introduce subquery for SUM aggregation to prevent over-counting across joins - Add validation requiring both start and end dates before applying date range filter ## Attribute Search (fixes #2919, #2722) - Add `SHOW_IN_SEARCH` flag (value 8) to Attribute model to separate searchability from table visibility - Add `parse_attribute_search()` to parse syntax like `color:blue AND size:large` - Add `applyNamedAttributeSearch()` supporting decimal and date types with locale-aware parsing - Support AND/OR logic for multi-attribute queries ## Sorting - Add `get_attribute_sort_definition_id()` to detect attribute column sorting - Join attribute tables dynamically when sorting by attribute columns - Use `MAX()` for consistent results when sorting by attribute values - Replace static sort column list with dynamic headers via `itemSortColumns()` - Add `sanitizeSortColumnAttribute()` to validate attribute definition IDs as sort columns ## Tax & Data Row - Streamline tax computation in `getItemDataRow()` ## Low Inventory Filter - Require valid `stock_location_id` before applying low inventory filter - Add conditional logic to sort by sum of quantities across all locations when `stock_location_id` is invalid ## Localization - Add `show_in_search` / `show_in_search_visibility` strings to all language files - Translated: de-DE, es-ES, fr, it; English placeholder for remaining locales - Unify single-quote style across all attribute language files ## Refactoring & Style - Adopt camelCase naming throughout (variables, helpers, methods) - Replace `sanitizeSortColumnAttribute` with reusable `sanitizeSortColumn` from `Secure_Controller` - Simplify column key extraction using `array_key_first` - Apply PSR-12 formatting ## Tests - Add tests for tax computation, quantity aggregation (single- and multi-location), named attribute search, free-text parsing, and date/decimal type handling - Add `ensureStockLocation` helper to auto-create missing stock locations in tests - Refactor tests to handle config cache issues Co-authored-by: Ollama <ollama@steganos.dev> |
||
|
jekkos
|
edcb4bb655 |
Fix GHSA-frx7-c5vv-m3mr: recompute cashup total server-side and force owner identity (#4706)
* Fix GHSA-frx7-c5vv-m3mr: recompute cashup total server-side and force owner identity * Add required description field to cashup test POSTs |
||
|
jekkos
|
47aade5024 |
fix(locale): validate language_code against known locales to block path traversal (#4704)
* fix(locale): validate language_code against known locales to block path traversal postSaveLocale() stored language_code from user input with no allow-list validation, and it later flows into Language::setLocale()/load() where the locale segment is require()'d. An authenticated config-grant account could store a relative path (e.g. ../../public/uploads) and, combined with a planted file in public/uploads/, achieve unauthenticated RCE on the next request. Validate the submitted language against array_keys(get_languages()) before storing, and harden languageExists() to reject path separators and dot-dot sequences. Adds regression tests. * test(locale): give locale fixture valid reference-code min/max defaults * fix(locale): reject null bytes in languageExists guard A stored language_code containing a NUL byte passes the existing path-separator and parent-dir checks, then reaches file_exists(). On PHP 8.5+ file_exists() throws a ValueError for NUL-byte paths, which breaks configuration loading. Reject NUL bytes in the guard and add regression tests. |
||
|
|
184918d914 |
fix(security): handle special characters in .env key values and improve insertion logic (#4656)
* fix(security): handle special characters in `.env` key values and improve insertion logic - Escape backslashes and dollar signs in `applyEnvKeyReplacement` to prevent unintended value corruption. - Ensure new keys are inserted after `encryption.key` for better organization and manageability. - Add explicit cast to int to prevent wrong concatenation operator warning. Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com> * fix(security): handle null return in `applyEnvKeyReplacement` and ensure proper `.env` updates - Update `applyEnvKeyReplacement` to return `null` on failure, improving error handling. - Adjust calls to `atomicWriteFile` with updated content to prevent unintended behavior. Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com> * fix(security): improve error logging and exception messages in file locking - Add detailed logging for file open and locking errors in `security_helper`. - Remove unused `helper` and `checkThrottleEncryption` calls from `Events` for cleanup. Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com> * fix(security): improve atomic file write and handle encryption key placement - Throw `RandomException` for better error reporting in `atomicWriteFile`. - Simplify Windows-specific `rename()` fallback logic. - Fix `encryption.key` assignment order to ensure consistency in `.env` updates. Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com> * fix(security): improve `.env` file handling and add unit tests for helper functions - Suppress warnings in `file_get_contents` to prevent unnecessary error logs. - Update `applyEnvKeyReplacement` to use `preg_replace_callback` for better safety. - Add comprehensive unit tests for `security_helper` functions to ensure `.env` updates and key management work as expected. Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com> * fix(security): enhance `.env` update logic and add robust exception handling - Add `RandomException` to improve error reporting in encryption key management. - Introduce environment file locking for safer `.env` updates. - Ensure `applyEnvKeyReplacement` properly handles and inserts old key comments. - Replace direct file writes with `atomicWriteFile` for consistency. Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com> * fix(security): refactor `.env` file initialization and encryption key handling - Introduce `initializeEnvFile` for reusable `.env` setup logic. - Add `backupEnvFile` and `writeNewEncryptionKey` for robust key management with backups. - Simplify and clean up redundant `.env` handling code paths. Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com> * fix(security): clarify `checkEncryption` docblock return value description Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com> * fix(security): escape backslashes and dollar signs in `applyEnvKeyReplacement` - Ensure `applyEnvKeyReplacement` properly escapes special characters when inserting or appending `.env` keys. - Add new unit tests to validate correct handling of backslashes and dollar signs. Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com> * fix(i18n): add localized error messages and improve error reporting in `security_helper` - Add missing translations for error messages across multiple language files. - Update `security_helper` to use localized exception messages with placeholders. Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com> * Redesign encryption/throttle key provisioning as read-only runtime - checkEncryption()/checkThrottleEncryption() are now read-only guards that throw when no valid key is provisioned, instead of writing .env at request time. - Add rotateEncryptionKey() and provisionThrottleKey() for explicit, idempotent provisioning. - Add php spark env:provision (app/Commands/EnvProvision.php) so Docker can provision keys once at container startup before any request. - Add app/Libraries/CI3SecretConverter.php shared CI3->CI4 secret converter (AES-128-CBC decrypt + CI4 re-encrypt/verify/save) used by both the interactive migration and the docker startup path. - Refactor convertToCI4 migration to use the shared converter. - Persist .env in a named volume and run spark env:provision on boot; stop baking .env into the shipped image. - Add guard/rotation/throttle + converter tests; clean up orphaned msg_pwd_required language keys across all locales. * fix: save CI4 ciphertext in env:provision and bind-mount a .env file Addresses CodeRabbit review on PR #4656: - env:provision CI3 branch was persisting *plaintext* secrets (saveAll($plain)) instead of the CI4 ciphertext, unlike the ConvertToCI4 migration. Now encrypts with encryptAll(), verifies the round trip, and saves the ciphertext. - The ospos_env named volume mounted at /app/.env made .env a directory, so atomicWriteFile's rename() failed and spark env:provision could not start apache. Switch to a bind mount of a host file (./.env) which persists and stays a file. - Add a regression test asserting the command persists ciphertext (not plaintext). * chore: trim redundant docblocks in EnvProvision and provision throttle.key in CI Follow up on @objecttothis review comments: - app/Commands/EnvProvision.php: remove the boilerplate docblocks the property names already convey (group/name/usage/description, run()), the two inline step comments, the anyNonEmpty() param docblock, and the legacySecretsPresent() docblock. Keeps the class-level docblock since it is the only place that states the read-only runtime design + the never-persist-plaintext invariant. - .github/workflows/phpunit.yml: provision a per-run throttle.key the same way the encryption key is already provisioned. The PR makes checkThrottleEncryption() a read-only guard that throws when env('throttle.key') is unset; CI only started exporting ENCRYPTION_KEY, so every test that goes through the Throttle filter (7 ThrottleTest cases + 4 LoginTest cases) failed with "No throttle key is provisioned. Run `php spark env:provision`". Writing `throttle.key=<KEY>` into .env matches what `php spark env:provision` does on a real container start. * fix(ci): write throttle.key into .env instead of exporting an OS env var The previous attempt exported throttle.key via GITHUB_ENV, but CodeIgniter's env() helper resolves in the order $_ENV[$key] ?? $_SERVER[$key] ?? getenv($key), and DotEnv populates $_ENV['throttle.key'] from the .env file first. Because the .env (copied from .env.example) ships with the empty placeholder throttle.key='', that $_ENV entry exists as '' and short-circuits the ?? chain before getenv() is reached — so the OS env var was never consulted and every Throttle/Login test still threw 'No throttle key is provisioned'. Write the per-run key into the .env file itself (sed-replacing the empty placeholder), which is exactly what `php spark env:provision` does in production and is the single source env() actually reads from. Verify the replacement happened (grep -Eq '^throttle\.key=.') so a future change to the placeholder format fails the run loudly instead of silently breaking the 11 throttle-dependent tests. * fix(security): restore CI3->CI4 auto-provisioning gated by .env writability checkEncryption()/checkThrottleEncryption() again provision the keys inline when .env is writable (empty key -> generate; short key -> decrypt, rotate, re-encrypt, verify, persist legacy CI3 secrets). When .env is not writable they assume the key was provisioned externally (e.g. docker env:provision) and throw. Update helper tests to match and correct the EnvProvision docblock that claimed the runtime was strictly read-only. * test(security): make short-key conversion branch injectable and test it checkEncryption() now accepts an optional CI3SecretConverter so the CI3->CI4 conversion branch can be exercised in unit tests without a database. Adds testCheckEncryptionConvertsCi3ShortKeyWhenEnvWritable which seeds CI3-era ciphertexts via a fake Appconfig model and asserts the key is rotated and the payload verifies back to the original plaintext. * fix(security): abort on backup/read/saveAll failure to avoid data loss Three related data-integrity fixes: - backupEnvFile() now returns true/false based on whether the backup actually exists and is readable. rotateEncryptionKey() aborts before destroying the key when the backup could not be written to disk. - rotateEncryptionKey() and provisionThrottleKey() throw RuntimeException(Error.unable_to_read_env_file) when the .env read fails, instead of silently replacing the whole file with an empty string. This prevents a permission error from wiping all keys. - checkEncryption() and EnvProvision::run() now both roll back to the backup with abortEncryptionConversion() when the post-rotation saveAll() throws, matching the migration path (which already did this). A failing fake Appconfig is used to exercise this in the new testCheckEncryptionRollsBackWhenSaveAllFails test. * fix(ci): skip comment job in deploy-pr.yml when prepare was not run The comment job had if: always(), so it ran even when the prepare job was skipped (e.g. review was not approved). With PR_NUMBER empty the gh api call posted to issues//comments, received a 404, and the entire run showed up as failure. Guard the job with needs.prepare.result == 'success' so it only runs when PR_NUMBER is valid. * address coderabbit open items: placeholder guards, message neutrality, ar-EG alignment - backupEnvFile(): fail when mkdir() or either chmod() fails, so the pre-rotation backup is actually persisted before the key is replaced - email/message config views: only show the 'already set' placeholder when the secret is actually present (prevented false positives on fresh installs) - Error.unable_to_create_env_file / .unable_to_read_env_file (en + en-GB): use key-neutral wording since both keys are provisioned with the same keys - ar-EG/Error.php: align all => arrows on the longest key Item 7 (filesystem test isolation) is a larger refactor — the tests are serial on CI and tearDown() restores state per test. Left for follow-up. * test(security): isolate helper FS tests via Config\SecurityEnv Introduce Config\SecurityEnv holding envPath/backupPath/lockPath so the security helper reads its target paths from shared configuration instead of hardcoded ROOTPATH/WRITEPATH literals. security_helperTest.php now redirects all three to a unique per-run sandbox under sys_get_temp_dir() and tears it down in tearDown(), so the suite no longer reads/writes the repository's real .env and is safe to run in parallel. No helper signature changes; production callers unaffected. Addresses CodeRabbit item 7 (issue #4700). Co-Authored-By: opencode <bot@opencode.ai> * fix(security): run key-conversion as one locked transaction Address CodeRabbit Major findings from the 4th re-review of the env helper and its callers: 1. Hold .env.lock for the entire CI3 -> CI4 conversion transaction (backup -> rotate -> re-encrypt -> verify -> persist -> cleanup) so a concurrent worker cannot interleave a key write between the rotation and the ciphertext save. Split rotateEncryptionKey into a lock-free core (rotateEncryptionKeyUnlock) plus the existing lock wrapper and a new rotateEncryptionKeyTransaction that owns the lock across the full unit and performs both the in-lock rollback (abortEncryptionConversion) and the in-lock backup removal on success. 2. Treat the legacy value '0' as non-empty data so key rotation still persists the re-encrypted ciphertext when '0' is the only stored secret (array_filter would have dropped it and skipped saveAll). 3. Wrap the post-rotation re-encrypt/verify/saveAll sequence in a catch (Throwable) across all three call-sites so CI4 EncryptionException, ReflectionException from batch_save, a failed round-trip verify, and any other failure all roll the .env key back to the pre-rotation state. 4. In Docker Compose, use long-syntax bind with create_host_path: false and document in INSTALL.md that the host .env must be a regular file (a missing one is no longer auto-created as a directory, and the mount now rejects a missing source on Compose implementations that support the flag). Files touched: app/Helpers/security_helper.php, app/Commands/EnvProvision.php, app/Database/Migrations/20220127000000_convertToCI4.php, docker-compose.yml, INSTALL.md. All 4 existing helper tests still pass via CI. * fix(security): make abortEncryptionConversion fail loudly on restore failure The rollback path restored the .env backup with a suppressed file_put_contents() and an unchecked file_get_contents(). If the restore failed after the key had already been rotated, .env was left holding the new CI4 key while the DB still held CI3-era ciphertext, so the data became undecryptable after the next restart. Now the backup read is checked for false and the restore goes through the existing atomicWriteFile() helper; either failure throws so the error is surfaced instead of silently corrupting the config. Adds a regression test that forces an unreadable backup and asserts the throw plus that .env is left untouched. * fix(security): guard abortEncryptionConversion backup read before touching it Validate the backup is a regular readable file (is_file/is_readable) before reading it, so a missing/malformed backup fails loudly instead of emitting a file_get_contents() warning. The unreadable-backup regression test now exercises this guard rather than relying on a promoted warning. --------- Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com> Co-authored-by: jekkos <jeroen.peelaerts@gmail.com> Co-authored-by: jekkos <jekkos@users.noreply.github.com> Co-authored-by: opencode <bot@opencode.ai> |
||
|
objecttothis
|
6a36cdc3e9 |
fix(licenses): guard malformed data, parallelize gulp tasks, require Node 20
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> |
||
|
jekkos
|
28755dfd50 |
fix(tests): resolve all phpunit failures — clean-DB suite green (#4626) (#4691)
* 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. |
||
|
objecttothis
|
9ecabf6f41 |
fix(sales): harden unsuspend with auth, status gating, and null safety
- 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 |
||
|
jekkosandobjecttothis
|
839821e2eb |
bugfix(sales): reject non-negative gift-card amount_tendered (#4674)
* 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> |
||
|
objecttothis
|
f5f9052de1 |
fix(sales): harden payment validation and gift card handling
- 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) |
||
|
objecttothis
|
fdc1c38b43 |
feat(validation, tests): add valid_path_strict rule and integrate into mailpath validation (#4684)
- 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> |
||
|
objecttothis
|
6db3dde491 |
Bugfix tax names (#4677)
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> |
||
|
objecttothis
|
905a447e55 |
fix(reports, home): resolve double-URL-decoding bypass for method grants (#4660) (#4666)
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> |
||
|
objecttothis
|
8cfd1a4b1d |
fix(sales): enforce reports_sales grant on search endpoint (#4673)
- 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> |
||
|
objecttothis
|
d63d31700d |
Ensure payload data is escaped to prevent XSS (#4664)
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> |
||
|
objecttothis
|
84bddcdd88 |
Feature admin account safeguards (#4657)
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> |
||
|
objecttothis
|
c701d21886 |
fix(sales): gate per-record endpoints behind reports_sales grant (#4627)
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 |
||
|
objecttothis
|
9ee530e454 |
Hotfix: Fix CI3 database migration caused by regression (#4649)
* 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> |
||
|
objecttothis
|
cc93c31355 |
fix(items): add explicit sentinel value for clearing supplier in bulk edit (#4617)
* 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. |
||
|
objecttothis
|
29a9b1a7e7 |
Bugfix: Resolve Race Condition in Rewards and Gift Card Spending (#4640)
* 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> |
||
|
objecttothis
|
a2b91493c7 |
Feature: CodeIgniter Throttler (#4619)
* 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
|
||
|
objecttothis
|
8bd2a51cb5 |
fix(items): validate item_number and skip receiving quantity default for temp items (#4621)
* 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 |
||
|
Sai Asish Yandobjecttothis
|
0e8fc0963a |
Reject item CSV imports whose header row is missing required columns (#4597)
* 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> |
||
|
objecttothisandTravis Garrison
|
5c9b1b81e6 |
fix(xss): remove redundant escaping that double-encoded item attribute values (#4628)
* 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> |
||
|
richardmilles
|
08b0951a84 |
fix: use db_connect() for item save transactions (#4636)
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 |
||
|
objecttothisandTravis Garrison
|
f5ba1709eb |
fix(security): sanitize filenames and escape logo path in config (#4630)
* fix(security): sanitize filenames and escape logo path in config - Sanitize uploaded filename in Config.php via preg_replace, strip chars outside [a-zA-Z0-9_-] before storing raw_name - Escape $logo_src with esc(..., 'attr') in info_config.php view to prevent XSS via crafted logo path/filename Prevents stored XSS and path traversal from unsanitized filenames used in config uploads. Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * fix(config): sanitize uploaded config filenames Replace inline regex filename sanitization with sanitize_filename() helper to prevent path traversal via crafted upload filenames. Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> --------- Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> Co-authored-by: Travis Garrison <travis@chiraqbookstore.com> |
||
|
Rayan Abdul Caderandobjecttothis
|
aa96ad0284 |
refactor: apply PSR-12 naming to Attribute definition methods (#4624)
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> |
||
|
objecttothisandTravis Garrison
|
3bafa07e04 |
fix(sales): enforce server-side authorization for price changes (#4631)
Client-side \"change_price\" flag is UI-only, not trustworthy. Compare submitted price against current cart price server-side and require sales_change_price grant when they differ, else reject with not_authorized error. Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> Co-authored-by: Travis Garrison <travis@chiraqbookstore.com> |
||
|
|
2c69dc0c34 |
fix(config): validate theme param to prevent XSS via invalid theme (#4620)
* fix(config): validate theme param to prevent XSS via invalid theme Add validation rule for theme field before batch save, rejecting requests with unrecognized theme values. Add test coverage for theme validation in postSaveGeneral. Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * Update app/Config/Validation/OSPOSRules.php Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> Co-authored-by: Travis Garrison <travis@chiraqbookstore.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> |
||
|
|
0a4c418690 |
fix: get_definition_by_name() returns single row instead of multi-dimensional array (#4452) (#4464)
Co-authored-by: jekkos <jekkos@users.noreply.github.com> Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com> |
||
|
|
c0abf80c86 |
fix: Wrap postSave() in single transaction for atomicity (#4506)
* fix: wrap postSave() in single transaction for atomicity - Remove internal transaction from Item_taxes->save_value() to allow controller-level transaction - Wrap entire save sequence (item, taxes, quantities, inventory, attributes) in single transaction - Ensure all operations succeed or all fail together - Prevents partial writes when saveItemAttributes() fails after item/tax/quantity saves succeed Fixes #4474 * fix: Use explicit transBegin/transCommit/transRollback for atomicity - Replace transStart/transComplete with transBegin/transCommit/transRollback - Check all success conditions before committing - Explicit rollback on failure Address CodeRabbit review feedback * refactor(items): rename postSave locals to camelCase per PSR-12 Convert snake_case variables to camelCase in postSave() and related item-save logic to match project naming convention for new methods. Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> --------- Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> Co-authored-by: Ollama <ollama@steganos.dev> Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com> Co-authored-by: Travis Garrison <travis@chiraqbookstore.com> |
||
|
|
cdf477f823 |
fix(login): skip auth validation on new install to allow migration (#4609)
* fix(login): skip auth validation on new install to allow migration On fresh installs with no DB version, login validation would fail before migrations could run. Check MY_Migration::getCurrentVersion() and bypass credential check when no version exists. Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * fix(login): distinguish DB unavailable from empty install `getCurrentVersion()` previously returned `int` with 0 used for both \"empty database\" and \"DB connection failure\". This conflated two distinct states, causing login to skip auth when DB was unreachable. Changes: - Return type widened to `?int`: null = DB unavailable, 0 = confirmed empty DB (new install), positive int = migrated - Login controller now returns 503 on null (DB error) before checking isNewInstall - isNewInstall gate now uses `=== 0` instead of falsy check Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * fix(login): return 503 JSON response when DB connection unavailable Extract getCurrentVersion() before building data array to distinguish null (connection failure) from 0 (new install). Return early with JSON 503 when DB is unreachable instead of rendering broken view. Update return type hint to include ResponseInterface. Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> --------- Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> Co-authored-by: Travis Garrison <travis@chiraqbookstore.com> Co-authored-by: jekkos <jekkos@users.noreply.github.com> |
||
|
objecttothisandTravis Garrison
|
eb4c3d8764 |
fix(giftcard): correct return type and rename getGiftcardId method (#4600)
- Rename get_giftcard_id to getGiftcardId (PSR-12 camelCase) - Fix return type from bool to int|false - Replace loose == with strict === comparison - Update all call sites in Giftcards controller and Sale model Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> Co-authored-by: Travis Garrison <travis@chiraqbookstore.com> |
||
|
objecttothisandTravis Garrison
|
9c542efaf6 |
Feature: Payment reference code (#4587)
* Add `reference_code` to sale payment queries and group by statements Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * Refactor `Sales` controller to improve payment handling readability and replace snake_case with camelCase variables Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * Add missing translations for `Sales` language file and include new keys like `must_enter_rrn` and `reference_code` Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * Refactor `Sales` payment handling to use camelCase and extend `addPayment` with `referenceCode` support Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * Refactor `Sales` models, controllers, and libraries to adopt camelCase naming conventions and improve readability Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * Add translations and updates for `must_enter_reference_code` and `reference_code` across language files and update `Sales` controller to replace `must_enter_rrn` with the new key Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * feat(sales): add reference code input and payment type helper - Add `get_reference_code_payment_types()` to locale_helper as single source of truth for card-requiring payment types - Add reference code row to register view, shown/hidden via JS based on selected payment type - Fix payment type dropdown width to 100% for consistent layout - Add min-width to payment buttons and right-padding to button group Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * feat(config): add payment reference code length configuration - Add payment_reference_code_min and payment_reference_code_max fields to Config controller save logic - Add translation keys for reference code length limits across all language files (min/max label + section header) - Align array key formatting in Config controller for readability Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * style(lang): normalize string delimiters to single quotes across all language files Convert double-quoted array keys and values to single quotes in all app/Language/*/Config.php and app/Language/*/Sales.php variants. No translation content changed — formatting only. Also add Localization section to AGENTS.md documenting language file conventions for new keys and fallback behavior. Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * feat(lang): add payment reference code length translations Add localized strings for payment_reference_code_length_limits, payment_reference_code_length_max_label, and payment_reference_code_length_min_label across all supported locales. Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * test(config,sales): add payment reference code min/max validation tests - Add baseLocalePayload() helper in ConfigTest for postSaveLocale tests - Add testSaveLocale_AcceptsValidReferenceCodeMinMax and related boundary tests - Add Sale_libPaymentTest for payment reference code validation in Sale_lib Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * fix(sales): add type-specific validation for amount_tendered Gift card payments use amount_tendered as giftcard number (integer); cash/other payments require decimal_locale format. Apply correct validation rule per payment type instead of generic required. Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * fix(lang): replace self-closing </br> with <br> in all locales Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * fix(sales): use configurable precision in discount comparison Replace hardcoded precision 2 with totals_decimals() when comparing discount against item total via bccomp/bcmul, so discount validation respects the configured decimal precision setting. Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * fix(lang): correct Azerbaijani translations in Config.php Replace placeholder/mismatched strings with accurate translations: - email_mailpath, email_smtp_pass, invoice_email_message - number_locale_invalid/required, receipt_template - reward_configuration, right, tax_decimals, theme Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * feat(sales): add reference code support to payment edit flow - Add reference_code field to new payment row in sale edit form - Persist reference_code on insert in Sale model - Validate reference_code_new in Sales controller using configurable min/max length rules Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * feat(lang): add Georgian (ka) language stubs for Config and Sales Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * Match the fallback maximum reference_code length to the maximum of the field in the db Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * Bug fixes - Add validation of UI settings to prevent overridden values from being passed. - Correct maximum value in JS for payment_reference_code maximum length to 40. - Fix bug causing copy_entire_sale() to incorrectly copy the reference code and cash_adjustment Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> --------- Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> Co-authored-by: Travis Garrison <travis@chiraqbookstore.com> |
||
|
objecttothisandTravis Garrison
|
dec736ad6c |
Bugfix: Fix problems with migration UI in login (#4589)
* refactor(login): rename methods and keys to camelCase, add initialization keys - Rename snake_case methods to camelCase in MY_Migration and callers: is_latest() → isLatest(), migrate_to_ci4() → migrateToCI4(), get_latest_migration() → getLatestMigration(), get_current_version() → getCurrentVersion() - Rename snake_case array keys to camelCase in Login controller: has_errors → hasErrors, is_new_install → isNewInstall, is_latest → isLatest, latest_version → latestVersion, gcaptcha_enabled → gcaptchaEnabled - Add initialization_required, initialization_message, initialize keys to all language files to support new install flow messaging Signed-off-by: Travis Garrison <travis@chiraqbookstore.com> * refactor(login): move auth validation before migration, reorder language keys alphabetically - Validate credentials with login_check rule before attempting migration - Return 401 with invalid credentials message on validation failure - Alphabetically reorder and align language file keys across all locales - Add new migration status keys: migrating_database, migration_complete, migration_complete_login, migration_complete_migrate - Remove deprecated keys: migration_needed, migration_initializing, migration_running, migration_required 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
|
e6388deed8 |
Fix overly lenient date validation (#4574)
* Fix overly lenient date validation - In the past date validation would roll over dates that didn't exist into the next month. Now they return an error. Signed-off-by: objec <objecttothis@gmail.com> * Refactor naming - Refactor parameter - Refactor function name. Signed-off-by: objec <objecttothis@gmail.com> * Add unit tests for LocaleHelper Signed-off-by: objec <objecttothis@gmail.com> * Remove files from being tracked. Signed-off-by: objec <objecttothis@gmail.com> --------- Signed-off-by: objec <objecttothis@gmail.com> |
||
|
jekkosandOllama
|
84aeeb52fe |
fix(security): Fix DOMPDF RCE and customer email sanitization (#4568)
* fix(security): Fix DOMPDF RCE and customer email sanitization - Disable isPhpEnabled in DOMPDF to prevent RCE via embedded PHP in HTML - Disable isRemoteEnabled to prevent SSRF attacks - Add email validation and sanitization in CSV import (FILTER_SANITIZE_EMAIL, FILTER_VALIDATE_EMAIL) - Reject invalid email formats during customer import * fix(security): Escape email addresses in mailto() to prevent XSS Email columns in bootstrap tables had escaping disabled (line 52) and mailto() function doesn't escape its parameters. This fix escapes email addresses before passing to mailto() in: - get_person_data_row() (employees) - get_customer_data_row() (customers) - get_supplier_data_row() (suppliers) Attack vector: Malicious email via CSV import renders XSS in table view. * test(security): Add tests for customer CSV import email validation Tests cover: - Valid email acceptance - Invalid email rejection with row-specific error - XSS payload sanitization in email field - Mixed valid/invalid email handling - Email with special characters sanitization Verifies fixes for customer email import vulnerability. * fix(security): Allow empty email addresses in customer import - Empty emails are now allowed (customers may not have email addresses) - Validation only applies when email is non-empty - Added test case for empty email acceptance This fixes a regression where FILTER_VALIDATE_EMAIL rejected empty strings, breaking imports for customers without email addresses. --------- Co-authored-by: Ollama <ollama@steganos.dev> |
||
|
objecttothis
|
b7384296c1 |
Bugfix: Sale search in register not handling trailing space properly (#4557)
* Fix is_valid_receipt method bug Strings submitted with a trailing space and no number caused an unhandled exception because Sale::exists() expects an int but a string was passed to it. - Add guards - Minor PSR refactor Signed-off-by: objec <objecttothis@gmail.com> * Address review comments Signed-off-by: objec <objecttothis@gmail.com> --------- Signed-off-by: objec <objecttothis@gmail.com> |
||
|
objecttothis
|
b0dddc22a3 |
Bugfixes to get Migration working on MySQL and MariaDB (#4551)
* Bugfixes to get Migration working on MySQL Signed-off-by: objec <objecttothis@gmail.com> * MariaDB compatibility fixes - Drop foreign key constraints before making charset changes - Fix dropAllForeignKeyConstraints helper function. - Added `IF EXISTS` to DROP statements - Do not try to readd FK constraints for tables which were dropped. - MariaDB 11.8.x changes the default implicit collation to uca1400 which breaks the IndiaGST migration, et. al. Explicitly declare utf8_general_ci in affected migrations. Signed-off-by: objec <objecttothis@gmail.com> * Fix changes which break MySQL migrations - MySQL does not support IF EXISTS in foreign key constraints. Since the PHP is now handling dropping those constraints, these lines are redundant. Remove them. Signed-off-by: objec <objecttothis@gmail.com> * Resolve code review recommendations - Add try/catch around DB connect statement - Heed result of execute_script function and throw an exception on failure. Signed-off-by: objec <objecttothis@gmail.com> * Refactor out duplicate code Signed-off-by: objec <objecttothis@gmail.com> * Initialize array variable causing potential issues Signed-off-by: objec <objecttothis@gmail.com> --------- Signed-off-by: objec <objecttothis@gmail.com> |
||
|
jekkosandOllama
|
9c89a2e2cb |
fix: Capture CSV import failures in save_tax_data and save_inventory_quantities (#4507)
* fix: capture CSV import failures in save_tax_data and save_inventory_quantities - Change save_tax_data() return type from void to bool - Change save_inventory_quantities() return type from void to bool - Accumulate failure status with &= operator in save_inventory_quantities - Update postImportCsvFile() to capture return values and set isFailedRow - Properly propagate failures to failCodes array Fixes #4475 * fix: Change isset to !empty for items_taxes_data check - isset was always true since array was initialized - Use !empty to properly check if there are tax items to save Address CodeRabbit review feedback * fix: Capture inventory insert result in save_inventory_quantities - Combine inventory insert result with success tracking - Use &= operator to accumulate failures from both operations - Ensure failures from inventory inserts are propagated Address CodeRabbit review feedback --------- Co-authored-by: Ollama <ollama@steganos.dev> |
||
|
jekkosandOllama
|
2f51c4ef52 |
fix(security): SQL injection and path traversal vulnerabilities (#4539)
Security fixes for two vulnerabilities:
1. SQL Injection in Summary Sales Taxes Report (GHSA-5j9m-2f98-cjqw)
- Fixed unsanitized user input concatenation in getData() method
- Applied proper escaping using $this->db->escape() for start_date/end_date
- Consistent with existing _where() method implementation
2. Path Traversal in Receipt Template (GHSA-h6wm-fhw2-m3q3)
- Added ALLOWED_RECEIPT_TEMPLATES whitelist constant
- Added isValidReceiptTemplate() validation method
- Validate receipt_template before saving in Config controller
- Validate receipt_template before rendering in receipt view
- Default to 'receipt_default' for invalid values
- Consistent with invoice_type fix pattern (commit
|
||
|
jekkosandOllama
|
def0c27a0e |
fix(security): Path traversal vulnerability in getPicThumb (#4545)
Security impact: - Authenticated attackers could read arbitrary files on the server - Path traversal via unsanitized pic_filename parameter - Could read .env, config files, encryption keys Fix: - Apply basename() to strip directory components - Validate file extension to allowlist image types only - Add explicit error response for invalid file types CVE: Pending Affected: <= 3.4.2 Reported by: Kamran Saifullah (VulDB) Co-authored-by: Ollama <ollama@steganos.dev> |
||
|
|
90c981b6b7 |
feat: Bank transfer and wallet payment option added #4540 (#4547)
--------- Co-authored-by: Lotussoft Youngtech <lotussoftyoungtech@gmail.com> Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com> |