Fix check crash and raw render error on unreadable project dirs - #14966
Merged
Merged
Conversation
cderv
added this pull request to stack #14967
September 30, 2026 16:09
Collaborator
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
1 of 2 tasks
`quarto check` builds a full project context just to read `config.engines` for external engine registration. That walks every input file under the project root and opens a disk cache in `<root>/.quarto`. With a stray `_quarto.yml` in a parent directory (e.g. the home dir), check walks that whole tree and crashes with a raw PermissionDenied on the first unreadable directory, and it creates a `.quarto` dir at that root, even though it is a diagnostic command that never uses input files. The config part of `projectContext()` (upward `_quarto.yml` search, extension detector pass, profiles, dotenv, vars, translations and `project:` normalization) is now `resolveProjectConfig()`, which also reports which `_quarto.yml` set the root. `projectContext()` calls it and then builds the context and walks the inputs as before. `initializeProjectContextAndEngines()` (check, create, create-project, call engine) uses it directly: none of these callers use input files, and `resolveEngines()` reads only `config.engines`. Project-type `config()` hooks are skipped on that path, and none of them touch `engines`. Adds a test helper that makes a directory unreadable (icacls on Windows, chmod elsewhere, unavailable as root), and llm-docs coverage of project context resolution. Refs #14960
Callers outside project resolution (check, create, call engine) should not have to build an extension loader to read project config. projectContext() still passes its render services' loader so render reuses the extension cache.
The fixture is built at registration, and an ignored test never runs its teardown, so running as root left the temp project behind.
A stray _quarto.yml in a parent directory silently turns everything below
it into one project, and nothing told the user which file was in play.
`quarto check info` now prints the project root and the _quarto.yml that
set it, or that no project was found (single-file mode). With --output,
the JSON gets info.project = { dir, configFile } (configFile null when an
extension's project-type detector set the root), or null outside a
project.
A root at the user home directory or a filesystem root is almost always
an accident, so check warns on stderr in that case; the JSON file stays
data only.
The resolution comes from resolveProjectConfig(), which
initializeProjectContextAndEngines() now returns instead of discarding.
The other callers (create, create-project, call engine) ignore it.
The home-directory test spawns quarto as a subprocess with HOME and
USERPROFILE pointing at a temp project: in dev mode the harness runs
quarto in-process and applies TestContext.env through Deno.env.set,
which would leak into concurrently running tests.
Refs #14960
…view A stray _quarto.yml (e.g. in the home directory) turns every path below it into a project, and render then walks the whole tree. When that walk hits a directory the user cannot list, render and preview printed the raw Deno readdir error with a stack trace, which says nothing about why Quarto was reading that directory. The walk now tags its PermissionDenied with the project root, and projectContext() adds the _quarto.yml that set it. The error object itself is unchanged, so inspect and other callers report exactly what they did before. render and preview turn only tagged errors into one framed error: the Deno message as is (it carries the unreadable path), the project root, the _quarto.yml, and a hint that the file may have been created by accident. Catching around projectContext() as a whole was avoided because config, profile and cache reads can raise PermissionDenied too. projectContext() also left the <root>/.quarto disk cache open and the session temp dir behind when it threw after building the context: cleanup was only registered on success. Each branch now releases the context before rethrowing. On Windows the open cache made the project directory impossible to remove in the same process (os error 32). Refs #14960
A project with `project: render` globs lists its inputs through resolvePathGlobs, whose expandGlob traversal is separate from the addDir walk. A recursive glob such as `**/*.qmd` reaching an unreadable directory still surfaced the raw readdir error with a stack trace in render and preview. Tag the PermissionDenied from that traversal the same way. File reads in addFile are left untagged on purpose: the framed message says a directory could not be read.
CheckConfiguration now requires a project field, and this test builds the object inline, so it failed type-checking on CI.
On macOS, Deno.makeTempDirSync returns a path under /var/folders, a symlink to /private/var/folders, while quarto derives the project root from the process cwd, which is a real path. Tests comparing the project root or the readdir path in the framed permission error against the temp dir would then fail on the scheduled macOS smoke runs. check-info-project.test.ts already resolves its temp dir the same way.
resolveEngines only reads project.config, but its ProjectContext parameter forced initializeProjectContextAndEngines to fabricate a context with `as ProjectContext` casts, both for a resolved project and for the no-project bundled-engine case. Taking Pick<ProjectContext, "config"> lets it pass the config directly; full-context callers still type-check. The no-project helper's dynamic imports bought nothing, since project-context.ts already loads extension.ts statically, so they become static imports.
…eDir src/quarto.ts monkey-patches Deno.realPathSync to normalizePath, which does not follow symlinks. The test harness imports src/quarto.ts through quarto-cmd.ts, so the macOS /var -> /private/var resolution added to the temp project helpers was a no-op, and quarto check's home-directory comparison missed a symlinked HOME. originalRealPathSync keeps the unpatched function.
The existing home-warning test passes an already resolved path as HOME, so it also passed with the monkey-patched Deno.realPathSync that never followed the symlink. Pointing HOME at a symlink to the project root fails without originalRealPathSync in userHomeDir. Skipped on Windows, where creating a directory symlink needs Developer Mode or admin rights.
The basic markdown render in checkInstall and the engine renders through checkRender write their document to the system temp dir. That dir can sit beneath a project root: on Windows %TEMP% is under the home directory, so a stray ~/_quarto.yml made render() resolve the home project for the temp file, create its .quarto dir and walk its inputs, failing on any unreadable directory even though `check info` no longer walks. A temp document has no relation to any project, so both renders now get an explicit single-file project context.
`check install` never reaches checkRender(), so the existing subprocess test left the engine-check render path uncovered. Calling checkRender() in process with a temp context rooted inside the project exercises it without touching the process environment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a
_quarto.ymlis left in the home directory by accident, every path under~becomes part of one project. Input discovery then walks all of~, and on the first directory Quarto cannot read (Photos Library.photoslibraryon macOS in #14960) the command fails with a rawPermissionDenied ... readdirerror and a stack trace. This happens also withquarto check, which is the command a user runs to understand what is wrong. I reproduced it on Windows with anicaclsdeny on a directory inside the project.New improved behavior
quarto check infooutputRoot Cause
quarto checkbuilds a fullprojectContext()only to readconfig.engines(initializeProjectContextAndEngines()insrc/command/command-utils.ts). So it runs the input walk and creates<root>/.quarto, while nothing in check uses input files. The walk has no error handling, sorenderandpreviewshow the Deno error as is.Fix
resolveProjectConfig()is extracted fromprojectContext(): upward_quarto.ymlresolution and the whole config block (profiles, engine extensions, dotenv, vars, translations).projectContext()calls it and then walks as before.check,create,create-projectandcall enginenow use only the config part, so they don't walk inputs or create<root>/.quartoanymore. The config block is kept whole so check sees the same engines and profiles as render.quarto check inforeports the project root and the_quarto.ymlthat set it (info.project = { dir, configFile }in JSON,nulloutside a project), and warns when the root is the home directory or a filesystem root.project.renderglob expansion tag aPermissionDeniedwith the project dir and config file, and rethrow the same error.renderandpreviewturn a tagged error into one framed error without stack, with the unreadable dir, the project root, the_quarto.ymland a hint that it may have been created by accident.inspectis unchanged on purpose, it is machine-read and must be complete or fail..quartocache and session temp dir before rethrowing.We don't skip unreadable dirs anywhere. A render or inspect that is silently incomplete is worse than failing.
llm-docs/project-context-architecture.mddocuments how the project context is resolved and the cache lifecycle.Test Plan
checkfrom a subdir of a project with an unreadable dir succeeds and reports the project, and creates no<root>/.quartocheck infoJSONinfo.projectinside and outside a project, home-dir warning through aHOME/USERPROFILEoverlayrenderandproject.renderglob expansion fail with the framed error,inspectkeeps the raw errorprintStack=falseis pinned intests/unit/project/project-context-unreadable-dir.test.ts. Dev builds always setQUARTO_DEBUG=true, so the smoke test ignores the appended stack. Checked by hand withQUARTO_DEBUG=falsefor render, preview of a file and preview of a dir.chmod 000path is only exercised on Ubuntu and macOS is untested.The unreadable-dir fixture uses an
icaclsdeny on Windows andchmod 000elsewhere. These tests are not registered when running as root. GitHub Ubuntu runners are non-root, so CI enforces the denial. Deno's resource sanitizer can't be used inunitTest, the harness itself leaks a file handle and a timeout timer. So the cache cleanup test relies on os error 32 on Windows, plus a check on all OSes that no.quarto/quarto-session-temp*is left behind.Depends on #14965
Fixes #14960