Repository: fermyon/spin (rust)

Issue:
Catch invalid key/value store at build time
Currently, Spin's SQLite key value feature only supports a single `"default"` store per application. The implementation [hard codes only supporting this store](https://github.com/fermyon/spin/blob/665d3a023d2a192b12f32dedec9d89b8fd9f9e7e/crates/trigger/src/lib.rs#L393). Access to this store is granted by the user on the component level in a `Spin.toml`, so to grant my `hello` component access to the store I do the following:
```toml
[[component]]
id = "hello"
source = "target/wasm32-wasi/release/rust_key_value.wasm"
key_value_stores = ["default"]
```

However, currently I can do the following and build and run an application without an error on `spin build --up`. Only when my app tries `Store::open("foo")` during runtime is a `NoSuchStore` error returned. Which as a user may not make sense since I added it to the component:
```
[[component]]
id = "hello"
source = "target/wasm32-wasi/release/rust_key_value.wasm"
key_value_stores = ["default", "foo"]
# OR the following
# key_value_stores = ["foo"]
```

Clear documentation on default store only may suffice, but catching during build time may be better.

Pull request:
Validate allowed key-value stores
Fixes #1232.

```
$ spin up
Error: Invalid key-value store 'hello'. This version of Spin supports only the 'default' store.

$ spin deploy
Error: Invalid key-value store 'hello'. This version of Spin supports only the 'default' store.

Learn more at https://developer.fermyon.com/cloud/faq
```

Unlike `allowed_http_hosts`, this implementation puts the validation logic in the loader, rather than putting it with the `key-value` component and adding a crate reference.  I did this solely to avoid making `loader` dependent on EVEN MOAR things.  It does risk a maintenance burden; if we ever need more sophisticated parsing and validation logic for these IDs, then it's probably worth doing it in the host component and taking the dependency.  Happy to do that now if people feel it's better.

Patch:
diff --git a/crates/loader/src/lib.rs b/crates/loader/src/lib.rs
index 0eb307f3..7ce84abf 100644
--- a/crates/loader/src/lib.rs
+++ b/crates/loader/src/lib.rs
@@ -16,6 +16,7 @@ pub mod cache;
 mod common;
 pub mod digest;
 pub mod local;
+mod validation;
 
 /// Load a Spin application configuration from a spin.toml manifest file.
 pub use local::from_file;
diff --git a/crates/loader/src/local/mod.rs b/crates/loader/src/local/mod.rs
index 73426f91..7745c551 100644
--- a/crates/loader/src/local/mod.rs
+++ b/crates/loader/src/local/mod.rs
@@ -28,7 +28,10 @@ use spin_manifest::{
 };
 use tokio::{fs::File, io::AsyncReadExt};
 
-use crate::{bindle::BindleConnectionInfo, cache::Cache, digest::bytes_sha256_string};
+use crate::{
+    bindle::BindleConnectionInfo, cache::Cache, digest::bytes_sha256_string,
+    validation::validate_key_value_stores,
+};
 use config::{
     FileComponentUrlSource, RawAppInformation, RawAppManifest, RawAppManifestAnyVersion,
     RawAppManifestAnyVersionPartial, RawComponentManifest, RawComponentManifestPartial,
@@ -126,6 +129,10 @@ pub fn validate_raw_app_manifest(raw: &RawAppManifestAnyVersion) -> Result<()> {
         .components
         .iter()
         .try_for_each(|c| validate_allowed_http_hosts(&c.wasm.allowed_http_hosts))?;
+    manifest
+        .components
+        .iter()
+        .try_for_each(|c| validate_key_value_stores(&c.wasm.key_value_stores))?;
 
     if raw
         .as_v1()
diff --git a/crates/loader/src/validation.rs b/crates/loader/src/validation.rs
new file mode 100644
index 00000000..64eba82e
--- /dev/null
+++ b/crates/loader/src/validation.rs
@@ -0,0 +1,48 @@
+use anyhow::{anyhow, Result};
+
+const ONLY_ALLOWED_KV_STORE: &str = "default";
+
+pub(crate) fn validate_key_value_stores(key_value_stores: &Option<Vec<String>>) -> Result<()> {
+    match key_value_stores
+        .iter()
+        .flatten()
+        .find(|id| *id != ONLY_ALLOWED_KV_STORE)
+    {
+        None => Ok(()),
+        Some(invalid) => {
+            let err = anyhow!("Invalid key-value store '{invalid}'. This version of Spin supports only the '{ONLY_ALLOWED_KV_STORE}' store.");
+            Err(err)
+        }
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn kv_empty_list_is_allowed() {
+        validate_key_value_stores(&None).expect("None should be valid");
+        validate_key_value_stores(&Some(vec![])).expect("Empty vector should be valid");
+    }
+
+    #[test]
+    fn default_store_is_allowed() {
+        validate_key_value_stores(&Some(vec!["default".to_owned()]))
+            .expect("Default store should be valid");
+        validate_key_value_stores(&Some(vec!["default".to_owned(), "default".to_owned()]))
+            .expect("Default store twice should be valid");
+    }
+
+    #[test]
+    fn non_default_store_is_not_allowed() {
+        validate_key_value_stores(&Some(vec!["hello".to_owned()]))
+            .expect_err("'hello' store should be invalid");
+    }
+
+    #[test]
+    fn no_sneaky_hiding_non_default_store_behind_default_one() {
+        validate_key_value_stores(&Some(vec!["default".to_owned(), "hello".to_owned()]))
+            .expect_err("'hello' store should be invalid");
+    }
+}
diff --git a/crates/trigger/src/lib.rs b/crates/trigger/src/lib.rs
index 3adf5317..e8e431f2 100644
--- a/crates/trigger/src/lib.rs
+++ b/crates/trigger/src/lib.rs
@@ -25,7 +25,7 @@ use spin_config::{
 };
 use spin_core::{Config, Engine, EngineBuilder, Instance, InstancePre, Store, StoreBuilder};
 
-const SPIN_CONFIG_ENV_PREFIX: &str = "SPIN_CONFIG";
+const SPIN_CONFIG_ENV_PREFIX: &str = "SPIN_APP";
 const DEFAULT_SQLITE_DB_DIRECTORY: &str = ".spin";
 const DEFAULT_SQLITE_DB_FILENAME: &str = "sqlite_key_value.db";
 
diff --git a/docs/content/sips/002-app-config.md b/docs/content/sips/002-app-config.md
index 265f6b89..9397f7f2 100644
--- a/docs/content/sips/002-app-config.md
+++ b/docs/content/sips/002-app-config.md
@@ -89,8 +89,8 @@ prefix = "MY_APP_"
 #### Example providers
 
 - Environment provider
-  - Configured with a prefix, e.g. `SPIN_CONFIG_`
-  - `key_one` -> `SPIN_CONFIG_KEY_ONE`
+  - Configured with a prefix, e.g. `SPIN_APP_`
+  - `key_one` -> `SPIN_APP_KEY_ONE`
 - File provider
   ```json
   {"key_one": "value-one"}
diff --git a/examples/config-rust/.env b/examples/config-rust/.env
index 85b64997..5501154e 100644
--- a/examples/config-rust/.env
+++ b/examples/config-rust/.env
@@ -1,1 +1,1 @@
-SPIN_CONFIG_DOTENV = "dotenv"
+SPIN_APP_DOTENV = "dotenv"


Test patch:
diff --git a/tests/testcases/key-value/spin.toml b/tests/testcases/key-value/spin.toml
index 764ebdaa..7891c0bc 100644
--- a/tests/testcases/key-value/spin.toml
+++ b/tests/testcases/key-value/spin.toml
@@ -7,7 +7,7 @@ version = "1.0.0"
 
 [[component]]
 id = "hello"
-key_value_stores = ["default", "foo"]
+key_value_stores = ["default"]
 source = "target/wasm32-wasi/release/key_value.wasm"
 [component.trigger]
 route = "/..."
diff --git a/tests/testcases/key-value/src/lib.rs b/tests/testcases/key-value/src/lib.rs
index d257879c..a6886e12 100644
--- a/tests/testcases/key-value/src/lib.rs
+++ b/tests/testcases/key-value/src/lib.rs
@@ -7,7 +7,8 @@ use spin_sdk::{
 
 #[http_component]
 fn handle_request(_req: Request) -> Result<Response> {
-    ensure!(matches!(Store::open("foo"), Err(Error::NoSuchStore)));
+    // TODO: once we allow users to pass non-default stores, test that opening
+    // an allowed-but-non-existent one returns Error::NoSuchStore
     ensure!(matches!(Store::open("forbidden"), Err(Error::AccessDenied)));
 
     let store = Store::open_default()?;