fix(check): refuse paths outside the project, and name the missing paths it skips

.changeset/check-paths-outside-project.md

@@ -0,0 +1,5 @@
+---
+"@taskless/cli": patch
+---
+
+`check` no longer reports a clean result for paths it never scanned. A path outside the project, existing or not, is refused with `INVALID_INPUT` and exit 1. A path that does not exist is still skipped with exit 0, so raw `git diff --name-only` output can still be piped in, but the skipped paths are now named in a notice (`notices` under `--json`).

packages/cli/src/agent/check.md

@@ -1,4 +1,4 @@
-# Topic: check     (CLI v%(CLI_VERSION)s / topic v7)
+# Topic: check     (CLI v%(CLI_VERSION)s / topic v8)
 
 ## Goal
 Run the applicable rules against the codebase and report matches. Two
@@ -209,8 +209,15 @@ delete the rules to make `check` pass, and do not suggest
    ```
    %(TASKLESS_CLI)s check --json $(git diff --name-only main...HEAD)
    ```
-   Paths that don't exist on disk are silently filtered, so you can
-   pipe raw `git diff` output directly without pre-filtering.
+   Paths that don't exist on disk are skipped, so you can pipe raw
+   `git diff` output directly without pre-filtering. The skipped paths
+   are named in a notice (`notices` under `--json`), so a mistyped
+   path is visible rather than reading as clean.
+
+   Paths must be inside the project (the working directory, or
+   `--dir`). A path outside it, existing or not, is refused with
+   `INVALID_INPUT` and exit 1, and nothing is scanned. Run `check`
+   from the project that holds the files instead.
 
    When runtime rules were present but not run (e.g. logged out), the
    JSON also carries `"skipped": [{ "rule": "<name>", "reason": "…" }]`
@@ -253,27 +260,30 @@ delete the rules to make `check` pass, and do not suggest
    (exit code 1); `success: true` with a non-empty `results` array
    means there are only
    warning/info/hint findings (exit code 0); `success: true` with an
-   empty `results` array means the codebase is clean. Findings are
+   empty `results` array means the codebase is clean, unless a notice
+   says the supplied paths were skipped, in which case nothing was
+   scanned. Findings are
    never reported via the `{ ok: false, code, message }` envelope:
    the envelope only appears when the scan itself fails (e.g.
    `SCAN_FAILED`) and the normal results payload is absent.
 
 ## Exit codes
 
 - `0`: All checks passed, no rules configured, or all supplied
-  paths missing
+  paths missing (named in a notice)
 - `1`: Errors detected, scan failed, runtime rules withheld because
   the plan does not include them, an issued ast-grep or Vale rule was
-  edited or copied under a new id, a rule was unaccounted for, or two
-  engines share a rule id
+  edited or copied under a new id, a rule was unaccounted for, two
+  engines share a rule id, or a supplied path is outside the project
 
 ## Errors
 
 When `--json` is set, failures emit `{ ok: false, code, message }`:
 
-| code          | meaning               | fix                              |
-|---------------|-----------------------|----------------------------------|
-| `SCAN_FAILED` | ast-grep scan errored | Report; check rule YAML validity |
+| code            | meaning                       | fix                                       |
+|-----------------|-------------------------------|-------------------------------------------|
+| `SCAN_FAILED`   | ast-grep scan errored         | Report; check rule YAML validity          |
+| `INVALID_INPUT` | a path is outside the project | Run from the project that holds the files |
 
 ## See Also
 

packages/cli/src/agent/ci.md

@@ -1,4 +1,4 @@
-# Topic: ci     (CLI v%(CLI_VERSION)s / topic v4)
+# Topic: ci     (CLI v%(CLI_VERSION)s / topic v5)
 
 ## Goal
 Wire `%(TASKLESS_CLI)s check` into the user's existing CI so rules run
@@ -59,8 +59,10 @@ which CI they use. If multiple match, ask which should run Taskless.
 | Azure Pipelines     | `System.PullRequest.TargetBranch`             |
 | Bitbucket Pipelines | `BITBUCKET_PR_DESTINATION_BRANCH`             |
 
-  `%(TASKLESS_CLI)s check` silently filters paths that don't exist, so raw
-  `git diff --name-only` output can pipe in directly.
+  `%(TASKLESS_CLI)s check` skips paths that don't exist (naming them in a
+  notice), so raw `git diff --name-only` output can pipe in directly.
+  Run it from the repository root: `git diff` paths are relative to the
+  root, and a path outside the project is refused.
 
 **Recommended default:** diff scan on PRs, full scan on pushes to
 main.

packages/cli/src/commands/check.ts

@@ -1,5 +1,12 @@
-import { resolve, isAbsolute, relative } from "node:path";
-import { stat } from "node:fs/promises";
+import {
+  basename,
+  dirname,
+  isAbsolute,
+  join,
+  relative,
+  resolve,
+} from "node:path";
+import { realpath, stat } from "node:fs/promises";
 import process from "node:process";
 import { defineCommand } from "citty";
 
@@ -30,34 +37,110 @@ async function pathExists(absolutePath: string): Promise<boolean> {
   }
 }
 
+/** `target` relative to `root`, or undefined when it lies outside `root`. */
+function relativeInside(root: string, target: string): string | undefined {
+  const relativePath = relative(root, target);
+  const escapes =
+    relativePath === ".." ||
+    relativePath.startsWith(`..${"/"}`) ||
+    relativePath.startsWith(`..${"\\"}`) ||
+    isAbsolute(relativePath);
+  return escapes ? undefined : relativePath;
+}
+
 /**
- * Resolve each positional path against cwd and filter out any that don't
- * exist on disk. Returns a list of paths relative to cwd so `sg scan` can
- * be spawned from cwd and use those paths directly.
+ * `absolutePath` with its nearest existing ancestor directory replaced by that
+ * directory's real path.
+ *
+ * The ancestor rather than the path itself, for two reasons: a missing path
+ * (a file deleted in the diff, even its whole directory) still has a real
+ * spelling to compare, and a symlink inside the project pointing out of it is
+ * judged by where it sits, not by where it leads, exactly as it was before.
  */
-async function filterExistingPaths(
+async function realParentSpelling(absolutePath: string): Promise<string> {
+  let ancestor = dirname(absolutePath);
+  const rest = [basename(absolutePath)];
+  for (;;) {
+    try {
+      return join(await realpath(ancestor), ...rest);
+    } catch {
+      const parent = dirname(ancestor);
+      if (parent === ancestor) return absolutePath;
+      rest.unshift(basename(ancestor));
+      ancestor = parent;
+    }
+  }
+}
+
+interface ClassifiedPaths {
+  /** In the project and on disk, relative to cwd so `sg scan` can use them. */
+  kept: string[];
+  /** In the project and not on disk, as the caller wrote them. */
+  missing: string[];
+  /** Outside the project, as the caller wrote them. */
+  outside: string[];
+}
+
+/**
+ * Sort each positional path into the three cases `check` answers differently.
+ *
+ * All three used to collapse into "drop it silently", so a path outside the
+ * project, or one that does not exist, produced `success: true` with nothing
+ * scanned and nothing said. That is a clean result for a check that did not
+ * look, and a caller cannot tell it from a clean codebase (#475).
+ *
+ * Containment is decided from the path as written, before asking whether it
+ * exists, so `/no/such/file.py` is reported as outside rather than as missing.
+ * Both sides are compared as written AND through their real paths: on macOS
+ * `/tmp` is `/private/tmp`, and `process.cwd()` returns the real spelling, so
+ * a path handed over in either spelling must not read as escaping the project
+ * while cwd is in the other.
+ */
+async function classifyPaths(
   cwd: string,
   rawPaths: string[]
-): Promise<string[]> {
+): Promise<ClassifiedPaths> {
   const resolvedCwd = resolve(cwd);
-  const kept: string[] = [];
+  const roots = [resolvedCwd];
+  try {
+    const realCwd = await realpath(resolvedCwd);
+    if (realCwd !== resolvedCwd) roots.push(realCwd);
+  } catch {
+    // cwd not existing is reported later by whatever reads the project.
+  }
+
+  const classified: ClassifiedPaths = { kept: [], missing: [], outside: [] };
   for (const rawPath of rawPaths) {
     const absolutePath = isAbsolute(rawPath)
       ? resolve(rawPath)
       : resolve(resolvedCwd, rawPath);
-    if (!(await pathExists(absolutePath))) continue;
-    const relativePath = relative(resolvedCwd, absolutePath);
-    // Reject paths that escape cwd (e.g. `../outside-project`) so `sg scan`
-    // never traverses outside the project directory.
-    const escapesCwd =
-      relativePath === ".." ||
-      relativePath.startsWith(`..${"/"}`) ||
-      relativePath.startsWith(`..${"\\"}`) ||
-      isAbsolute(relativePath);
-    if (escapesCwd) continue;
-    kept.push(relativePath === "" ? "." : relativePath);
+    const spellings = [absolutePath, await realParentSpelling(absolutePath)];
+    const relativePath = roots
+      .flatMap((root) =>
+        spellings.map((spelling) => relativeInside(root, spelling))
+      )
+      .find((candidate) => candidate !== undefined);
+    if (relativePath === undefined) {
+      classified.outside.push(rawPath);
+    } else if (await pathExists(absolutePath)) {
+      classified.kept.push(relativePath === "" ? "." : relativePath);
+    } else {
+      classified.missing.push(rawPath);
+    }
   }
-  return kept;
+  return classified;
+}
+
+/**
+ * The notice for paths that were skipped because they do not exist.
+ *
+ * Skipped rather than refused: raw `git diff --name-only` output names
+ * deleted files, and the CI recipe pipes it straight in, so a missing path is
+ * an ordinary input. It is still named, so a mistyped path is visible.
+ */
+function missingPathsNotice(missing: string[]): string {
+  const noun = missing.length === 1 ? "path that does" : "paths that do";
+  return `Skipped ${missing.length} ${noun} not exist: ${missing.join(", ")}`;
 }
 
 /**
@@ -205,22 +288,49 @@ export const checkCommand = defineCommand({
     try {
       const positionalPaths = extractPositionalPaths(rawArgs);
       const hadExplicitPaths = positionalPaths.length > 0;
-      const existingPaths = hadExplicitPaths
-        ? await filterExistingPaths(cwd, positionalPaths)
-        : [];
+      const {
+        kept: existingPaths,
+        missing,
+        outside,
+      } = await classifyPaths(cwd, positionalPaths);
+
+      // Refused, not skipped. `check` only scans inside the project, and a
+      // path outside it is a caller pointing at the wrong place: reporting it
+      // clean is the false negative #475 describes.
+      if (outside.length > 0) {
+        const message = `${outside.length === 1 ? "Path is" : "Paths are"} outside the project (${resolve(cwd)}): ${outside.join(", ")}. \`check\` only scans inside the project; run it from that project, or pass \`--dir\`.`;
+        if (args.json) {
+          writeJsonError("INVALID_INPUT", message);
+        } else {
+          console.error(`Error: ${message}`);
+        }
+        process.exitCode = 1;
+        return;
+      }
+
+      const pathNotices =
+        missing.length > 0 ? [missingPathsNotice(missing)] : [];
 
       // If the user passed paths but none exist (e.g. all-deleted diff),
-      // exit cleanly with empty results rather than falling back to a full scan.
+      // exit cleanly with empty results rather than falling back to a full
+      // scan, saying which paths were skipped.
       if (hadExplicitPaths && existingPaths.length === 0) {
         if (args.json) {
           console.log(
             JSON.stringify(
-              checkOutputSchema.parse({ success: true, results: [] })
+              checkOutputSchema.parse({
+                success: true,
+                results: [],
+                ...(pathNotices.length > 0 ? { notices: pathNotices } : {}),
+              })
             )
           );
+        } else {
+          for (const notice of pathNotices) warnNotice(notice);
         }
         return;
       }
+      for (const notice of pathNotices) warnNotice(notice);
 
       // REFUSES rather than migrates. This used to call
       // `ensureTasklessDirectory`, so a command whose entire job is to report
@@ -330,6 +440,7 @@ export const checkCommand = defineCommand({
               checkOutputSchema.parse({
                 success: true,
                 results: [],
+                ...(pathNotices.length > 0 ? { notices: pathNotices } : {}),
               })
             )
           );
@@ -410,7 +521,11 @@ export const checkCommand = defineCommand({
         const results = dispatched.results;
 
         for (const notice of dispatched.notices) warnNotice(notice);
-        const runNotices = [...plan.notices, ...dispatched.notices];
+        const runNotices = [
+          ...pathNotices,
+          ...plan.notices,
+          ...dispatched.notices,
+        ];
         for (const failure of dispatched.failures) warn(`Error: ${failure}`);
 
         let errorCount = 0;