fix(runtime): require complete braced environment references

apps/runtime/src/config.c

@@ -31,8 +31,6 @@
  * against any of them reads this one and nothing of ours. */
 #define PORT_ALIAS "PORT"
 
-/* Only a sigil followed by RUNTIME_PREFIX opens a reference, which is what leaves a
- * secret's own '$' alone — see the format contract in config.h. */
 #define REFERENCE_SIGIL '$'
 #define REFERENCE_OPEN '{'
 #define REFERENCE_CLOSE '}'
@@ -316,50 +314,24 @@ struct reference {
   const char *after;
 };
 
-static bool at_reference(const char *text) {
-  if (*text != REFERENCE_SIGIL) {
-    return false;
-  }
-  const char *name = text + 1;
-  if (*name == REFERENCE_OPEN) {
-    name++;
-  }
-  return starts_with(name, RUNTIME_PREFIX);
-}
-
-static const char *find_reference(const char *value) {
+static const char *find_reference(const char *value, struct reference *out) {
   for (const char *cursor = strchr(value, REFERENCE_SIGIL); cursor != NULL;
        cursor = strchr(cursor + 1, REFERENCE_SIGIL)) {
-    if (at_reference(cursor)) {
-      return cursor;
+    if (cursor[1] != REFERENCE_OPEN || !starts_with(cursor + 2, RUNTIME_PREFIX)) {
+      continue;
     }
-  }
-  return NULL;
-}
-
-/* `text` starts at a sigil at_reference has already accepted, so the only way this
- * fails is a braced form nobody closed — a typo rather than a value meant literally. */
-static bool read_reference(const char *text, struct reference *out) {
-  const char *cursor = text + 1;
-  bool braced = *cursor == REFERENCE_OPEN;
-  if (braced) {
-    cursor++;
-  }
-  out->name = cursor;
-  while (is_name_character(*cursor)) {
-    cursor++;
-  }
-  out->length = (size_t)(cursor - out->name);
-  if (braced) {
-    if (*cursor != REFERENCE_CLOSE) {
-      log_line("a tenant variable names %.*s with no closing '%c'", (int)out->length, out->name,
-               REFERENCE_CLOSE);
-      return false;
+    const char *name = cursor + 2;
+    const char *end = name;
+    while (is_name_character(*end)) {
+      end++;
+    }
+    if (*end != REFERENCE_CLOSE) {
+      continue;
     }
-    cursor++;
+    *out = (struct reference){.name = name, .length = (size_t)(end - name), .after = end + 1};
+    return cursor;
   }
-  out->after = cursor;
-  return true;
+  return NULL;
 }
 
 static bool names_key(const struct reference *reference, const char *key) {
@@ -430,25 +402,19 @@ static void append(struct arena *arena, const char *text, size_t length) {
  * which is almost every one of them. */
 static bool expand_entry(const struct instance_config *config, struct arena *arena, char **entry) {
   const char *value = strchr(*entry, '=') + 1;
-  if (find_reference(value) == NULL) {
+  struct reference reference;
+  const char *found = find_reference(value, &reference);
+  if (found == NULL) {
     return true;
   }
 
   char *expansion = arena->cursor;
   append(arena, *entry, (size_t)(value - *entry));
 
-  for (const char *remaining = value;;) {
-    const char *found = find_reference(remaining);
-    if (found == NULL) {
-      append(arena, remaining, strlen(remaining));
-      break;
-    }
+  const char *remaining = value;
+  while (found != NULL) {
     append(arena, remaining, (size_t)(found - remaining));
 
-    struct reference reference;
-    if (!read_reference(found, &reference)) {
-      return false;
-    }
     /* uint32_t's whole range rather than the port's: anything narrower is a
      * truncation the compiler is right to refuse. */
     char rendered[sizeof("4294967295")];
@@ -465,8 +431,9 @@ static bool expand_entry(const struct instance_config *config, struct arena *are
     }
     append(arena, substitution, strlen(substitution));
     remaining = reference.after;
+    found = find_reference(remaining, &reference);
   }
-  append(arena, "", 1);
+  append(arena, remaining, strlen(remaining) + 1);
 
   if (arena->overflowed) {
     log_line("instance.env expands to more than %d bytes", CONFIG_MAX_EXPANDED_BYTES);

apps/runtime/src/config.h

@@ -15,13 +15,11 @@
  * called NIBRUN_HTTP_PORT arrives as ENV_NIBRUN_HTTP_PORT and is read as the
  * tenant's, which is what lets config_build_environment drop it on its own terms.
  *
- * A tenant value may name a runtime one it is handed: `$NIBRUN_HTTP_PORT` and
- * `${NIBRUN_HTTP_PORT}` both expand, and a name this runtime does not offer fails the
- * boot rather than reaching the tenant as itself. Nothing else expands, so a secret
- * holding `$`, `$$` or `$HOME` arrives byte for byte — the prefix is what keeps the
- * substitution off values it was never meant for. The cost is that a value holding a
- * literal `$NIBRUN_` has no representation, which is the bargain the format already
- * makes for one holding a newline.
+ * Only complete `${NIBRUN_NAME}` references expand in tenant values. A complete
+ * reference to a name this runtime does not offer fails the boot. Bare names,
+ * unmatched braces and every other `$` remain literal, so secrets survive byte for
+ * byte unless they contain a complete runtime reference. There is no escape syntax
+ * for a literal complete runtime reference.
  *
  * The runtime carries no defaults for any of it. DEFAULT_RESTART_POLICY in
  * packages/protocol is the only place those values exist; the agent resolves them

apps/runtime/tests/test-config.c

@@ -143,7 +143,7 @@ static void expands_the_public_address(void) {
   EXPECT(parse(&config, REQUIRED "NIBRUN_PUBLIC_IPV4=203.0.113.7\n"
                                  "NIBRUN_EXTRA_PUBLIC_PORT=22003\n"
                                  "ENV_ANNOUNCED=${NIBRUN_PUBLIC_IPV4}:${NIBRUN_EXTRA_PUBLIC_PORT}\n"
-                                 "ENV_RTC_PORT=$NIBRUN_EXTRA_PUBLIC_PORT\n"));
+                                 "ENV_RTC_PORT=${NIBRUN_EXTRA_PUBLIC_PORT}\n"));
   char *const *environment = config_build_environment(&config);
   EXPECT(strcmp(value_of(environment, "ANNOUNCED"), "203.0.113.7:22003") == 0);
   EXPECT(strcmp(value_of(environment, "RTC_PORT"), "22003") == 0);
@@ -168,18 +168,17 @@ static void expands_a_runtime_reference(void) {
   EXPECT(parse(&config, REQUIRED "NIBRUN_HOSTNAME=my-app.nibrun.app\n"
                                  "ENV_BARE=$NIBRUN_HTTP_PORT\n"
                                  "ENV_BRACED=${NIBRUN_HTTP_PORT}\n"
-                                 "ENV_WITHIN=http://$NIBRUN_HOSTNAME:${NIBRUN_HTTP_PORT}/health\n"
+                                 "ENV_WITHIN=http://${NIBRUN_HOSTNAME}:${NIBRUN_HTTP_PORT}/health\n"
                                  "ENV_UNDER_THE_VOLUME=${NIBRUN_DATA_DIR}/state.db\n"
-                                 "ENV_TWICE=$NIBRUN_HTTP_PORT-$NIBRUN_HTTP_PORT\n"
+                                 "ENV_TWICE=${NIBRUN_HTTP_PORT}-${NIBRUN_HTTP_PORT}\n"
                                  "ENV_ADJACENT=${NIBRUN_HTTP_PORT}0\n"));
 
   char *const *environment = config_build_environment(&config);
-  EXPECT(strcmp(value_of(environment, "BARE"), "8080") == 0);
+  EXPECT(strcmp(value_of(environment, "BARE"), "$NIBRUN_HTTP_PORT") == 0);
   EXPECT(strcmp(value_of(environment, "BRACED"), "8080") == 0);
   EXPECT(strcmp(value_of(environment, "WITHIN"), "http://my-app.nibrun.app:8080/health") == 0);
   EXPECT(strcmp(value_of(environment, "UNDER_THE_VOLUME"), "/app/data/state.db") == 0);
   EXPECT(strcmp(value_of(environment, "TWICE"), "8080-8080") == 0);
-  /* Braces are the whole reason there are two forms: without them this names PORT0. */
   EXPECT(strcmp(value_of(environment, "ADJACENT"), "80800") == 0);
 }
 
@@ -199,19 +198,42 @@ static void leaves_every_other_dollar_alone(void) {
   EXPECT(strcmp(value_of(environment, "TRAILING"), "the cost is $") == 0);
 }
 
+static void preserves_incomplete_references(void) {
+  const char *literals[] = {
+      "$", "{", "}", "${", "${}", "$NIBRUN_HTTP_PORT", "$NIBRUN_NOTHING",
+      "$NIBRUN_PUBLIC_IPV4", "$NIBRUN_EXTRA_PUBLIC_PORT", "${NIBRUN_HTTP_PORT",
+      "${NIBRUN_NOTHING", "{NIBRUN_HTTP_PORT}", "${NIBRUN_HTTP_PORT!}",
+      "${NIBRUN_HTTP_PORT with spaces}", "secret$NIBRUN_HTTP_PORT}suffix",
+  };
+  for (size_t index = 0; index < sizeof(literals) / sizeof(literals[0]); index++) {
+    char text[CONFIG_MAX_BYTES];
+    snprintf(text, sizeof(text), "%sENV_SECRET=%s\n", REQUIRED, literals[index]);
+    struct instance_config config;
+    EXPECT(parse(&config, text));
+    EXPECT(strcmp(value_of(config_build_environment(&config), "SECRET"), literals[index]) == 0);
+  }
+}
+
+static void expands_complete_references_among_literal_characters(void) {
+  struct instance_config config;
+  EXPECT(parse(&config, REQUIRED "ENV_MIXED=$${NIBRUN_HTTP_PORT}|{${NIBRUN_DATA_DIR}}|"
+                                 "${NIBRUN_BROKEN:${NIBRUN_HTTP_PORT}|$\n"));
+  EXPECT(strcmp(value_of(config_build_environment(&config), "MIXED"),
+                "$8080|{/app/data}|${NIBRUN_BROKEN:8080|$") == 0);
+}
+
 /* A reference nobody can answer fails the boot rather than reaching the tenant as
  * itself, where it would read as a value somebody meant to write. */
 static void rejects_a_reference_it_cannot_answer(void) {
-  EXPECT(rejects(REQUIRED "ENV_A=$NIBRUN_NOTHING\n"));
-  EXPECT(rejects(REQUIRED "ENV_A=$NIBRUN_HTTP_PORT0\n"));
-  EXPECT(rejects(REQUIRED "ENV_A=${NIBRUN_HTTP_PORT\n"));
+  EXPECT(rejects(REQUIRED "ENV_A=${NIBRUN_NOTHING}\n"));
+  EXPECT(rejects(REQUIRED "ENV_A=${NIBRUN_HTTP_PORT0}\n"));
   /* Offered, but this instance was issued no hostname. */
-  EXPECT(rejects(REQUIRED "ENV_A=$NIBRUN_HOSTNAME\n"));
+  EXPECT(rejects(REQUIRED "ENV_A=${NIBRUN_HOSTNAME}\n"));
   /* Likewise for an app that asked for no port. */
-  EXPECT(rejects(REQUIRED "ENV_A=$NIBRUN_PUBLIC_IPV4\n"));
-  EXPECT(rejects(REQUIRED "ENV_A=$NIBRUN_EXTRA_PUBLIC_PORT\n"));
+  EXPECT(rejects(REQUIRED "ENV_A=${NIBRUN_PUBLIC_IPV4}\n"));
+  EXPECT(rejects(REQUIRED "ENV_A=${NIBRUN_EXTRA_PUBLIC_PORT}\n"));
   /* The supervisor's own, and never handed to a tenant. */
-  EXPECT(rejects(REQUIRED "ENV_A=$NIBRUN_MAX_RESTARTS\n"));
+  EXPECT(rejects(REQUIRED "ENV_A=${NIBRUN_MAX_RESTARTS}\n"));
 }
 
 static void rejects_an_expansion_that_does_not_fit(void) {
@@ -417,6 +439,8 @@ int main(void) {
   drops_a_tenant_public_address();
   expands_a_runtime_reference();
   leaves_every_other_dollar_alone();
+  preserves_incomplete_references();
+  expands_complete_references_among_literal_characters();
   rejects_a_reference_it_cannot_answer();
   rejects_an_expansion_that_does_not_fit();
   rejects_a_broken_file();

packages/protocol/src/domain/app.ts

@@ -87,15 +87,13 @@ const OFFERED = RUNTIME_VALUE_NAMES.join('|');
 const NEEDS_A_PORT = EXTRA_PUBLIC_PORT_VALUES.map((value) => value.name).join('|');
 const NAME_CHARACTER = '[A-Za-z0-9_]';
 
-// A value as the guest reads it: anything but a `$`, a `$` that opens no reference — which is what
-// leaves a bcrypt hash and a literal `$HOME` alone — and the two forms that expand. A name the
-// guest would refuse matches none of them, so it has no way through.
+// Match the guest's complete reference syntax so secrets with bare names or unmatched
+// braces remain literal. Complete references to unavailable names still fail validation.
 const TENANT_VALUE_PATTERN = [
   '^(?:',
   '[^$]',
-  `|\\$(?!\\{?${RUNTIME_VALUE_PREFIX})`,
+  `|\\$(?!\\{${RUNTIME_VALUE_PREFIX}${NAME_CHARACTER}*\\})`,
   `|\\$\\{(?:${OFFERED})\\}`,
-  `|\\$(?:${OFFERED})(?!${NAME_CHARACTER})`,
   ')*$',
 ].join('');
 
@@ -115,12 +113,8 @@ export function interpolableRuntimeValue(name: string): string {
   return `\${${name}}`;
 }
 
-// Both forms that expand, and only the names an app has to have asked for. Not a schema pattern
-// like the one above: whether this is allowed depends on the config beside it, which is not
-// something a value can be validated against on its own.
-const NAMES_A_PORT = new RegExp(
-  `\\$(?:\\{(?:${NEEDS_A_PORT})\\}|(?:${NEEDS_A_PORT})(?!${NAME_CHARACTER}))`,
-);
+// Whether these references are allowed depends on the app's public-port config.
+const NAMES_A_PORT = new RegExp(`\\$\\{(?:${NEEDS_A_PORT})\\}`);
 
 /** Whether `value` names a runtime value only an app with an extra public port is given. */
 export function namesExtraPublicPortValues(value: string): boolean {

packages/protocol/tests/protocol.test.ts

@@ -378,10 +378,8 @@ describe('a variable named __proto__ is not one', () => {
  * the only end of this where whoever wrote it is still listening.
  */
 describe('a value naming a runtime value', () => {
-  // biome-ignore lint/suspicious/noTemplateCurlyInString: the syntax being validated, not an interpolation
-  const OFFERED = '${NIBRUN_HOSTNAME}';
-  // biome-ignore lint/suspicious/noTemplateCurlyInString: the syntax being validated, not an interpolation
-  const MISSPELLED = '${NIBRUN_HSOTNAME}';
+  const OFFERED = `\${NIBRUN_HOSTNAME}`;
+  const MISSPELLED = `\${NIBRUN_HSOTNAME}`;
 
   function holding(value: string) {
     return { CALLBACK_URL: value };
@@ -391,28 +389,46 @@ describe('a value naming a runtime value', () => {
     return isValidMessage({ schema: TenantEnvironmentSchema, value: holding(value) });
   }
 
-  test('both forms the guest expands are accepted', () => {
+  test('complete runtime references are accepted', () => {
     expect(accepts(`https://${OFFERED}/callback`)).toBe(true);
-    expect(accepts('$NIBRUN_HTTP_PORT')).toBe(true);
+    expect(accepts(`\${NIBRUN_HTTP_PORT}`)).toBe(true);
   });
 
   test('a name the guest does not offer is refused', () => {
     expect(accepts(`https://${MISSPELLED}/callback`)).toBe(false);
-    expect(accepts('$NIBRUN_HTTP_PORTS')).toBe(false);
+    expect(accepts(`\${NIBRUN_HTTP_PORTS}`)).toBe(false);
   });
 
-  // The guest reads a name to its last name character, so this one is NIBRUN_HOSTNAME with no
-  // closing brace rather than the value someone meant.
-  test('a brace nobody closed is refused', () => {
-    expect(accepts('https://${NIBRUN_HOSTNAME')).toBe(false);
+  test('a brace nobody closed is literal', () => {
+    expect(accepts(`https://\${NIBRUN_HOSTNAME`)).toBe(true);
   });
 
-  // The prefix is the whole of what expands, which is what lets a secret hold a `$` at all: a
-  // bcrypt hash and a password that reads like a shell variable are values like any other.
   test('a $ that opens no reference is a $', () => {
     expect(accepts('$2y$10$K3JqBQ8Rt7uVwXyZaBcDeF')).toBe(true);
     expect(accepts('$HOME/bin')).toBe(true);
-    expect(accepts('$$')).toBe(true);
+    for (const literal of [
+      '$',
+      '{',
+      '}',
+      `\${`,
+      `\${}`,
+      '$NIBRUN_HTTP_PORT',
+      '$NIBRUN_NOTHING',
+      '$NIBRUN_PUBLIC_IPV4',
+      '$NIBRUN_EXTRA_PUBLIC_PORT',
+      `\${NIBRUN_NOTHING`,
+      '{NIBRUN_HTTP_PORT}',
+      `\${NIBRUN_HTTP_PORT!}`,
+      `\${NIBRUN_HTTP_PORT with spaces}`,
+      'secret$NIBRUN_HTTP_PORT}suffix',
+    ]) {
+      expect(accepts(literal)).toBe(true);
+      expect(
+        isValidMessage({ schema: TenantEnvironmentPatchSchema, value: holding(literal) }),
+      ).toBe(true);
+    }
+    expect(accepts(`$${OFFERED}|{${OFFERED}}|\${NIBRUN_BROKEN:${OFFERED}|$`)).toBe(true);
+    expect(accepts(`\${NIBRUN_BROKEN:${MISSPELLED}`)).toBe(false);
   });
 
   test('an edit is held to the same rule', () => {
@@ -430,30 +446,34 @@ describe('a value naming a runtime value', () => {
  * a value is allowed depends on the config beside it — so it is a question rather than a pattern.
  */
 describe('a value naming a runtime value only some apps are given', () => {
-  test('either name, in either form', () => {
-    // biome-ignore lint/suspicious/noTemplateCurlyInString: the syntax being validated
-    expect(namesExtraPublicPortValues('${NIBRUN_PUBLIC_IPV4}')).toBe(true);
-    expect(namesExtraPublicPortValues('$NIBRUN_EXTRA_PUBLIC_PORT')).toBe(true);
-    expect(namesExtraPublicPortValues('udp://$NIBRUN_PUBLIC_IPV4:$NIBRUN_EXTRA_PUBLIC_PORT')).toBe(
+  test('either name in a complete reference', () => {
+    expect(namesExtraPublicPortValues(`\${NIBRUN_PUBLIC_IPV4}`)).toBe(true);
+    expect(namesExtraPublicPortValues(`\${NIBRUN_EXTRA_PUBLIC_PORT}`)).toBe(true);
+    expect(namesExtraPublicPortValues(`\${NIBRUN_PUBLIC_IPV4}:\${NIBRUN_EXTRA_PUBLIC_PORT}`)).toBe(
       true,
     );
   });
 
   test('a name every app is given is not one of them', () => {
     expect(namesExtraPublicPortValues('$NIBRUN_HOSTNAME')).toBe(false);
-    // biome-ignore lint/suspicious/noTemplateCurlyInString: the syntax being validated
-    expect(namesExtraPublicPortValues('${NIBRUN_HTTP_PORT}')).toBe(false);
+    expect(namesExtraPublicPortValues(`\${NIBRUN_HTTP_PORT}`)).toBe(false);
   });
 
-  // The guest reads a name to its last name character, so this is a longer name it does not offer
-  // rather than one of these with something after it.
   test('a longer name is a different name', () => {
-    expect(namesExtraPublicPortValues('$NIBRUN_PUBLIC_IPV4X')).toBe(false);
+    expect(namesExtraPublicPortValues(`\${NIBRUN_PUBLIC_IPV4X}`)).toBe(false);
   });
 
   test('a value naming nothing names none of them', () => {
     expect(namesExtraPublicPortValues('$2y$10$K3JqBQ8Rt7uVwXyZaBcDeF')).toBe(false);
-    expect(namesExtraPublicPortValues('NIBRUN_PUBLIC_IPV4')).toBe(false);
+    for (const literal of [
+      'NIBRUN_PUBLIC_IPV4',
+      '$NIBRUN_PUBLIC_IPV4',
+      '$NIBRUN_EXTRA_PUBLIC_PORT',
+      `\${NIBRUN_PUBLIC_IPV4`,
+      `\${NIBRUN_EXTRA_PUBLIC_PORT!}`,
+    ]) {
+      expect(namesExtraPublicPortValues(literal)).toBe(false);
+    }
   });
 });
 

skills/deploy-to-nibrun/SKILL.md

@@ -247,9 +247,10 @@ it uses when it is not on nibrun.
 
 A binary that insists on a variable name of its own reaches the same values through it —
 `APP_BASE_URL=https://${NIBRUN_HOSTNAME}`, `DATABASE_URL=file:${NIBRUN_DATA_DIR}/app.db` — and the
-guest expands it before exec. Only the `NIBRUN_` names above expand, and only those: a secret
-holding a `$` arrives untouched, `${PORT}` is not one of them, and anything else is refused when
-you deploy it.
+guest expands it before exec. Only complete `${NIBRUN_NAME}` references to the names above
+expand. Bare names such as `$NIBRUN_HTTP_PORT`, unmatched braces, and other dollar signs remain
+literal. A complete reference to an unknown `NIBRUN_` name is refused when you deploy it;
+`${PORT}` remains literal.
 
 ## A second public port