* 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>
- Add column to indicate control setting (installed, enabled).
- Add column to indicate plugin.
- Rework business logic to read the status properly.
- Renamed the migration to properly reflect which version it's released in.
Signed-off-by: objec <objecttothis@gmail.com>
- getItems() gets Item data from the item table for an array of item ids.
- getAttributeValuesBulk() front loads attribute values for an array of items.
Signed-off-by: objec <objecttothis@gmail.com>
- getDateAdded and getDatesAdded in Inventory.php
- GetDistinctCategories in Item.php
- GetBulkItemQuantities in Item_quantity.php
- GetBulkInfo in Item_taxes.php
- GetStockLocationsByItem in Stock_location.php
Signed-off-by: objec <objecttothis@gmail.com>
- Pass an array to QueryBuilder->whereNotIn()
- Refactor function names for PSR compliance
- Add explanatory PHPdocs and corrections
- Correct bug with items_taxes model
- Refactor local variables for PSR compliance
Signed-off-by: objec <objecttothis@gmail.com>
- Add missing `MailchimpPlugin.` prefix to lang() calls.
- Do not subscribe customer if consent is not true.
- Escape output in tabular_helper.php
- Removed testConnection() as unneeded code
- Fix activity count logic
- Whitelist Sort Column Headers for Plugins.php
- Store encrypted API key as base64 instead of raw binary to prevent truncation
- Rollback on batchSave partial failure.
- Remove dead code.
- Disable plugin before uninstalling it.
- Fix getPluginSettings() internal key leak
- Add action column to plugin headers function
- Automatically add grant to all admins in case person_id 1 is not active
Signed-off-by: objec <objecttothis@gmail.com>
- Fix the output of pluginContent in the pluginHelper
- Register view injection events
- Correct the parameter type in getMailchimpViewData
- Correct the statusOptions creation business logic
- Removed unnecessary view injection point
- Corrected which variable was passed to the customer_saved event
- Assigned $customer_data['person_id'] on customer update
- Added renderView() function to BasePlugin.php
Signed-off-by: objec <objecttothis@gmail.com>
* fix: Catch mysqli_sql_exception in DB fallback handlers for fresh Docker installs
On a fresh Docker install with an empty database, the ospos_sessions
table doesn't exist yet. The CSRF filter triggers session initialization
before the login/migration page can be reached.
The existing code in Session.php, OSPOS.php, and MY_Migration.php
catches DatabaseException, but the MySQLi driver throws
mysqli_sql_exception (which extends RuntimeException, not
DatabaseException) when the table doesn't exist. This causes an
unhandled exception resulting in HTTP 500.
Fix: Change all three catch blocks from to
so that mysqli_sql_exception and any other unexpected
database errors are caught, allowing the app to fall back gracefully:
- Session.php: Falls back to FileHandler so sessions work without DB
- OSPOS.php: Falls back to empty settings so config loads work
- MY_Migration.php: Falls back to version 0 / false so the migration
check passes gracefully
This allows the login page with migration UI to be served on first
access, so the initial schema migration can run.
Fixes#4524
---------
Co-authored-by: Ollama <ollama@steganos.dev>
In PR #4250 (commit 29c3c55), orWhere was added to match items by
either item_id or item_number, but the OR condition was not wrapped
in groupStart()/groupEnd(). This causes:
1. Wrong SQL semantics: generates
WHERE item_id = ? OR item_number = ? AND deleted = 0
instead of
WHERE (item_id = ? OR item_number = ?) AND deleted = 0
Due to AND binding tighter than OR, the deleted filter only applies
to the item_number branch, allowing deleted items to match via item_id.
2. Performance: the unscoped OR causes MySQL to bypass the item_id
primary key index and fall back to full table scans when item_number
is a string column compared against a numeric parameter.
Both exists() and get_item_id() are fixed by wrapping the OR
conditions in groupStart()/groupEnd() for proper parenthesization.
Co-authored-by: Ollama <ollama@steganos.dev>
- Corrected grammar in PHPdocs
- PSR refactoring of local variables and code blocks
- Moved MailchimpPlugin.php to its own plugin folder
- Refactored out mailchimp code to the plugin
- Created customer_loaded event trigger
Signed-off-by: objec <objecttothis@gmail.com>
- Merge Config and Core File Changes 4.6.3 > 4.6.4
- Merge Config and Core File Changes 4.6.4 > 4.7.0
- Added app\Config\WorkerMode.php
- Merge Config and Core File Changes Not previously merged
- Added app\Config\Hostnames.php
- Corrected incorrect CSS property used in invoice.php view.
- Corrected unknown CSS properties used in register.php view.
- Used shorthand CSS in debug.css
- Corrected indentation in barcode_sheet.php view.
- Corrected indentation in footer.php view.
- Corrected indentation in invoice_email.php view.
- Replaced obsolete attributes with CSS style attributes in barcode_sheet.php
- Replaced obsolete attribute in error_exception.php
- Replaced obsolete attribute in invoice_email.php
- Replaced obsolete attribute in quote_email.php
- Replaced obsolete attributes in work_order_email.php
- Fixed indentation in system_info.php
- Replaced <strong> tag outside <p> tags, which isn't allowed, with style attributes.
- Simplified js return logic and indentation fixes in tax_categories.php
- Simplified js return logic in tax_codes.php
- Simplified js return logic in tax_jurisdictions.php
- Removed unnecessary labels in manage views.
- Rewrite JavaScript function and PHP to be more readable in bar.php, hbar.php, line.php and pie.php
- Added type declarations, return types and an import to app\Config\Services
- Updated Attribute.php parameter type
- Updated Receiving_lib.php parameter type
- Updated Receivings.php parameter types and updated PHPdocs
- Updated tabular_helper.php parameter types and updated PHPdocs
- Added type declarations and corrected PHPdocs in url_helper.php
- Added return types to functions
- Revert $objectSrc value in ContentSecurityPolicy.php
- Correct return type in Customer->get_stats()
- Correct return type in Item->get_info_by_id_or_number()
- Correct misspelling in border-spacing
- Added missing css style semicolons
- Resolve operator precedence ambiguity.
- Resolve column mismatch.
- Added missing escaping in view.
- Updated requirement for PHP 8.2
- Resolve unresolved conflicts
- Added PHP 8.2 requirement to the README.md
- Fixed bugs in display of UI
- Fixed duplicated `>` in app\Views\Expenses\manage.php
- Removed excess whitespace at the end of some lines in table_filter_persistence.php
- Added missing `>` in app\Views\Expenses\manage.php
- Corrected grammar in PHPdoc in table_filter_persistence.php
- Remove bug causing `\` to be injected into the new giftcard value
- Fix bug causing DROPDOWN Attribute Values to not save correctly
- Added check for null in $normalizedItemId
- Removing < PHP 8.2 from linting and tests
- Update Linter to not include PHP 8.2 and 8.1
- Remove PHP 8.1 unit test cycle.
- Update Bug Report Template
- Update Composer files for CodeIgniter 4.7.2
- Updated INSTALL.md to reflect changes.
---------
Signed-off-by: objec <objecttothis@gmail.com>
PSR and Readability Changes
- Removed unused import
- Corrected PHPdoc to include the correct return type
- Refactored out a function to get attribute data from the row in a CSV item import.
- refactored snake_case variables and function names to camelCase
- Refactored the naming of saveAttributeData() to better reflect the functions purpose.
- Improved PHPdocs
- Remove whitespace
- Remove unneeded comment
- Refactored abbreviated variable name for clarity
- Removed $csvHeaders as it is unused
- Corrected spacing and curly brace location
- Refactored Stock Locations validation inside general validation
Bugfixes
- Fixed bug causing attribute_id and item_id to not be properly assigned when empty() returns true.
- Fixed bug causing CSV Item import to not update barcode when changed in the import file.
- Fixed saveAttributeValue() logic causing attribute_value to be updated to a value that already exists for a different attribute_id
- Fixed bug preventing Category as dropdown functionality from working
- Fixed bug preventing barcodes from updating. in Item CSV Imports
- Corrected bug in stock_location->save_value()
- Corrected incorrect helper file references.
- Removed duplicate call to save attribute link
- Rollback transaction on failure before returning false
- Rollback transaction and return 0 on failure to save attribute link.
- Account for '0' being an acceptable TEXT or DECIMAL attributeValue.
- Corrected Business logic
- Resolved incorrect array key
- Account for 0 in column values
- Correct check empty attribute check
- Previously 0 would have been skipped even though that's a valid value for an attribute.
- Removed unused foreach loop index variables
- Corrected CodeIgniter Framework version to specific version
UnitTest Seeder and tests
- Created a seeder to automatically prepare the test database.
- Modified the Unit Test setup to properly seed the test database.
- Wrote a unit test to test deleting an attribute from an item through the CSV.
- Corrected errors in unit tests preventing them from passing. save_value() returns a bool, not the itemId
- Fix Unit Tests that were failing
- Corrected the logic in itemUpdate test
- Replaced precision test with one reflecting testing of actual value.
- This test does not test cash rounding rules. That should go into a different test.
- Correct expected value in test.
- Update app/Database/Seeds/TestDatabaseBootstrapSeeder.php
- Added check to testImportDeleteAttributeFromExistingItem
- Correct mocking of dropdowns
- Remove code depending on removed database.sql
- Removed FQN in seeder() call
- Added checks in Database seeder
- Moved the function to the attribute model where it belongs which allows testability.
Case Change Capability (CSV Import and Form)
- CSV Import and view Case Changes of `attribute_value`
- Store attribute even when just case is different.
- Add getAttributeValueByAttributeId() to assist in comparing the value
- Corrected Capitalization in File Handling Logic
CSV Import Attribute Link Deletion Capability
- Validation checks bypass magic word cells.
- Delete the attribute link for an item if the CSV contains `_DELETE_`
- Added calls to deleteOrphanedValues()
- Items CSV Import Attribute Delete
- Exclude the itemId in the check to see if the barcode number exists
Error Checking and Reporting Improvements
- Fail the import if an invalid stock location is found in the CSV
- Return false if deleteAttributeLinks fails
- Match sanitization of description field to Form submission import
- Fold errors into result and return value
- Populated $allowedStockLocations before sending it to the validation function
- Added logic to not ignore failed saveItemAttributes calls
- Add error checking to failed row insert
- Reworked &= to && logic so that it short-circuits the function call after if success is already false.
- Add transaction to storeCSVAttributeValue function to prevent deleting the attribute links before confirming the new value successfully saved.
- Modified generate_message in Db_log.php to be defensive.
Attribute Improvements
- Move ATTRIBUTE_VALUE_TYPES to the helper
- Normalize AttributeId in saveAttributeLink()
- normalize itemId in saveAttributeLink()
- Account for '0' in column values for allow_alt_description
- Remove duplicate saveAttributeValue call
- Correct return value of function
- Like other save_value() functions, the location_data variable is passed by reference.
- Unlike other save_value() functions, the location_data variable is not being updated with the primary key id.
- Added updateAttributeValue() function as part of logic fix.
- Added attribute_helper.php
- Simplified logic to store attribute values
---------
Signed-off-by: objec <objecttothis@gmail.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
The PluginConfig class extends CodeIgniter\Model which has its own set() method
for query building. Renaming get()/set() to getValue()/setValue() avoids this conflict.
Also fixed:
- batchSave() to use setValue() instead of set()
- Updated all callers in PluginManager and BasePlugin to use renamed methods
- Move plugin discovery to pre_system in Events.php (allows events to be registered before they fire)
- Add plugin existence check in disablePlugin()
- Add is_subclass_of check before instantiating plugin classes
- Fix str_replace prefix removal in getPluginSettings using str_starts_with + substr
- Add down() migration to drop table on rollback
- Fix saveSettings to JSON-encode arrays/objects
- Update README to use MailchimpPlugin as reference implementation
- Remove CasposPlugin examples from documentation
- Ensure total = subtotal + tax by deriving total from rounded components
- Use assertEqualsWithDelta for float comparisons in tests
- Add defensive null coalescing in calculateSummary helper
- Add missing 'count' key to test data rows
- Add testRoundingAtBoundary test case
The report had calculation inconsistencies where:
1. Per-line totals (subtotal + tax) didn't equal the total column
2. Column totals didn't match the sum of individual rows
Root cause: subtotal, tax, and total were calculated independently
using different formulas and rounding at different stages, leading to
cumulative rounding errors.
Fix:
- Use item_tax_amount from database as the source of truth for tax
- Derive subtotal from sale_amount (handling both tax_included and
tax_not_included modes correctly)
- Calculate total = subtotal + tax consistently for each line
- Override getSummaryData() to sum values from getData() rows,
ensuring summary totals match the sum of displayed rows
Fixes#4112
Add 'only_debit' filter to Daily Sales and Takings dropdown. Reuses
existing 'Sales.debit' language string for the filter label. Includes
filter default initialization in getSearch() to prevent PHP warnings.
Fixes#4439
* Fix DECIMAL attribute not respecting locale format
Issue: DECIMAL attribute values were displayed as raw database values
instead of being formatted according to the user's locale settings.
Fix:
1. Modified Attribute::get_definitions_by_flags() to optionally return
definition types along with names (new $include_types parameter)
2. Updated expand_attribute_values() in tabular_helper.php to detect
DECIMAL attributes and apply to_decimals() locale formatting
3. Updated callers (Reports, Items table) to pass include_types=true
where attributes are displayed
The DECIMAL values in table views (items, sales reports, receiving reports)
now respect the configured locale number format, matching DATE attributes
which already use locale-based formatting.
* Apply PSR-12 camelCase naming to new variables
Response to PR review comments:
- Rename to
- Rename to
- Rename to
---------
Co-authored-by: Ollama <ollama@steganos.dev>
* Fix SQL injection in suggestions column configuration
The suggestions_first_column, suggestions_second_column, and
suggestions_third_column configuration values were concatenated
directly into SQL SELECT statements without validation, allowing
SQL injection attacks through the item search suggestions.
Changes:
- Add whitelist validation in Config controller to only allow
valid column names (name, item_number, description, cost_price,
unit_price)
- Add defensive validation in Item model's get_search_suggestion_format()
and get_search_suggestion_label() methods
- Default invalid values to 'name' column for safety
- Add unit tests to verify malicious inputs are rejected
This is a critical security fix as attackers with config permissions
could inject arbitrary SQL through these configuration fields.
Vulnerability reported as additional injection point in bug report.
* Refactor: Move allowed suggestions columns to Item model constants
Extract the list of valid suggestion columns into two constants in the Item model for better cohesion:
- ALLOWED_SUGGESTIONS_COLUMNS: valid column names
- ALLOWED_SUGGESTIONS_COLUMNS_WITH_EMPTY: includes empty string for config validation
This consolidates the validation logic in one place and makes it reusable across Config controller and Item model.
* Address PR review comments: improve validation and code quality
Changes:
- Use camelCase naming for validateSuggestionsColumn() method (PSR-12)
- Add field-aware validation with different fallbacks for first vs other columns
- Handle non-string POST input by checking is_string() before validation
- Refactor duplicate validation logic into suggestionColumnIsAllowed() helper
- Use consistent camelCase variable names ($suggestionsFirstColumn)
- Update tests to validate constants and behavior rather than implementation
- Tests now focus on security properties of the allowlist itself
The validation now properly handles:
- First column: defaults to 'name' when invalid
- Second/Third columns: defaults to '' (empty) when invalid
- Non-string inputs: treated as invalid with appropriate fallback
---------
Co-authored-by: Ollama <ollama@steganos.dev>
This implements a clean plugin architecture based on PR #4255 discussion:
Core Components:
- PluginInterface: Standard contract all plugins must implement
- BasePlugin: Abstract class with common functionality
- PluginManager: Discovers and loads plugins from app/Plugins/
- Plugin_config: Model for plugin settings storage
Architecture:
- Each plugin registers its own event listeners via registerEvents()
- No hardcoded plugin dependencies in core Events.php
- Generic event triggers (item_sale, item_change, etc.) remain in core code
- Plugins can be enabled/disabled via database settings
- Clean separation: plugin orchestrators vs MVC components
Example Implementations:
- ExamplePlugin: Simple plugin demonstrating event logging
- MailchimpPlugin: Integration with Mailchimp for customer sync
Admin UI:
- Plugin management controller at Controllers/Plugins/Manage.php
- Plugin management view at Views/plugins/manage.php
Database:
- ospos_plugin_config table for plugin settings (key-value store)
- Migration creates table with timestamps
Documentation:
- Comprehensive README with architecture patterns
- Simple vs complex plugin examples
- MVC directory structure guidance
The bulk edit function iterated over all $_POST keys without a whitelist,
allowing authenticated users to inject arbitrary database columns (e.g.,
cost_price, deleted, item_type) into the update query. This bypassed
CodeIgniter 4's $allowedFields protection since Query Builder was used
directly.
Fix: Add ALLOWED_BULK_EDIT_FIELDS constant to Item model defining the
explicit whitelist of fields that can be bulk-updated. Use this constant
in the controller instead of iterating over $_POST directly.
Fields allowed: name, category, supplier_id, cost_price, unit_price,
reorder_level, description, allow_alt_description, is_serialized
Security impact: High (CVSS 8.1) - Could allow price manipulation and
data integrity violations.
The previous SQL injection fix (GHSA-hmjv-wm3j-pfhw) used named parameter
syntax :search: with having(), but CodeIgniter 4's having() method does
not support named parameters. This caused the query to fail.
The fix uses havingLike() which properly:
- Escapes the search value to prevent SQL injection
- Handles the LIKE clause construction internally (wraps value with %)
- Works correctly with HAVING clauses for aggregated columns
This maintains the security fix while actually working on CI4.
Parameterize LIKE queries in HAVING clause to prevent SQL injection
when search_custom filter is enabled. Also sanitize search parameter
input at controller level for defense-in-depth.
Fixes vulnerability where user input was directly interpolated into
SQL queries without sanitization.
- Move admin modules list from is_admin method to ADMIN_MODULES constant
- Rename is_admin() to isAdmin() following CodeIgniter naming conventions
- Rename can_modify_employee() to canModifyEmployee() following conventions
- Update all callers in Employees controller and tests
* Fix second-order SQL injection in currency_symbol config
The currency_symbol value was concatenated directly into SQL queries
without proper escaping, allowing SQL injection attacks via the
Summary Discounts report.
Changes:
- Use $this->db->escape() in Summary_discounts::getData() to properly
escape the currency symbol value before concatenation
- Add htmlspecialchars() validation in Config::postSaveLocale() to
sanitize the input at storage time
- Add unit tests to verify escaping of malicious inputs
Fixes SQL injection vulnerability described in bug report where
attackers with config permissions could inject arbitrary SQL through
the currency_symbol field.
* Update test to use CIUnitTestCase for consistency
Per code review feedback, updated test to extend CIUnitTestCase
instead of PHPUnit TestCase to maintain consistency with other
tests in the codebase.
---------
Co-authored-by: Ollama <ollama@steganos.dev>
- Non-admin employees can no longer view/modify admin accounts
- Non-admin employees can no longer delete admin accounts
- Non-admin employees can only grant permissions they themselves have
- Added is_admin() and can_modify_employee() methods to Employee model
- Prevents privilege escalation via permission grants
Add tests for BOLA fix and permission delegation
- EmployeeTest: Unit tests for is_admin() and can_modify_employee() methods
- EmployeesControllerTest: Test cases for authorization checks (integration tests require DB)
- ReportsControllerTest: Test validating the constructor redirect fix pattern
Fix return type error in Employees controller
Use $this->response->setJSON() instead of echo json_encode() + return
to properly satisfy the ResponseInterface return type.
* Add attachment cid when sending emails (#4308)
Also check if an encryption key is set before decrypting the SMTP
password.
* Upgrade to CI 4.6.3 (#4308)
* Fix for changing invoice id in email (#4308)
- Refactored function names for PSR-12 compliance
- Programmatically cascade delete attribute_link rows when a drop-down attribute is deleted but leave attribute_link rows associated with transactions.
- Added `WHERE item_id IS NOT NULL` to migration to prevent failure on MySQL databases during migration
- Retroactive correction of migration to prevent MySQL databases from failing.
- Refactored generic functions to helper
- Reverted attribute_links foreign key to ON DELETE RESTRICT which is required for a unique constraint on this table. Cascading deletes are now handled programmatically.
- Migration Session table to match Code Igniter 4.6
- Add index to attribute_links to prevent query timeout in items view on large databases
- Added overridePrefix() function to the migration_helper. Any time QueryBuilder is adding a prefix to the query when we don't want it to, this query can be used to override the prefix then set it back after you're done.
- Added dropAllForeignKeyConstraints() helper function.
- Added deleteIndex() helper function.
- Added indexExists() helper function.
- Added primaryKeyExists() helper function.
- Added recreateForeignKeyConstraints() helper function.
- Added CRUD section headings to the Attribute model.
- Replaced `==` with `===` to prevent type juggling.
- Removed unused delete_value function.
- Reworked deleteDefinition() and deleteDefinitionList() functions to delete rows from the attribute_links table which are associated.
- Added deleteAttributeLinksByDefinitionId() function
Implement Cascading Delete
- Function to delete attribute links with one or more attribute definitions.
- Call function to implement an effective cascading delete.
- Refactor function naming to meet PSR-12 conventions
Fix Migration
- Add drop of Generated Column to prevent failure of migration on MySQL databases.
Fix Migration
- Removed blank lines
- Refactored function naming for PSR compliance
- Reformatted code for PSR compliance
- Added logic to drop dependent foreign key constraints before deleting an index then recreating them.
Migrate ospos_sessions table
- DROP and CREATE session table to prevent migration problems on populated databases
Fixed Bug in Migration
- In the event that item_id = null (e.g., it's a dropdown) it should not be included in the results.
Fixed bug in Dropdown deletes
- Removed delete_value function in Attributes Controller as it is unused.
- Renamed postDelete_attribute_value function for PSR-12 compliance.
- Renamed delete_value Attribute model function for PSR-12 compliance.
- Refactored out function to getAttributeIdByValue
- Replaced == with === to prevent type juggling
- Reorganized parts of model to make it easier to find CRUD functions.
Refactoring
- PSR-12 Compliance formatting changes
- Refactored several generic functions into the migration_helper.php
- First check if primary key exists before attempting to create it.
- Grouped functions together in migration_helper.php
- phpdoc commenting functions
Optimizing Indices
- There are two queries run while opening the Items view which time out on large databases with weak hardware. These indices cut the query execution in half or better.
Add Unique constraint back into attribute_links
- This migration reverts ospos_attribute_links_ibfk_1 and 2 to ON DELETE RESTRICT. Cascade delete is done programmatically. This is needed to have a unique column on the attribute_links table which prevents duplicate attributes from begin created with the same item_id-attribute_id-definition_id combination
Correct spacing after if for PSR-12
Minor code cleanup.
- Removed Comments separating sections of code in Attribute model
- Removed extra log line to prevent cluttering of the log
* Improve code style and PSR-12 compliance
- refactored code formatting to adhere to PSR-12 guidelines
- standardized coding conventions across the codebase
- added missing framework files and reverted markup changes
- reformatted arrays for enhanced readability
- updated language files for consistent styling and clarity
- minor miscellaneous improvements
