Skip to content

fix(analysis): allow safe contained instruction aliases - #51

Open
oldwinter wants to merge 5 commits into
QoderAI:mainfrom
oldwinter:codex/contained-structural-symlinks
Open

fix(analysis): allow safe contained instruction aliases#51
oldwinter wants to merge 5 commits into
QoderAI:mainfrom
oldwinter:codex/contained-structural-symlinks

Conversation

@oldwinter

Copy link
Copy Markdown
Contributor

Summary

  • allow direct, relative, same-directory tracked instruction-route symlinks to resolve inside a Git workspace
  • reject links that escape the root, target ignored/untracked/non-structural files, alias manifests, cross ownership scopes, or introduce symlink chains
  • use the canonical target for reads and document the accepted topology contract

Verification

  • node --test test/workspace-topology.test.mjs (16 tests passed)
  • npm run check after npm ci (1101 tests passed; npm package and runtime bundle verification passed)

No matching open upstream issue was found.

oldwinter and others added 5 commits August 2, 2026 04:59
Resolve structural entries canonically so tracked provider adapters inside the topology root remain inspectable while escaping links stay rejected.

Tests: node --test test/workspace-topology.test.mjs; frozen general-tasks evidence-bundle

Co-authored-by: Codex (GPT 5.6 Sol) <codex@openai.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the workspace-topology discovery logic to allow a narrowly-scoped class of safe, contained, tracked instruction-route symlinks (e.g., CLAUDE.md -> AGENTS.md) while continuing to reject unsafe symlink patterns. It also documents the topology contract and adds regression tests to lock in the intended behavior.

Changes:

  • Allow direct, same-directory, tracked instruction-route symlink redirects inside the Git root and read content via the canonical target path.
  • Add workspace topology tests covering accepted contained instruction links and multiple rejected symlink topologies (cross-scope, chained hop, ignored/untracked/non-structural targets, manifest aliasing).
  • Add a spec describing intent, acceptance scenarios, non-goals, and verification evidence.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
test/workspace-topology.test.mjs Adds regression coverage for accepting a contained tracked instruction symlink and rejecting unsafe symlink variants.
scripts/workspace-topology/manifests.mjs Implements canonical-path-based structural safety checks and narrowly permits safe instruction-route symlink redirects.
docs/specs/2026-08-02-contained-structural-symlinks.md Documents the accepted symlink topology contract, risks, and verification steps.
Suppressed comments (1)

scripts/workspace-topology/manifests.mjs:192

  • safeRedirect relies on case-sensitive route comparisons (canonicalRoute === item.route, trackedRoutes.has(canonicalRoute), and dirnameRoute(...) === ...). On Windows’ default case-insensitive filesystems this can incorrectly mark safe structural files/links as unsafe when realpath() returns different casing. Suggest using a win32-normalized key (e.g. lowercased) for these route comparisons while keeping the original route values for reporting.
      const canonicalRoute = path.relative(canonicalRoot, canonical).split(path.sep).join("/");
      const safeRedirect = canonicalRoute === item.route
        || (
          item.provenance === "tracked"
          && trackedRoutes.has(canonicalRoute)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +171 to +174
const target = path.normalize(await readlink(absolute));
return !path.isAbsolute(target)
&& path.dirname(target) === "."
&& path.resolve(path.dirname(absolute), target) === canonical;
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.

2 participants