Skip to content

Fix false local references from slash-separated prose - #451

Open
Tomauskasz wants to merge 1 commit into
NVIDIA:mainfrom
Tomauskasz:fix/reference-prose-false-positives
Open

Fix false local references from slash-separated prose#451
Tomauskasz wants to merge 1 commit into
NVIDIA:mainfrom
Tomauskasz:fix/reference-prose-false-positives

Conversation

@Tomauskasz

Copy link
Copy Markdown

Summary

  • Ignore ordinary slash-separated prose such as reads/writes during plain-text reference extraction.
  • Preserve explicit extensionless paths such as ./references/guide.
  • Preserve extension-bearing paths such as references/guide.md and existing Markdown destinations.

Fixes #450.

Root cause

The plain relative-path pattern accepted a multi-segment token when both the explicit ./ prefix and final file extension were absent. The resolver therefore could not distinguish ordinary prose from an extensionless local path.

The patch requires one explicit path signal for unquoted plain-text references: an explicit ./ prefix or a final extension.

Verification

  • tests/nodes/test_security_remediation.py: 70 passed.
  • ruff check: passed.
  • ruff format --check: passed.
  • git diff --check: passed.
  • A live local-source scan of n8n-post-change-runtime-proof completed with 100% coverage, zero unresolved references, and analysis_completeness.is_complete: true.

Tradeoff

An unquoted extensionless token such as references/guide is ambiguous with prose. Authors must use ./references/guide, Markdown link syntax, or an extension-bearing path.

AI assistance disclosure

Codex inspected the reference resolver, reproduced the defect, drafted the implementation and tests, and ran the verification commands. I reviewed the root cause, diff, behavior, tradeoff, and final pull-request text.

Signed-off-by: Tomas <tomas.bitvinskas@copla.com>
@Tomauskasz

Copy link
Copy Markdown
Author

End-to-end follow-up:

  • All five upstream checks pass.
  • Installed the PR as a packaged uv-tool snapshot.
  • The target skill's ordinary slash prose produced zero unresolved references.
  • Its real Markdown reference still resolved.
  • SkillSpector reported LOW/SAFE, 100% coverage, complete analysis, zero ledger exceptions, and exit 0 with --fail-on-incomplete.
  • SkillEvaluator then accepted the security report and completed a full four-case paired Tier 3 run.

This confirms the parser change at the installed consumer boundary.

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[SkillSpector Review]

The prose false positives are removed, but the new explicit-path branch also drops valid root-level extensionless references such as ./guide. Please retain that existing behavior and add the missing boundary regression. All required checks are green.


_PLAIN_RELATIVE_PATH = re.compile(
r"(?<![\w:/.-])((?:\./)?(?:[A-Za-z0-9_.-]+/)+[A-Za-z0-9_.-]+(?:\.[A-Za-z0-9]{1,12})?)(?![\w/.-])"
r"(?<![\w:/.-])((?:\./(?:[A-Za-z0-9_.-]+/)+[A-Za-z0-9_.-]+|"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[P2] Preserve root-level extensionless ./ references. This branch requires at least one additional directory component after the ./ prefix, so ./guide no longer matches even though an explicit ./ is the syntax signal promised by the issue/PR (and the old regex did match it). Allow zero or more directory segments after ./ and add ./guide to the regression matrix.

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.

Plain slash-separated prose is misclassified as local file references

3 participants