Skip to content

fix(platform-schema): harden check_code_ref — reject empty symbols and path traversal #1340

Description

@chaodu-agent

Why

check_code_ref in crates/platform-schema/tests/conformance.rs is the anti-drift engine — it validates that every source code-ref in platform schema files still points at a real file and symbol. However, it has edge cases that reduce trust in the validation:

  1. Empty symbol bypass"file.rs#" (trailing # with no symbol) parses to symbol = Some(""). Since text.contains("") always returns true in Rust, the check silently passes. A typo or copy-paste error could create a ref that looks validated but isn't.

  2. Path traversalroot.join(r.file) accepts ../ components. A source like "../../etc/passwd" would pass is_file(). While PR review would likely catch this, defense-in-depth should reject it programmatically.

  3. Inconsistent URL handlingfeature_sources_exist_in_tree rejects non-code-ref sources with a confusing "expected a file ref" error. quirk_code_sources_exist_in_tree correctly uses is_code_ref() to skip URLs. If a contributor adds a URL to a feature source, they get a misleading error.

Benefits

  • Higher confidence in anti-drift — no silent bypass routes exist; if CI passes, refs are genuinely validated
  • Defense-in-depth — even if a bad ref slips through review, CI catches it
  • Better contributor DX — clear error messages for each failure mode (not a raw Rust panic)
  • Consistent behavior — feature and quirk source validation follow the same logic

Proposed Fix

1. Reject empty symbol

if let Some(sym) = r.symbol {
    if sym.is_empty() {
        return Err("empty symbol after '#' is not allowed".to_string());
    }
    let text = fs::read_to_string(&path).map_err(|e| e.to_string())?;
    if !text.contains(sym) {
        return Err(format!("symbol {sym:?} not found in {:?}", r.file));
    }
}

2. Prevent path traversal

let resolved = root.join(r.file).canonicalize().map_err(|e| e.to_string())?;
if !resolved.starts_with(root) {
    return Err(format!("source path {:?} escapes repo root", r.file));
}

3. Align URL handling

In feature_sources_exist_in_tree, add before the error push:

if !is_code_ref(src) {
    continue;  // skip URLs, same as quirk test
}

Context

Identified during group review of #1295 (B1, B4, B5 flagged independently). Ref: #1294.

Metadata

Metadata

Assignees

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions