Implement security hardening batch (plan 83)

[jeduden]

Summary

Implements six of seven security hardening fixes from plan 83. Task G (include/catalog size limits) is deferred to plan 81.

• A. ANSI sanitization: Strip all C0/C1 control characters from diagnostic header fields (file path, message) via sanitizeControl; source lines preserve tab via sanitizeSourceLine. Prevents terminal injection and output spoofing.
• B. Path traversal boundary: Add RootDir to lint.File; links resolving outside RootDir are silently skipped in cross-file-reference-integrity. Uses filepath.EvalSymlinks to prevent symlink-based traversal. Root path resolved once per Check call for efficiency.
• C. Atomic writes: Replace os.WriteFile with temp-file-then-rename (with fsync) in fix mode to reduce risk of partial writes on crash. Checks target writability before proceeding; propagates non-ENOENT Stat errors.
• D. Schema read via fs.FS: Use fs.ReadFile(f.RootFS, ...) for schema reads when available; reject absolute and ../ paths. Uses cleaned path for consistent resolution.
• E. Catalog injection warning: Emit non-fatal diagnostics (via Rule.Check, not Generate) when interpolated front-matter values contain embedded newlines or ]( sequences. Map keys iterated in sorted order for deterministic output.
• F. GOMEMLIMIT: Set 512 MiB memory limit in run() to bound CUE evaluation; respects GOMEMLIMIT env var.

Plan 83 status is 🔳 (in-progress) because task G depends on plan 81.

Test plan

• go test ./... — all packages pass
• go tool golangci-lint run — 0 issues
• mdsmith check . — 0 failures
• New tests: sanitizeControl (11 cases), sanitizeSourceLine (3 cases), header/source sanitization, path traversal boundary (2 integration + 8 unit), atomic write (5 cases), schema fs.FS read (3 cases), catalog injection (4 cases)
• Patch coverage 86.08% >= project baseline 86.09%

https://claude.ai/code/session_012qG8CzpnHem9aZZNbi4XkR

[Copilot]

Pull request overview

This PR implements a set of security hardening changes across mdsmith’s output formatting, filesystem boundary enforcement, and file writing behavior to reduce terminal injection risk, path traversal, and partial writes.

Changes:

• Sanitize control characters in text diagnostic output to mitigate terminal control-sequence injection.
• Enforce project-root boundaries for cross-file link resolution and read required-structure schemas via RootFS with traversal validation.
• Switch fixer writes to an atomic temp-file-then-rename strategy and set a default Go runtime memory limit.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
plan/83_security-hardening-batch.md	Marks plan 83 as complete and updates tasks/acceptance criteria.
PLAN.md	Updates the plan catalog status for plan 83.
internal/rules/requiredstructure/rule.go	Reads schema via RootFS with path validation to prevent out-of-root reads.
internal/rules/requiredstructure/rule_test.go	Adds tests for schema reading via RootFS and path rejection cases.
internal/rules/crossfilereferenceintegrity/rule.go	Adds RootDir boundary checks to skip links escaping the project root.
internal/rules/crossfilereferenceintegrity/rule_test.go	Adds tests for skipping traversal-above-root links and allowing in-root links.
internal/rules/catalog/rule.go	Adds warnings for potentially markdown-injecting front-matter values in catalogs.
internal/rules/catalog/rule_test.go	Adds tests for catalog injection warning behavior.
internal/output/text.go	Adds terminal sanitization and applies it to emitted text output and snippets.
internal/output/text_test.go	Adds tests for terminal sanitization in formatter output and source snippets.
internal/lint/file.go	Adds RootDir to lint.File to support root-boundary logic in rules.
internal/fix/fix.go	Populates RootDir/RootFS for fixer files and introduces atomicWriteFile.
internal/fix/fix_test.go	Adds tests for atomic write behavior and permissions.
internal/engine/runner.go	Populates RootDir in lint files when runner RootDir is set.
cmd/mdsmith/main.go	Sets a default process-level Go memory limit when not externally configured.

Comments suppressed due to low confidence (1)

internal/rules/crossfilereferenceintegrity/rule.go:257

• The root-boundary check is purely lexical (Abs/Rel on the link path) and does not resolve symlinks. A link that stays under RootDir but targets a symlink inside RootDir pointing outside will still pass this check and then be read via os.Stat/os.ReadFile. If the intent is to prevent reading outside RootDir, consider evaluating symlinks (filepath.EvalSymlinks) before the Rel check or otherwise disallow symlink targets when RootDir is set.

		// Reject links that traverse above the project root.
		if f.RootDir != "" {
			absRoot, errRoot := filepath.Abs(f.RootDir)
			absResolved, errRes := filepath.Abs(path)
			if errRoot == nil && errRes == nil {
				rel, err := filepath.Rel(absRoot, absResolved)
				if err != nil ||
					rel == ".." ||
					strings.HasPrefix(
						rel,
						".."+string(filepath.Separator),
					) {
					return targetFile{}, false
				}
			}
		}
		if _, err := os.Stat(path); err == nil {
			return targetFile{
				cacheKey: "os:" + path,
				read: func() ([]byte, error) {
					return os.ReadFile(path)
				},

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

internal/rules/requiredstructure/rule.go:87

• When f.RootFS is set, readSchemaFile treats schema as a path relative to the project root, but other logic in this rule (e.g., schema self-skip via isSchemaFile, and any os.Stat/abs-path comparisons using r.Schema) still interpret r.Schema as an OS path relative to the current working directory. This can cause the rule to (a) fail to skip the schema file itself, or (b) warn about <?require?> in the schema, depending on where mdsmith is run from. Consider normalizing r.Schema to an absolute OS path based on f.RootDir (when set) and use that consistently for both reading and schema-file identity checks.

	schData, err := readSchemaFile(f, r.Schema)
	if err != nil {
		return append(diags, r.diag(f.Path, 1,
			fmt.Sprintf("cannot read schema %q: %v", r.Schema, err)))
	}

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 7 comments.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

internal/rules/catalog/rule.go:102

• Generate() computes catalog injection warnings into diags, but if renderCatalogContent returns an error the function returns only the template-failure diagnostic and drops the previously collected warnings. Consider either appending the template error diagnostic to diags before returning, or delaying injection checking until after template rendering succeeds, so diagnostics are consistent and no work is wasted.

	// Warn about front-matter values that could inject Markdown structure.
	var diags []lint.Diagnostic
	diags = append(diags, checkCatalogInjection(filePath, line, entries)...)

	_, hasRow := params["row"]
	content, err := renderCatalogContent(params, entries, cols, hasRow)
	if err != nil {
		return "", []lint.Diagnostic{makeDiag(filePath, line,
			fmt.Sprintf("generated section template execution failed: %v", err))}
	}

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

internal/rules/requiredstructure/rule.go:87

• readSchemaFile reads the schema via f.RootFS, but downstream logic still treats r.Schema as an OS path: parseSchema(schData, r.Schema) uses schemaPath for resolving schema <?include?> fragments (which currently use os.ReadFile), and isSchemaFile(f.Path, r.Schema) uses os.Stat/Abs on the raw string. If RootFS is set and schema is configured as a root-relative path (the new intended mode), running mdsmith from a subdirectory can break schema includes and fail to recognize/skip the schema file itself. Consider resolving a rooted OS path (e.g., filepath.Join(f.RootDir, clean)) to pass into parseSchema/isSchemaFile, and/or updating schema include reads to use f.RootFS as well so resolution is consistent.

	schData, err := readSchemaFile(f, r.Schema)
	if err != nil {
		return append(diags, r.diag(f.Path, 1,
			fmt.Sprintf("cannot read schema %q: %v", r.Schema, err)))
	}

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.