fix(installer): harden config-load against malformed config.yaml

Per augment review on #2348: loadExistingConfig stored any truthy
yaml.parse result (including scalars like '42'), which would later crash
_hoistCoreKeysFromLegacyModuleConfigs at \`key in cfg\` with
"Cannot use 'in' operator to search for ... in 42".

- loadExistingConfig: only keep parses that are plain objects (not
  scalars or arrays). A corrupt config.yaml is now treated the same as
  a parse error — skipped, not crashed-on.
- _hoistCoreKeysFromLegacyModuleConfigs: belt-and-suspenders type guards
  on _existingConfig.core (in case it's populated by some other path)
  and on each module cfg in the loop.
- Test Suite 43 adds 2 assertions covering a scalar core/config.yaml:
  loadExistingConfig must not crash, and bmm.project_name must still
  hoist into a clean core bucket.
This commit is contained in:
Brian Madison 2026-04-27 23:28:41 -05:00
parent b3f337c147
commit 93f6547554
2 changed files with 42 additions and 3 deletions

View File

@ -2943,8 +2943,38 @@ async function runTests() {
'hoist still strips the duplicate from bmm so writeCentralConfig partition stays clean', 'hoist still strips the duplicate from bmm so writeCentralConfig partition stays clean',
); );
// Malformed config.yaml (parses to a scalar) must not crash loadExistingConfig
// or the hoist pass — they should treat it as "no config for that module"
// and continue. Regression for augment review on PR #2348.
const fixtureRoot43c = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-fixture-43c-'));
const bmadDir43c = path.join(fixtureRoot43c, '_bmad');
await fs.ensureDir(path.join(bmadDir43c, '_config'));
await fs.writeFile(path.join(bmadDir43c, '_config', 'manifest.yaml'), 'modules: []\n', 'utf8');
await fs.ensureDir(path.join(bmadDir43c, 'core'));
await fs.ensureDir(path.join(bmadDir43c, 'bmm'));
// Scalar YAML — yaml.parse returns the literal 42 (truthy non-object).
// Pre-fix this crashed _hoistCoreKeysFromLegacyModuleConfigs with
// "Cannot use 'in' operator to search for 'project_name' in 42".
await fs.writeFile(path.join(bmadDir43c, 'core', 'config.yaml'), '42\n', 'utf8');
await fs.writeFile(path.join(bmadDir43c, 'bmm', 'config.yaml'), 'project_name: rescued\n', 'utf8');
const officialModules43c = new OfficialModules();
let crashErr;
try {
await officialModules43c.loadExistingConfig(fixtureRoot43c);
} catch (error) {
crashErr = error;
}
assert(!crashErr, 'loadExistingConfig does not crash on a scalar core/config.yaml', crashErr?.stack);
assert(
officialModules43c.existingConfig.core?.project_name === 'rescued',
'scalar core gets replaced with {} and bmm.project_name still hoists in',
);
await fs.remove(fixtureRoot43).catch(() => {}); await fs.remove(fixtureRoot43).catch(() => {});
await fs.remove(fixtureRoot43b).catch(() => {}); await fs.remove(fixtureRoot43b).catch(() => {});
await fs.remove(fixtureRoot43c).catch(() => {});
} catch (error) { } catch (error) {
console.log(`${colors.red}Test Suite 43 setup failed: ${error.message}${colors.reset}`); console.log(`${colors.red}Test Suite 43 setup failed: ${error.message}${colors.reset}`);
console.log(error.stack); console.log(error.stack);

View File

@ -903,7 +903,10 @@ class OfficialModules {
try { try {
const content = await fs.readFile(moduleConfigPath, 'utf8'); const content = await fs.readFile(moduleConfigPath, 'utf8');
const moduleConfig = yaml.parse(content); const moduleConfig = yaml.parse(content);
if (moduleConfig) { // Only keep plain object parses. A corrupt config.yaml that parses
// to a scalar or array would crash later code that does `key in cfg`
// / `Object.keys(cfg)`; treat it the same as a parse error.
if (moduleConfig && typeof moduleConfig === 'object' && !Array.isArray(moduleConfig)) {
this._existingConfig[entry.name] = moduleConfig; this._existingConfig[entry.name] = moduleConfig;
foundAny = true; foundAny = true;
} }
@ -947,9 +950,15 @@ class OfficialModules {
); );
if (coreKeys.size === 0) return; if (coreKeys.size === 0) return;
this._existingConfig.core = this._existingConfig.core || {}; // Belt-and-suspenders: loadExistingConfig already filters non-object parses,
// but anyone calling _hoistCoreKeysFromLegacyModuleConfigs in isolation (or
// future code paths populating _existingConfig directly) shouldn't be able
// to crash this with a scalar / array.
const existingCore = this._existingConfig.core;
this._existingConfig.core = existingCore && typeof existingCore === 'object' && !Array.isArray(existingCore) ? existingCore : {};
for (const [moduleName, cfg] of Object.entries(this._existingConfig)) { for (const [moduleName, cfg] of Object.entries(this._existingConfig)) {
if (moduleName === 'core' || !cfg || typeof cfg !== 'object') continue; if (moduleName === 'core' || !cfg || typeof cfg !== 'object' || Array.isArray(cfg)) continue;
for (const key of Object.keys(cfg)) { for (const key of Object.keys(cfg)) {
if (!coreKeys.has(key)) continue; if (!coreKeys.has(key)) continue;
if (!(key in this._existingConfig.core)) { if (!(key in this._existingConfig.core)) {