Skip to content

Fix unbound variable error in column existence checks#39

Open
burakkurkcu wants to merge 1 commit intowebcoyote:mainfrom
burakkurkcu:main
Open

Fix unbound variable error in column existence checks#39
burakkurkcu wants to merge 1 commit intowebcoyote:mainfrom
burakkurkcu:main

Conversation

@burakkurkcu
Copy link
Copy Markdown
Contributor

Summary

Fixes an unbound variable error that could occur during database migration when the column-existence checks in init_db received non-numeric input.

Problem

The migration logic for adding ram_mb, base_name, and ssh_user columns used:

has_base_name=$(sqlite3 "$DB_FILE" "PRAGMA table_info(instances);" | grep -c 'base_name') || true
if [[ "$has_base_name" -eq 0 ]]; then
    ...
fi

The -eq operator evaluates both sides in arithmetic context, which treats bare words as variable names. If the captured value ever contained something non-numeric (e.g. a path like /Users/... leaking into the pipeline), bash would parse Users as a variable reference and — with set -u active — fail with:

db.sh: line 69: Users: unbound variable

The || true fallback was also masking genuine sqlite3 failures, letting migrations silently no-op.

Fix

  • Introduced a column_exists helper that relies on grep -q's exit status, avoiding arithmetic context entirely.
  • Anchored the grep pattern as |column| to match against pipe-delimited PRAGMA table_info output and prevent false matches on column name prefixes (e.g. ram_mb matching ram_mb_limit).
  • Replaced all three has_* checks with if ! column_exists instances <col>.
  • Removed the || true fallbacks so real sqlite3 errors surface instead of being swallowed.

Testing

  • Fresh init on a new $DB_FILE — all three columns created as expected.
  • Re-running against an already-migrated DB — no-ops cleanly, no errors.
  • Running against a pre-migration DB missing all three columns — migrations apply correctly.
  • Verified set -u no longer trips on the migration path.

Risk

Low. Pure refactor of internal migration logic; no schema or API changes. The new helper is private to db.sh.

@webcoyote
Copy link
Copy Markdown
Owner

Hi, and thanks for creating a PR for this issue. Can you help me understand the problem? I'm not sure how user data would infect the grep operation because the PRAGMA command is not performing a select on the table data, it's getting back a list of the column names, and grep then counts.

❯ sqlite3 "$DB_FILE" "PRAGMA table_info(instances);"
0|name|TEXT|0||1
1|vm_name|TEXT|1||0
2|ram_mb|INTEGER|0||0
3|created_at|TEXT|0|datetime('now')|0
4|base_name|TEXT|0||0
5|ssh_user|TEXT|0||0

Can you help me repro this error in db.sh line 69?

@burakkurkcu
Copy link
Copy Markdown
Contributor Author

burakkurkcu commented Apr 21, 2026

I remember I was at v1.0.11 version then pulled v1.0.18 it was silent exiting at time, then reverted to v1.0.11 again for time. and then seeing @echthesia addressing bug I have I pulled v1.0.21 and it was fine. then I pulled today latest commit with merge tart message, I started getting error below.

To be honest I didn't get it as it seems just a thing from migration side code which should just pass by because I already migrated before. so I threw db.sh to claude to try a quick fix to get going at first. and this is it. after error gone these logs occured.

image

when I have some more time I can try same repro path by reverting to .11 create vm and everything, and then to latest commit in today to see if it can repro.

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.

2 participants