Skip to content

fix(cli): enforce content_path project-root containment for every consumer (CLI-2339) - #6498

Open
Coly010 wants to merge 2 commits into
developfrom
columferry/cli-2339-content_path-resolution-align-project-root-containment
Open

fix(cli): enforce content_path project-root containment for every consumer (CLI-2339)#6498
Coly010 wants to merge 2 commits into
developfrom
columferry/cli-2339-content_path-resolution-align-project-root-containment

Conversation

@Coly010

@Coly010 Coly010 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

CLI-2320 confined auth.email.template.*.content_path/auth.email.notification.*.content_path
resolution to the project root, but only inside config push's own content loader. This
centralizes that containment into the shared resolver legacyResolveEmailTemplateContentPath
(legacy-config-validate.ts), so it now protects every consumer, with no flag and no opt-out:
config push, start (an eager pre-Docker validation pass covering every configured template
plus every enabled notification — the same set Kong's mount builder consumes), and the shared
config-validation path reached by db/migration/status/stop/functions deploy/functions serve/functions download/gen types/inspect/bootstrap.

Linear: CLI-2339 (follow-up from CLI-2320's own PR review, #6489).

While extending the check's reach, two bugs surfaced and are fixed in the same change:

  • The canonicalization helper treated any realpath failure as "this path doesn't exist yet" and
    fell back to lexical resolution — which also covers a dangling symlink, an EACCES-blocked
    target, or a symlink loop, all of which exist on disk but couldn't be canonicalized. That let an
    in-root symlink pointing outside the project root bypass containment silently, most seriously for
    start's Kong mount (a root-privileged, rw Docker bind mount). Fixed by distinguishing
    "genuinely absent" from "exists but uncanonicalizable" and following a symlink to its real target
    before checking it. The ancestor walk was also rewritten iteratively to remove a stack-depth
    limit on deeply nested missing paths.
  • start's Kong mount resolved and validated a path early, then independently re-derived and used
    a second, unresolved path much later when building the Docker bind mount — a check/use gap and
    duplicated resolution logic. The validated, read-verified path is now threaded straight through
    to the bind-mount builder instead of being re-derived.

The rejection message now includes the declared content_path value and the project root (not the
fully symlink-dereferenced target, to avoid echoing back where an escaping symlink actually
points).

What changed

  • legacy-config-validate.tslegacyResolveEmailTemplateContentPath now canonicalizes and
    containment-checks its result before returning; new canonicalPathForContainment/
    canonicalizeExistingPath/isPathContainedInRoot helpers.
  • push.auth-email-content.ts — deleted its local, now-redundant containment helpers; both
    template and notification loading route through the shared resolver.
  • start.handler.ts/kong.service.tsresolveKongEmailTemplateMounts resolves, containment-
    checks, and read-verifies every Kong-mounted template/notification once, early, before any
    Docker work; LegacyKongEmailTemplateMount carries the resolved path, and
    legacyBuildKongEmailTemplateBind is now a pure formatter with no resolution logic of its own.
  • SIDE_EFFECTS.md updates across start, status, stop, db diff, migration squash,
    functions deploy/serve/download, and one line in apps/cli/AGENTS.md's "config validation has
    one home" section.

Follow-ups filed for the adjacent untrusted-path fields this ticket didn't touch (CLI-2344), and a
message-polish gap where an EACCES behind a followed symlink surfaces a raw filesystem error
instead of the usual containment message (CLI-2345) — in both cases the path is still rejected,
just with a less specific error.

…sumer (CLI-2339)

CLI-2320 confined auth.email.template.*/auth.email.notification.*
content_path resolution to the project root, but only inside config
push's own content loader. This centralizes that containment into the
shared resolver in legacy-config-validate.ts, so it now protects every
consumer with no flag and no opt-out: config push, start (an eager
pre-Docker validation pass covering every configured template plus
every enabled notification, the same set Kong's mount builder
consumes), and the shared config-validation path reached by db/
migration/status/stop/functions deploy/serve/download/gen types/
inspect/bootstrap.

Also fixes two bugs found while extending the check's reach:

- The canonicalization helper treated any realpath failure as "this
  path doesn't exist yet" and fell back to lexical resolution - which
  also covers a dangling symlink, an EACCES-blocked target, or a
  symlink loop, all of which exist on disk but couldn't be
  canonicalized. That let an in-root symlink pointing outside the
  project root bypass containment silently, most seriously for start's
  Kong mount (a root-privileged, rw Docker bind mount). Fixed by
  distinguishing "genuinely absent" from "exists but uncanonicalizable"
  and following a symlink to its real target before checking it. The
  ancestor walk was also rewritten iteratively to remove a stack-depth
  limit on deeply nested missing paths.
- start's Kong mount resolved and validated a path early, then
  independently re-derived and used a second, unresolved path much
  later when building the Docker bind mount - a TOCTOU gap and
  duplicated resolution logic. The validated, read-verified path is
  now threaded straight through to the bind-mount builder instead.

The rejection message now includes the declared content_path value and
the project root (not the fully symlink-dereferenced target, to avoid
echoing back where an escaping symlink actually points).

Follow-ups filed for the adjacent untrusted-path fields this ticket
didn't touch (CLI-2344), and a message-polish gap where an EACCES
behind a followed symlink surfaces a raw filesystem error instead of
the usual containment message (CLI-2345) - in both cases the path is
still rejected, just with a less specific error.
@Coly010 Coly010 self-assigned this Sep 7, 2026
@Coly010
Coly010 marked this pull request as ready for review September 7, 2026 15:11
@Coly010
Coly010 requested a review from a team as a code owner September 7, 2026 15:11
…9-content_path-resolution-align-project-root-containment

# Conflicts:
#	apps/cli/src/commands/db/diff/SIDE_EFFECTS.md
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Supabase CLI preview

npx --yes https://pkg.pr.new/supabase/cli/supabase@261fafc8a1ad5afa1f6bfe61c3279c3ed52d0956

Preview package for commit 261fafc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant