Skip to content

Configuration & Security Hardening (#36)#83

Open
askdba wants to merge 7 commits intomainfrom
fix/issue-36-config-security
Open

Configuration & Security Hardening (#36)#83
askdba wants to merge 7 commits intomainfrom
fix/issue-36-config-security

Conversation

@askdba
Copy link
Owner

@askdba askdba commented Mar 14, 2026

Implements issue #36:

  • Runtime config file permission checks (Unix): Refuse to load myvector_config_file if group/world readable or wrong ownership. Ownership check skipped when running as root (e.g. Docker).
  • docs/CONFIGURATION.md: Full config reference with options, defaults, validation rules, security requirements, examples.
  • Test updates: chmod 600 on config file in test-online-updates.sh and MTR tests (basic.test, distance.test).

Made with Cursor


Note

Medium Risk
Changes binlog thread startup to reject insecure myvector_config_file permissions/ownership on Unix, which can prevent online updates from starting if deployments/tests rely on looser file modes.

Overview
Hardens online-update configuration loading. The binlog thread now refuses to read myvector_config_file on Unix if it is group/world-accessible or not owned by the MySQL process user (ownership check skipped when running as root), and exits early with an error when rejected.

Documentation and tooling updates. Adds docs/CONFIGURATION.md and docs/ONLINE_INDEX_UPDATES.md, links them from README.md, records the change in CHANGELOG.md, updates MTR tests and the new scripts/test-online-updates.sh to chmod 0600 the config file, and introduces scripts/build-docker-local.sh for local image builds.

Written by Cursor Bugbot for commit dbcd0c8. This will update automatically on new commits. Configure here.

askdba added 5 commits March 14, 2026 01:35
Add docs/ONLINE_INDEX_UPDATES.md with:
- Overview of real-time index sync via MySQL binlogs
- Prerequisites: row-based binlog, REPLICATION CLIENT privileges, config file
- Step-by-step guide for online=Y and idcol in MYVECTOR options
- Complete example and troubleshooting table

Link from README Documentation section. Update CHANGELOG.

Fixes #81

Made-with: Cursor
- scripts/test-online-updates.sh: test online index updates flow
- scripts/build-docker-local.sh: build Docker image locally
- Assert DELETE removes id=2 from search results (exit 1 if found)
- Add Testing with Docker section to ONLINE_INDEX_UPDATES.md

Made-with: Cursor
- Add runtime config file permission checks (Unix): refuse to load if
  group/world readable or wrong ownership (skip ownership when running as root)
- Add docs/CONFIGURATION.md: options, defaults, validation, security
- Update ONLINE_INDEX_UPDATES.md with security note
- Add chmod 600 to test scripts and MTR tests for config file

Made-with: Cursor
}

return true;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TOCTOU race between permission check and file open

Low Severity

checkConfigFilePermissions uses stat() to verify permissions at line 434, but then readConfigFile opens the file separately via std::ifstream at line 473. Between these two operations, the file could be replaced (e.g., via symlink swap), defeating the permission check. The secure approach is to open() the file first, then fstat() on the file descriptor to check permissions atomically with respect to the opened file.

Additional Locations (1)
Fix in Cursor Fix in Web

readConfigFile now returns bool. On permission check failure, the binlog
thread exits immediately instead of retrying for ~10 minutes.

Made-with: Cursor
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

}

return true;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing config file now fatally blocks binlog thread

High Severity

checkConfigFilePermissions treats a non-existent file (stat() returning ENOENT) identically to a file with insecure permissions — both return false and cause the binlog thread to exit. The default myvector_config_file is "myvector.cnf", so any deployment where this file simply hasn't been created (common when not using online updates) will now log an error and kill the binlog thread on every plugin startup. Previously, readConfigFile would silently open the missing file via ifstream, read zero lines, and proceed with empty credentials. The fix is to distinguish ENOENT from actual permission failures and treat a missing file as a non-fatal condition.

Additional Locations (1)
Fix in Cursor Fix in Web

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