Add `pull_request` trigger to `php-linter.yml` so the `phplint` check runs on pull requests from forks. Previously, it only listened to `on: push`, causing the `phplint` required status check on `master` to stay permanently pending on fork PRs.
Resolves objecttothis's concern in #4726: the version was set to 3.4.3 before
the release was tagged, causing vulnerability reports to reference a non-existent
release. Bump to 3.4.4-dev so master correctly references the development
version.
* fix(release): handle -dev snapshot suffix in release workflows
- release.yml: strip -dev from App.php version when determining release target
- release.yml: append -dev to the bumped next version (App.php, package.json,
package-lock.json all get the snapshot suffix)
- build-release.yml: strip -dev from version for tag builds so tag validation
and archive naming use the clean release version
* feat(install): add one-line Ubuntu installer with CI test and docs
- Add scripts/install-ubuntu.sh: installs Apache/MariaDB/PHP, downloads the
latest stable release, configures the DB + Apache, and optionally sets up
Let's Encrypt SSL (interactive or via env vars).
- Add .github/workflows/install-script-test.yml: runs the installer in a fresh
Ubuntu container and sanity-checks the result.
- Document the installer in INSTALL.md.
- Harden the script: identifier-safe DB_NAME, quote/pipe rejection in
passwords, correct 'Candidate: (none)' PPA detection, keep unix_socket root
auth when no root password is set, select the .zip release asset, and use the
SSL domain as the public hostname for allowedHostnames.
- Normalize all language files to single quotes with aligned => operators;
no translation text or logic changes
- Add Georgian (ka) scaffolding across ~18 modules with placeholder
translations, matching project style conventions
- Add LanguageFilesTest to validate every PHP file under app/Language
parses without syntax errors; new files auto-discovered via glob
Add MySQL 8.4 compatibility, including a utf8mb4 conversion migration,
hardened foreign key handling, and expanded CI database coverage.
Migrations:
- Add Mysql84Utf8mb4Conversion migration using
sqlscripts/3.4.3_mysql84_utf8mb4_conversion.sql via executeScript()
- Drop ospos_people composite index before conversion, since its
prefix length exceeds the 3072-byte key limit under utf8mb4
- Add assertNoCollationCollisions() pre-flight check to fail fast on
unique values that collide under utf8mb4_unicode_520_ci, instead of
hitting "Duplicate entry" partway through ALTER TABLE
- Wrap the conversion in try/catch so foreign keys are recreated on
failure; log restore errors separately so they don't mask the
original exception
- Throw RuntimeException if re-adding the people index fails
- Remove the re-add of the sales_items_taxes FK in down(), which
MySQL 8.4 rejects without a unique key
- Revert edits to historical migrations; fix FK handling and table
names
migration_helper:
- Add shared dropAllForeignKeyConstraints() and
recreateForeignKeyConstraints(); table/column now optional, dropping
all FKs in the schema when omitted
- Match CONSTRAINT_SCHEMA when joining KEY_COLUMN_USAGE to
REFERENTIAL_CONSTRAINTS to avoid cross-schema name collisions
- Rebuild FK lookup with the query builder and bound values instead
of interpolated SQL
CI/tests:
- Add mysql:5.7, mariadb:10.6 and mariadb:10.11 to PHPUnit matrix
- Use master instead of latest image tag; improve CI robustness
- Mock table() in FailedQueryConnectionTrait so query builder chains
work against the failing connection
Signed-off-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Co-authored-by: aliworksx08 <57456290+aliworksx08@users.noreply.github.com>
Co-authored-by: jekkos <jeroen.peelaerts@gmail.com>
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* docs: move BUILD.md, INSTALL.md, UPGRADE.md into docs/
Closes#4751
- Move the three detailed guides from the repo root into docs/
- Update all links in README.md, .github/ISSUE_TEMPLATE.md,
docker-compose.yml, and cross-references within the moved guides
- Add docs/**/* to the gulp compress task so release archives
include the guides under their new docs/ path
- README now links to all three guides
- Root-level files (README, CHANGELOG, LICENSE, SECURITY.md,
CODE_OF_CONDUCT.md, AGENTS.md) remain in place
* docs: add working-dir note and remove stale 'docs' .gitignore entry
- Add explicit note that Docker/install commands run from repo root
- Remove stale 'docs' line from .gitignore (leftover from old ApiGen
output) so tracked files under docs/ can be modified normally
- Scope clipboard.js and bootstrap-tagsinput to configs/manage.php instead of global bundles
- Remove obsolete public/js/clipboard.min.js vendor asset and manual view script inclusion
- Add toast notifications ($.notify) and localized strings for copying system info
- Add localization keys to all Config language files
Also clarify in build.ps1, build-steps.ps1 and build-package.ps1 that the
'.\tools\Build\...' example is a repository-root command and that callers in
other directories must provide a valid path to the scripts
Remove the one-off CI3->CI4 migration tooling that is no longer needed:
tools/ci4-lang-migration/ and tools/ci4-upgrade-language-interpolation/.
Docker is kept at the repository root rather than under tools/, per
review feedback. The Windows build scripts remain in tools/build/.
Root-relative docker paths are restored in .gitignore, BUILD.md,
INSTALL.md, gulpfile.js, and the build-release workflow.
---------
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* docs: rewrite UPGRADE.md for current CI4 install model (#4748)
The old guide described the pre-3.0 CodeIgniter 3 flow (application/config/*.php,
manual SQL upgrade scripts) and was flagged as outdated in #4748. Rewrite it to
match the current model: config lives in the root .env, the schema auto-migrates
on first login, and uploads live in public/uploads.
Answer the reporter's direct questions: yes you can overwrite in place, and the
files to safeguard are the database, .env (DB creds + encryption.key) and
public/uploads. Warn that releases ship a fresh default .env, and note the
3.4.2+ production requirement for app.allowedHostnames.
* docs(upgrade): clarify rollback DB restore and avoid public display_errors
Address CodeRabbit review on #4750:
- Rolling back: state that, since migrations run on first login, the older
release may need the pre-upgrade database dump restored as well.
- Troubleshooting: stop recommending public display_errors on production;
point operators to web/PHP error logs and logger.threshold instead.
* 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>
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
* fix(models): handle query() returning false in result-consuming methods
As of CI 4.1.2, Connection::query() returns false when a query fails
(previously an empty result object). Guard the model/helper methods that
consume the result so a failed query degrades to an empty result instead
of throwing "Call to member function getResultArray() on false".
- Tax::get_taxes()
- Sale::get_all_suspended()
- Reports\Inventory_low::getData()
- migration_helper: dropAllForeignKeyConstraints(), indexExists(),
primaryKeyExists()
Adds QueryFalseGuardTest forcing query() to return false. Closes#3634.
* test(query-false): mock concrete driver instead of abstract BaseConnection
onlyMethods() does not stub the 16 abstract methods on BaseConnection, so
mocking it produced an invalid double. Mock the concrete driver class
(resolved from the live test connection) instead.
* fix(3634): fail loudly when migration metadata queries return false
Model read methods degrade to an empty result on a failed query, but the
migration metadata helpers (indexExists, primaryKeyExists,
dropAllForeignKeyConstraints) must not mistake a failed query for 'not
found' and proceed with schema changes. They now throw a DatabaseException
so the migration aborts loudly.
- Add an optional injectable BaseConnection param for testability.
- Add missing docstrings (docstring-coverage check).
- Add regression tests asserting the helpers throw on a failed query.
* fix(3634): use supplied connection for FK drop; stub error() in tests
Address CodeRabbit review: dropAllForeignKeyConstraints now builds its
Forge from the supplied connection (consistent with the lookup) instead of
always the default one. Test mock also stubs error() to avoid a warning on
the constructor-disabled connection.
* fix(3634): revert FK drop to Database::forge() (driver-specific forge)
new Forge($db) creates the base Forge which does not implement
dropForeignKey for the driver ('This feature is not available'), breaking
migrations. Restore Database::forge(), which loads the driver-specific
Forge via the factory. The optional connection param is retained solely
for testing the fail-loud behavior on the lookup path.
* test(3634): add regression test for Inventory_low::getData query-failure guard
Covers the low-stock report's degrade-to-empty behavior on a failed query,
addressing the CodeRabbit note that it lacked a focused test.
* test(3634): adopt test{Method}_{Purpose} naming convention
Align QueryFalseGuardTest method names with the convention from #4730
(testGetTaxes_ReturnsEmptyArrayWhenQueryFails, etc.), per review.
* test: split QueryFalseGuardTest into per-component test files
Apply CodeRabbit review: extract the shared failing-connection helpers
into tests/Support/FailedQueryConnectionTrait.php and move each test to
its component's test file (TaxTest, SaleTest, Inventory_lowTest,
migration_helperTest). Use fresh model instances instead of the shared
model() singleton so the injected failing connection does not leak into
other tests.
* style(tests): remove redundant docblocks from query-failure regression tests
---------
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
* fix(cashups,items): sync date globals from picker on Apply
Address CodeRabbit review on #4731: when a page loads with restored GET
dates the globals are overwritten after the picker initializes, so applying
an unchanged range (which skips the picker change callback) refreshed the
table with the stale globals instead of the visible range. Add a shared
apply.daterangepicker handler in the daterangepicker partial that copies the
picker range into start_date/end_date; it binds before page-level handlers
so refreshes and URL updates use the visible range.
* style(views): remove redundant comments in daterangepicker
* 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>
* 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>
* 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>
* fix(tabs): resolve inner tab active switching, adjust padding, and correct tab title showing 'system_conf' instead of 'system_info'
* fix(views): enhance config forms with multi-breakpoint responsive grids for better readability and usability on smaller screens
* refactor(shortcuts): remove redundant container wrappers and optimize grid column widths for proper spacing
* refactor(system-info): replace min-height hacks with form-horizontal rows, adopt utility classes, and fix timezone check
* style: replace hardcoded red with 'text-danger' on required labels for proper Bootswatch theme integration
* chore: update dev dependency
Name test methods test{MethodUnderTest}_{Behavior} (e.g.
testPostSave_RejectsNegativeTaxPercent) so they read as a
self-documenting sentence and need no docblock.
* fix(auth): return 401 JSON for AJAX requests on session timeout (#4696)
IsLoggedIn previously threw RedirectException('login') for every request
type. For XHR calls the browser transparently followed the 302 and fetched
the login page's HTML where JSON was expected, so the table silently
rendered empty and the failure looked like a broken sort.
- Server: IsLoggedIn::before() now returns a JSON 401 response when the
request is an AJAX call, keeping the redirect behavior for normal
navigations.
- Client: a global $(document).ajaxError handler in the authenticated
layout redirects to the login URL on a 401 response.
* fix(auth): run isLoggedIn before csrf for expired sessions (#4696)
An expired-session AJAX POST with an invalid CSRF token was hitting the
globally-configured csrf filter first, which throws a CSRF exception before
isLoggedIn can return its 401. Reorder the global before-filters so the
authentication check runs before CSRF validation; CSRF protection is fully
retained for authenticated requests.
---------
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Restore the daterangepicker apply and filter change handlers so
/expenses/search receives the selected start_date and end_date
instead of today's date for both. Also removes leftover comments
from the expenses manage view.
Fixes#4596
---------
Co-authored-by: jekkos <jeroen.peelaerts@gmail.com>
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>
* fix(security): report unwritable .env.lock, make throttle limits configurable
envFileIsWritable() previously checked is_writable(.env) (the file), which passes in Docker even when the mutex file is root-owned by a prior env:provision run. It now checks the real write path: the directory (to create .env.tmp.* + .env.lock) and any existing .env.lock must be writable, so the app throws the clear 'run env:provision' error instead of crashing on 'Unable to open .env.lock'.
The Throttle filter now reads throttle.capacity / throttle.seconds from env (default 5 per 60s); capacity <= 0 disables throttling, so operators serving sequential HTTP clients (e.g. Zabbix) are not caught by the lockout.
* fix(security): address CodeRabbit review findings
- Throttle: validate throttle.capacity as an integer before treating a non-positive value as 'disabled', so a non-numeric value (e.g. 'five') falls back to the default instead of silently bypassing the lockout. Apply the same validation to throttle.seconds.
- envFileIsWritable(): also reject an existing non-writable .env on Windows (where rename() cannot replace a read-only destination); keep the check Windows-only since POSIX rename() replaces a read-only dest when the directory is writable.
- Tests: restore the prior throttle.capacity env state in ThrottleTest (capture/restore instead of delete); add an invalid-capacity fallback case; skip the not-writable fixtures when running as root (where is_writable() is bypassed).
* fix(migration): guard ConvertToCI4 key-write branches with envFileIsWritable()
The migration's 'no key' and 'CI3 key' branches called rotateEncryptionKey()/rotateEncryptionKeyTransaction() directly, bypassing the envFileIsWritable() guard that checkEncryption() uses. On a fresh Docker/Compose install where the web runtime cannot write /app/.env.lock, this produced a raw 'fopen(/app/.env.lock): Permission denied' error instead of the actionable 'run php spark env:provision' message.
A valid CI4 key now short-circuits to checkEncryption() (no write); every write branch is gated on envFileIsWritable() first.
* fix: read provisioned encryption.key so config sees it
env:provision persists the key as 'encryption.key' in .env (matching the
throttle path and .env.example), but Config\Encryption only read the
ENCRYPTION_KEY env var. On a fresh Docker instance the provisioned key was
invisible to config('Encryption')->key, so the app believed no key existed
and tried to write one -- hitting the .env.lock permission wall.
Read encryption.key (via $_SERVER/$_ENV/getenv) first, then fall back to
ENCRYPTION_KEY for Docker '-e' usage. Mirrors checkThrottleEncryption().
* fix: cascade encryption.key lookup past empty-string sources
* refactor: extract Encryption::resolveKey() and test it without global env mutation
* fix(security): decode fallback-selected encryption key like BaseConfig
A key picked in the constructor fallback (notably ENCRYPTION_KEY, which
BaseConfig never inspects) was assigned verbatim, bypassing the
hex2bin:/base64: decode the parent applies to `encryption.key`. Route the
selected key through a parseKey() helper mirroring BaseConfig's
parseEncryptionKey() so prefixed values decrypt consistently, and add a
pure regression test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* chore: remove advisory ID from comments and tighten verbose comments
Per review: treat GHSA advisory IDs like secrets (drop from code) and
replace the multi-paragraph comments with concise one-liners that keep
the non-obvious "why". No logic changes.
---------
Co-authored-by: objecttothis <17935339+objecttothis@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per the project's policy of treating security advisory IDs as
secret-like, remove the identifiers embedded in source/test comments
and CHANGELOG entries. Each keeps its human-readable description (and
PR number where present), so traceability is preserved. No logic changes.
* fix(release): keep package-lock.json version in sync on bump
The release bump step updated app/Config/App.php and package.json but
left package-lock.json on the old version, so the lockfile drifted out
of sync with package.json on every release. Bump the @opensourcepos/
opensourcepos package version in package-lock.json (top-level and
packages."") using the same scoped approach, and stage it in the bump
commit.
Fixes#4717
* fix(release): sync package-lock.json to 3.4.3 on master
Bump the @opensourcepos/opensourcepos version in package-lock.json
(top-level + packages."") from 3.4.2 to 3.4.3 to match package.json,
which was already bumped during the 3.4.3 dev bump. This repairs the
current drift introduced by the bump commit so the upcoming 3.4.3
release ships with package.json and package-lock.json in sync.
Part of #4717
* chore: unified release workflow (git-cliff changelog + tag + optional bump)
- cliff.toml: git-cliff config (commit.author.name, Weblate/version-bump excluded)
- release.yml: replaces the 'Release Version Bump' workflow with a single
workflow that cuts the current release (changelog + tag + draft release)
and optionally bumps App.php to the next dev version
- build-release.yml: add official-release job (draft GitHub Release with
changelog + assets, triggered by tag push)
Supersedes the 'changelog only' scope: this now also handles tagging,
draft release, and the version bump in one place.
* fix(release): fail fast when a tag does not match the App.php version
Catches a manually-pushed mismatched tag before building, so a draft
release is never created without its archive.
* fix(release): move env block to step level (was inside run shell script)
The env: key was dedented into the run: | literal block, so the shell
would try to execute 'env:' as a command and abort the build.
Swap the swapped number/required validation strings in da, es-MX, ta,
en-GB, and tl to match the mapping used by other *_number/*_required
pairs and the receipt_config.php jQuery Validate wiring. Also clear
residual English autoreturn messages from non-English locales.
---------
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Rayan Abdul Cader <minutechreview@users.noreply.github.com>
Closes#4695
Master and PR builds were tagging every Docker image with the App.php
version (e.g. 3.4.2-master-<sha>), flooding Docker Hub with tags for
versions that were never released.
Docker tags are now scoped to the ref:
- master → master, <sha>
- branch → <branch>-<sha>
- semver tag → <version>, latest
Additional hardening:
- Release tag trigger restricted to three-component semver (N.N.N) so
non-semver tags (e.g. 3.preview) no longer publish a `latest` image
- Branch names sanitized: chars outside [a-zA-Z0-9_.-] replaced with _,
total tag truncated to stay within Docker's 128-char limit, leading
`.` or `-` prevented
- Fixed README.md claim that master builds push a `latest` tag
github-actions[bot]