Skip to content

YAMLStorage silently aliases datasources, models, and memories whose IDs are filesystem-equivalent #249

Description

@mattfaltyn

Summary

YAMLStorage uses raw logical identifiers as filenames for datasources, models,
and memories. On case-insensitive filesystems, distinct IDs such as X and x
address the same file, so the later write silently replaces the earlier entity.

This reproduces on the default macOS filesystem. SQLiteStorage preserves the
same identifiers separately.

Impact

YAMLStorage is the default backend, so this can affect standard macOS and Windows
installations:

  • datasource X can silently become datasource x, including its connection
    configuration;
  • model X can silently become model x, potentially directing subsequent
    queries to the wrong physical table;
  • memory X is overwritten by memory x;
  • legacy memory migration can collapse the records and then delete
    memories.yaml;
  • ingestion of quoted, case-sensitive database tables can overwrite one
    generated model with another.

Reproduction

import asyncio
import tempfile

from slayer.core.models import DatasourceConfig, SlayerModel
from slayer.storage.yaml_storage import YAMLStorage


async def main():
    tmp = tempfile.TemporaryDirectory()
    storage = YAMLStorage(tmp.name)

    await storage.save_datasource(
        DatasourceConfig(name="X", type="postgres", host="upper")
    )
    await storage.save_datasource(
        DatasourceConfig(name="x", type="postgres", host="lower")
    )
    await storage.save_model(
        SlayerModel(name="X", data_source="db", sql_table="upper")
    )
    await storage.save_model(
        SlayerModel(name="x", data_source="db", sql_table="lower")
    )

    upper_ds = await storage.get_datasource("X")
    lower_ds = await storage.get_datasource("x")
    upper_model = await storage.get_model("X", data_source="db")
    lower_model = await storage.get_model("x", data_source="db")

    print({
        "datasources": await storage.list_datasources(),
        "X_host": upper_ds.host,
        "x_host": lower_ds.host,
        "models": await storage.list_models("db"),
        "X_table": upper_model.sql_table,
        "x_table": lower_model.sql_table,
    })


asyncio.run(main())

Actual output on macOS:

{'datasources': ['X'], 'X_host': 'lower', 'x_host': 'lower',
 'models': ['X'], 'X_table': 'lower', 'x_table': 'lower'}

Running the same operations through SQLiteStorage produces the expected
result:

{'datasources': ['X', 'x'], 'X_host': 'upper', 'x_host': 'lower',
 'models': ['X', 'x'], 'X_table': 'upper', 'x_table': 'lower'}

The existing YAML memory contract also fails for the same reason:

poetry run pytest \
  'tests/test_memory_string_ids.py::TestUserSuppliedId::test_case_sensitive_ids[yaml]' \
  -q

get_memory("X") returns the value saved under x.

Root cause

These paths directly interpolate logical identity into a filesystem component:

  • YAMLStorage._model_path / _save_model_impl
  • YAMLStorage.save_datasource / get_datasource
  • YAMLStorage._memory_md_path
  • migrate_memories_layout

The public validators preserve case, and SQLite's text keys treat the values as
separate identities, but the YAML backend delegates identity semantics to the
host filesystem.

Suggested direction

Use one shared, reversible, filesystem-independent filename encoding for every
YAML entity type. Migrate existing plain filenames crash-safely, retain backward
compatibility during the transition, and verify that all logical IDs map
uniquely before deleting legacy storage.

A focused regression test should save X and x for datasource, model, and
memory identities against both YAML and SQLite backends and assert equivalent
behavior.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions