Merge pull request #12419 from quarto-dev/bugfix/11967

do not allow !expr tags outside of knitr execution

Author: cscheid

news/changelog-1.7.md

@@ -139,6 +139,7 @@ All changes included in 1.7:
 - ([#11606](https://github.com/quarto-dev/quarto-cli/discussions/11606)): Added a new `QUARTO_DOCUMENT_FILE` env var available to computation engine to the name of the file currently being rendered.
 - ([#11803](https://github.com/quarto-dev/quarto-cli/pull/11803)): Added a new CLI command `quarto call`. First users of this interface are the new `quarto call engine julia ...` subcommands.
 - ([#11951](https://github.com/quarto-dev/quarto-cli/issues/11951)): Raw LaTeX table without `tbl-` prefix label for using Quarto crossref are now correctly passed through unmodified.
+- ([#11967](https://github.com/quarto-dev/quarto-cli/issues/11967)): Produce a better error message when YAML metadata with `!expr` tags are used outside of `knitr` code cells.
 - ([#12117](https://github.com/quarto-dev/quarto-cli/issues/12117)): Color output to stdout and stderr is now correctly rendered for `html` format in the Jupyter and Julia engines.
 - ([#12264](https://github.com/quarto-dev/quarto-cli/issues/12264)): Upgrade `dart-sass` to 1.85.1.
 - ([#11803](https://github.com/quarto-dev/quarto-cli/pull/11803)): Added a new CLI command `quarto call`. First users of this interface are the new `quarto call engine julia ...` subcommands.

src/core/lib/yaml-validation/validator.ts

@@ -30,12 +30,69 @@ import {
 
 import { resolveSchema } from "./resolve.ts";
 
-import { MappedString } from "../text-types.ts";
-import { createLocalizedError } from "./errors.ts";
+import { MappedString, StringMapResult } from "../text-types.ts";
+import { createLocalizedError, createSourceContext } from "./errors.ts";
 import { InternalError } from "../error.ts";
+import { mappedIndexToLineCol } from "../mapped-text.ts";
+import { TidyverseError } from "../errors-types.ts";
 
 ////////////////////////////////////////////////////////////////////////////////
 
+function createNiceError(obj: {
+  violatingObject: AnnotatedParse;
+  source: MappedString;
+  message: string;
+}): TidyverseError {
+  const {
+    violatingObject,
+    source,
+    message,
+  } = obj;
+  const locF = mappedIndexToLineCol(source);
+
+  let location;
+  try {
+    location = {
+      start: locF(violatingObject.start),
+      end: locF(violatingObject.end),
+    };
+  } catch (_e) {
+    location = {
+      start: { line: 0, column: 0 },
+      end: { line: 0, column: 0 },
+    };
+  }
+
+  const mapResult = source.map(violatingObject.start);
+  const fileName = mapResult ? mapResult.originalString.fileName : undefined;
+  return {
+    heading: message,
+    error: [],
+    info: {},
+    fileName,
+    location: location!,
+    sourceContext: createSourceContext(violatingObject.source, {
+      start: violatingObject.start,
+      end: violatingObject.end,
+    }),
+  };
+}
+
+export class NoExprTag extends Error {
+  constructor(violatingObject: AnnotatedParse, source: MappedString) {
+    super(`Unexpected !expr tag`);
+    this.name = "NoExprTag";
+    this.niceError = createNiceError({
+      violatingObject,
+      source,
+      message:
+        "!expr tags are not allowed in Quarto outside of knitr code cells.",
+    });
+  }
+
+  niceError: TidyverseError;
+}
+
 class ValidationContext {
   instancePath: (number | string)[];
   root: ValidationTraceNode;
@@ -561,6 +618,12 @@ function validateObject(
         }
       }
     }
+    if (
+      value.result && typeof value.result === "object" &&
+      !Array.isArray(value.result) && value.result.tag === "!expr"
+    ) {
+      throw new NoExprTag(value, value.source);
+    }
     throw new InternalError(`Couldn't locate key ${key}`);
   };
   const inspectedProps: Set<string> = new Set();

src/core/schema/validate-document.ts

@@ -21,6 +21,7 @@ import { isObject } from "../lodash.ts";
 import { getFrontMatterSchema } from "../lib/yaml-schema/front-matter.ts";
 import { JSONValue, LocalizedError } from "../lib/yaml-schema/types.ts";
 import { MappedString } from "../lib/mapped-text.ts";
+import { NoExprTag } from "../lib/yaml-validation/validator.ts";
 
 export async function validateDocumentFromSource(
   src: MappedString,
@@ -77,22 +78,31 @@ export async function validateDocumentFromSource(
     ) {
       const frontMatterSchema = await getFrontMatterSchema();
 
-      await withValidator(frontMatterSchema, async (frontMatterValidator) => {
-        const fmValidation = await frontMatterValidator.validateParseWithErrors(
-          frontMatterText,
-          annotation,
-          "Validation of YAML front matter failed.",
-          errorFn,
-          reportOnce(
-            (err: TidyverseError) =>
-              error(tidyverseFormatError(err), { colorize: false }),
-            reportSet,
-          ),
-        );
-        if (fmValidation && fmValidation.errors.length) {
-          result.push(...fmValidation.errors);
+      try {
+        await withValidator(frontMatterSchema, async (frontMatterValidator) => {
+          const fmValidation = await frontMatterValidator
+            .validateParseWithErrors(
+              frontMatterText,
+              annotation,
+              "Validation of YAML front matter failed.",
+              errorFn,
+              reportOnce(
+                (err: TidyverseError) =>
+                  error(tidyverseFormatError(err), { colorize: false }),
+                reportSet,
+              ),
+            );
+          if (fmValidation && fmValidation.errors.length) {
+            result.push(...fmValidation.errors);
+          }
+        });
+      } catch (e) {
+        if (e.name === "NoExprTag") {
+          const err = e as NoExprTag;
+          error(tidyverseFormatError(err.niceError), { colorize: false });
+          throw e;
         }
-      });
+      }
     }
   } else {
     firstContentCellIndex = 0;

tests/docs/yaml/issue-11967.qmd

@@ -0,0 +1,5 @@
+---
+title: "Some title"
+format: html
+brand: !expr 'c("demo/_brand.yml")'
+---

tests/smoke/smoke-all.test.ts

@@ -55,7 +55,15 @@ async function guessFormat(fileName: string): Promise<string[]> {
   for (const cell of cells) {
     if (cell.cell_type === "raw") {
       const src = cell.source.value.replaceAll(/^---$/mg, "");
-      const yaml = parse(src);
+      let yaml;
+      try {
+        yaml = parse(src);
+      } catch (e) {
+        if (e.message.includes("unknown tag")) {
+          // assume it's not necessary to guess the format
+          continue;
+        }
+      }
       if (yaml && typeof yaml === "object") {
         // deno-lint-ignore no-explicit-any
         const format = (yaml as Record<string, any>).format;

tests/smoke/yaml-intelligence/issue-11967.test.ts

@@ -0,0 +1,27 @@
+/*
+ * issue-11967.test.ts
+ *
+ * Copyright (C) 2025 Posit Software, PBC
+ *
+ */
+
+import { testQuartoCmd } from "../../test.ts";
+import { fileLoader } from "../../utils.ts";
+import { printsMessage } from "../../verify.ts";
+
+const yamlDocs = fileLoader("yaml");
+
+const testYamlValidationFails = (file: string) => {
+  testQuartoCmd(
+    "render",
+    [yamlDocs(file, "html").input, "--to", "html", "--quiet"],
+    [printsMessage({level: "ERROR", regex: /\!expr tags are not allowed in Quarto outside of knitr code cells/})],
+  );
+};
+
+const files = [
+  "issue-11967.qmd",
+];
+
+files.forEach(testYamlValidationFails);
+