From 377a7b1fb773d4e1f6277a43344cf5acc7302abd Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 12 Jul 2026 13:12:13 +0000 Subject: [PATCH 1/5] feat(owen): public facade rebrand for the CLI, Action, and SARIF identity Public product identity is "Owen" going forward - a package published as "OwnSharp.Cli" with the permanent command "ownsharp" would have locked the public identity to a single-language framing the project doesn't actually have (Own.NET is language-neutral at the OwnIR/core level and may ship a TypeScript frontend later). This is a public facade rebrand, not an internal refactor - see docs/notes/owen-public-facade.md for the full account of what changed and, just as deliberately, what did not. Public identity implemented: - NuGet package ID: OwnSharp.Cli -> Owen.Cli (confirmed unclaimed on nuget.org at time of writing via the v3 flat-container API) - Tool command: ownsharp -> owen - Action display name: "Own.NET resource-leak check" -> "Owen lifetime/resource check"; default SARIF filename own-net.sarif -> owen.sarif - SARIF tool.driver.name: "Own.NET" -> "Owen" (ownlang/ownir.py + ownlang/diag_sarif.py - the one place both the C#-facts and the .own-diagnostics SARIF paths share) - Cache directory: ~/.ownsharp/core// -> ~/.owen/core//, with a simple fallback read (not a migration subsystem) from the previous location if a matching version was already unpacked there - Env var: OWEN_PYTHON preferred; legacy OWN_PYTHON still works as a temporary fallback and prints a deprecation note when it's the one actually used Kept unchanged (internal names, not mass-renamed): the OwnSharp.Cli project/namespace, OwnSharp.Extractor (project + its real ownsharp-extract.dll output filename), ownlang (package/module names, CLI, PYTHONPATH conventions), OwnIR, every OWN0xx diagnostic code, the Rust crates, every frontend/* directory name, historical docs/issues, the GitHub repo name, and scripts/own-check.sh (the Action still calls it internally). AssemblyName in the csproj stays "ownsharp" - it names the internal dll the owen shim launches, never seen by a user, so renaming it adds no user value. CLI contract additions: - `owen --help` is language-neutral at the product level while explicitly listing the one frontend actually wired in today ("Included frontend: .NET / C# (.cs, .csproj, .sln)") - no speculative TypeScript claim, no plugin framework added. - Unsupported input (e.g. a .ts file, or a directory with no .cs files) now fails explicitly with a new exit code 4 and an explicit message, instead of silently printing "0 findings" - indistinguishable from a genuinely clean C# scan before this change. CheckCommand.HasSupportedInput checks this before the extractor runs. Tests: ci.yml's ownsharp-cli-smoke job (key kept; only content changed) now pins package ID, command name, --help framing, --version, the unknown-command "owen:" prefix, a leak sample and a clean negative control both run from an installed checkout-free location, the Owen SARIF driver name, the exit-4 unsupported-input path (asserting the literal clean-scan phrase "0 findings." does NOT appear), OWEN_PYTHON's not-found path, and the legacy OWN_PYTHON fallback + its deprecation note - on both ubuntu-latest and windows-latest. Every one of these assertions was manually replicated and verified on this machine before being written into the workflow. Two other pre-existing SARIF driver-name assertions (ci.yml's own-check-codescan structural validation, tests/test_diag_sarif.py, tests/test_ownir.py) were updated to match "Owen" - caught by python tests/run_tests.py failing before the fix, not scope creep. PR separation: this is PR 1 of 3. Phase 3 (NuGet release pipeline, PR #244) and Phase 4 (Marketplace preparation, PR #245) both need to be rebuilt against this PR once merged, targeting Owen.Cli/owen and the Owen display name instead of the pre-rebrand identity their first drafts used. No oracle remeasurement or analyzer-semantics change is mixed into this PR. --- .github/workflows/ci.yml | 141 ++++++++++++++--- README.md | 7 +- README.ru.md | 5 +- action.yml | 18 +-- docs/notes/alpha-readiness.md | 8 +- docs/notes/owen-public-facade.md | 145 ++++++++++++++++++ frontend/roslyn/OwnSharp.Cli/CheckCommand.cs | 81 ++++++++-- frontend/roslyn/OwnSharp.Cli/CoreVendor.cs | 30 +++- .../roslyn/OwnSharp.Cli/OwnSharp.Cli.csproj | 28 +++- frontend/roslyn/OwnSharp.Cli/Program.cs | 35 +++-- .../roslyn/OwnSharp.Cli/PythonResolver.cs | 42 +++-- frontend/roslyn/OwnSharp.Cli/README.md | 90 ++++++----- frontend/roslyn/OwnSharp.Cli/ToolVersion.cs | 5 +- ownlang/diag_sarif.py | 6 +- ownlang/ownir.py | 4 +- tests/test_diag_sarif.py | 2 +- tests/test_ownir.py | 2 +- 17 files changed, 519 insertions(+), 130 deletions(-) create mode 100644 docs/notes/owen-public-facade.md diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ceafac49..2bd80500 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1717,15 +1717,15 @@ jobs: run: | # The contract GitHub's code-scanning ingest enforces, checked locally so # the upload (own-check-codescan job) is never the first place a drift is - # found: a single-run 2.1.0 log, the Own.NET driver, and every result + # found: a single-run 2.1.0 log, the Owen driver, and every result # carrying a catalogue ruleId + a located file. No upload, no permissions. out="$RUNNER_TEMP/own.sarif" scripts/own-check.sh --format sarif --severity warning -- frontend/roslyn/samples > "$out" echo "wrote $(wc -c < "$out") bytes" jq -e '.version == "2.1.0" and ((.runs | length) == 1)' "$out" >/dev/null \ || { echo "FAIL: not a single-run SARIF 2.1.0 log"; exit 1; } - jq -e '.runs[0].tool.driver.name == "Own.NET"' "$out" >/dev/null \ - || { echo "FAIL: tool.driver.name is not Own.NET"; exit 1; } + jq -e '.runs[0].tool.driver.name == "Owen"' "$out" >/dev/null \ + || { echo "FAIL: tool.driver.name is not Owen"; exit 1; } # A dangling ruleId or an unlocated result is the #1 reason GitHub rejects # a SARIF; startLine is optional (a file-level finding omits it -> // 1). jq -e ' @@ -1739,7 +1739,7 @@ jobs: and ((.locations[0].physicalLocation.region.startLine // 1) | type == "number")) ' "$out" >/dev/null \ || { echo "FAIL: a result is unlocated or references an undeclared rule"; exit 1; } - echo "OK: SARIF 2.1.0 — Own.NET driver, every result rule-backed + located" + echo "OK: SARIF 2.1.0 — Owen driver, every result rule-backed + located" - name: The composite action runs end-to-end (non-failing) uses: ./ with: @@ -1942,7 +1942,10 @@ jobs: # Alpha gate A (issue #202): the single delightful command, proven end-to-end # on a clean runner — install -> check -> findings. Packaging only, no - # analysis-behaviour change: OwnSharp.Cli bundles the *unmodified* extractor + # analysis-behaviour change: the underlying project stays OwnSharp.Cli + # internally (P-013; not mass-renamed), but the PUBLISHED identity is the + # Owen public facade (docs/notes/owen-public-facade.md) — package ID + # Owen.Cli, command `owen`. Bundles the *unmodified* extractor # (ProjectReference; invoked as a child process, same shape own-check.sh # already uses) and vendors the *unmodified* ownlang/ core, run by the # machine's own Python. Both ubuntu AND windows matter here specifically @@ -1950,7 +1953,7 @@ jobs: # Windows and a shell script on Unix, so they exercise genuinely different # process-launch mechanics; ubuntu-only would not prove the Windows path. ownsharp-cli-smoke: - name: ownsharp CLI (gate A) — clean install -> check -> findings + name: owen CLI (gate A) — clean install -> check -> findings strategy: fail-fast: false matrix: @@ -1959,7 +1962,7 @@ jobs: defaults: run: # bash (git-bash on Windows runners) so one script works on both legs; - # the thing under test is the ownsharp/dotnet/python binaries, not the + # the thing under test is the owen/dotnet/python binaries, not the # shell driving them. shell: bash steps: @@ -1976,14 +1979,31 @@ jobs: # published there yet, see P-013's Non-goals) -- pack from the source # this job already checked out. Deliberately OUTSIDE the timed window # below: it is not part of the "install -> check" claim being proven. - - name: Pack OwnSharp.Cli (pulls in the extractor via ProjectReference) - run: dotnet pack frontend/roslyn/OwnSharp.Cli/OwnSharp.Cli.csproj -c Release -o "$RUNNER_TEMP/ownsharp-nupkg" - - name: A minimal leak, in a scratch dir OUTSIDE the repo + - name: Pack Owen.Cli (pulls in the extractor via ProjectReference) + run: dotnet pack frontend/roslyn/OwnSharp.Cli/OwnSharp.Cli.csproj -c Release -o "$RUNNER_TEMP/owen-nupkg" + # --add-source alone is not enough (Codex review, PR #244): `dotnet tool + # install` queries every configured source (nuget.org included) IN + # PARALLEL and takes whichever answers first — once a same-numbered + # version is ever actually on nuget.org, this install could silently + # resolve from there instead of the just-packed artifact. An isolated + # nuget.config with removes the ambiguity. + - name: Isolated NuGet.config — the packed artifact is the ONLY visible source + run: | + cat > "$RUNNER_TEMP/isolated-nuget.config" < + + + + + + + EOF + - name: Seed a leak sample, a clean sample, and an unsupported-input sample -- all OUTSIDE the repo # Proves the tool needs nothing but itself + Python -- not the Own.NET # checkout (a real end user obviously won't have this repo on disk). run: | - mkdir -p "$RUNNER_TEMP/ownsharp-sample" - cat > "$RUNNER_TEMP/ownsharp-sample/Leak.cs" <<'EOF' + mkdir -p "$RUNNER_TEMP/owen-sample" "$RUNNER_TEMP/owen-clean" "$RUNNER_TEMP/owen-unsupported" + cat > "$RUNNER_TEMP/owen-sample/Leak.cs" <<'EOF' using System.IO; public class Leaky { @@ -1994,19 +2014,90 @@ jobs: } } EOF + cat > "$RUNNER_TEMP/owen-clean/Clean.cs" <<'EOF' + using System.IO; + public class Tidy + { + public void Run() + { + using var s = new MemoryStream(); + s.WriteByte(1); + } + } + EOF + cat > "$RUNNER_TEMP/owen-unsupported/app.ts" <<'EOF' + console.log("not C# -- the .NET/C# frontend is the only one wired in today"); + EOF - name: Start the clean-machine timer (install -> check -> findings) run: echo "SMOKE_START=$(date +%s)" >> "$GITHUB_ENV" - - name: dotnet tool install --global (the one install the user runs) - run: dotnet tool install --global OwnSharp.Cli --version 0.1.0 --add-source "$RUNNER_TEMP/ownsharp-nupkg" - - name: ownsharp check finds the leak + - name: dotnet tool install --global (the one install the user runs) -- pins the package ID + run: dotnet tool install --global Owen.Cli --version 0.1.0 --configfile "$RUNNER_TEMP/isolated-nuget.config" + - name: owen --help -- language-neutral product framing, explicit included-frontend list, no TypeScript claim + run: | + out=$(owen --help) + echo "$out" + echo "$out" | grep -qi "^owen " || { echo "FAIL: --help should open with the owen product line"; exit 1; } + echo "$out" | grep -q "Included frontend" || { echo "FAIL: --help should list the included frontend explicitly"; exit 1; } + echo "$out" | grep -q '\.cs, \.csproj, \.sln' || { echo "FAIL: --help should name the .NET/C# frontend's extensions"; exit 1; } + if echo "$out" | grep -qi "typescript"; then + echo "FAIL: --help must not claim TypeScript support before it's actually wired in"; exit 1 + fi + - name: owen --version -- pins the command name via PATH resolution alone + run: | + out=$(owen --version) + [ -n "$out" ] || { echo "FAIL: --version printed nothing"; exit 1; } + echo "OK: owen --version -> $out" + - name: "owen -- prefix is owen:, not the pre-rebrand ownsharp:" + run: | + set +e + out=$(owen bogus-command 2>&1) + rc=$? + set -e + echo "$out" + [ "$rc" -ge 2 ] || { echo "FAIL: expected a non-zero exit for an unknown command, got $rc"; exit 1; } + echo "$out" | grep -q "^owen: unknown command" || { echo "FAIL: expected an 'owen: unknown command' prefix"; exit 1; } + - name: owen check finds the leak (installed execution, outside any checkout) run: | set +e - out=$(ownsharp check "$RUNNER_TEMP/ownsharp-sample" --fail-on-finding 2>&1) + out=$(owen check "$RUNNER_TEMP/owen-sample" --fail-on-finding 2>&1) rc=$? set -e echo "$out" [ "$rc" -eq 1 ] || { echo "FAIL: expected exit 1 (findings), got $rc"; exit 1; } echo "$out" | grep -q "OWN001" || { echo "FAIL: expected OWN001 in the output"; exit 1; } + - name: owen check on clean code exits 0 (negative control) + run: | + set +e + out=$(owen check "$RUNNER_TEMP/owen-clean" --fail-on-finding 2>&1) + rc=$? + set -e + echo "$out" + [ "$rc" -eq 0 ] || { echo "FAIL: expected exit 0 on clean code, got $rc"; exit 1; } + - name: owen check --format sarif -- Owen-branded SARIF driver name + run: | + out=$(owen check "$RUNNER_TEMP/owen-sample" --format sarif) + echo "$out" | python3 -c " + import json, sys + d = json.load(sys.stdin) + n = d['runs'][0]['tool']['driver']['name'] + assert n == 'Owen', f'SARIF driver name is {n!r}, expected Owen' + print('OK: SARIF driver name is Owen') + " + - name: owen check on unsupported input fails explicitly -- never a silent clean scan + run: | + set +e + out=$(owen check "$RUNNER_TEMP/owen-unsupported" 2>&1) + rc=$? + set -e + echo "$out" + [ "$rc" -eq 4 ] || { echo "FAIL: expected exit 4 (no supported input), got $rc"; exit 1; } + echo "$out" | grep -qi "no supported input" || { echo "FAIL: expected an explicit unsupported-input message"; exit 1; } + # The exact phrase a genuinely clean C# scan would print (own-check's + # core render, e.g. "0 findings.") must NOT appear here -- that would + # mean unsupported input silently looked like a successful clean scan. + if echo "$out" | grep -q "^0 findings\.$"; then + echo "FAIL: must not report a clean 0-findings scan for unsupported input"; exit 1 + fi - name: Stop the timer -- report it, and gate on a generous regression ceiling # A hard ceiling, not a precision claim: CI timing varies with runner # load, so this is a regression guard (catch "it now takes 20 minutes"), @@ -2015,14 +2106,26 @@ jobs: elapsed=$(( $(date +%s) - SMOKE_START )) echo "install -> check -> findings: ${elapsed}s" [ "$elapsed" -lt 240 ] || { echo "FAIL: took ${elapsed}s (ceiling 240s) — see alpha-readiness.md gate A"; exit 1; } - - name: No Python found -> a fast, actionable failure (never an auto-download) + - name: No Python found via OWEN_PYTHON -> a fast, actionable failure (never an auto-download) run: | set +e - out=$(OWN_PYTHON=/definitely/does/not/exist/python3 ownsharp check "$RUNNER_TEMP/ownsharp-sample" 2>&1) + out=$(OWEN_PYTHON=/definitely/does/not/exist/python3 owen check "$RUNNER_TEMP/owen-sample" 2>&1) rc=$? set -e echo "$out" [ "$rc" -eq 3 ] || { echo "FAIL: expected exit 3 (Python not found), got $rc"; exit 1; } - echo "$out" | grep -qi "OWN_PYTHON" || { echo "FAIL: expected the OWN_PYTHON-specific message"; exit 1; } + echo "$out" | grep -q "OWEN_PYTHON" || { echo "FAIL: expected the OWEN_PYTHON-specific message"; exit 1; } echo "$out" | grep -Eiq "winget|apt|brew|python.org" || { echo "FAIL: expected an actionable install hint"; exit 1; } + - name: Legacy OWN_PYTHON still works as a temporary fallback, with a deprecation note + run: | + own_python="$(command -v python3 || command -v python)" + [ -n "$own_python" ] || { echo "FAIL: could not find a python3/python on PATH to test the fallback with"; exit 1; } + set +e + out=$(OWN_PYTHON="$own_python" owen check "$RUNNER_TEMP/owen-sample" --fail-on-finding 2>&1) + rc=$? + set -e + echo "$out" + [ "$rc" -eq 1 ] || { echo "FAIL: expected exit 1 (findings, via the legacy var), got $rc"; exit 1; } + echo "$out" | grep -qi "OWN_PYTHON is deprecated" || { echo "FAIL: expected a deprecation note when OWN_PYTHON is used"; exit 1; } + echo "$out" | grep -q "OWN001" || { echo "FAIL: expected OWN001 in the output even via the legacy var"; exit 1; } diff --git a/README.md b/README.md index 459f6eac..2ad5b3f8 100644 --- a/README.md +++ b/README.md @@ -27,8 +27,9 @@ scripts/own-check.sh --format human -- /path/to/your/csharp/repo ``` Needs Python 3.11+ and the .NET SDK on `PATH` — nothing to build, nothing to -`pip install`. A packaged single-command CLI (`ownsharp check`) also exists — -build-and-install-locally today, not yet published to nuget.org; see +`pip install`. A packaged single-command CLI (`owen check`, package +`Owen.Cli`) also exists — build-and-install-locally today, not yet published +to nuget.org; see [`frontend/roslyn/OwnSharp.Cli/README.md`](frontend/roslyn/OwnSharp.Cli/README.md) and [`docs/notes/alpha-readiness.md`](docs/notes/alpha-readiness.md) gate **A**. @@ -921,7 +922,7 @@ ownlang/ test_ownir.py # the OwnIR bridge: C# facts -> core -> OWN001 at the C# site frontend/roslyn/ # the C# extractor (Roslyn, CI-only) + .cs samples (P-001) OwnSharp.Extractor/ # ownsharp-extract (dotnet tool): facts only - OwnSharp.Cli/ # ownsharp (dotnet tool, gate A): extractor + vendored core, one install + OwnSharp.Cli/ # owen / Owen.Cli (dotnet tool, gate A): extractor + vendored core, one install rust/ # the Rust core migration (P-022): own-ir + own-syntax so far, # oracle-gated against this Python core — see rust/README.md pyproject.toml # gate: ruff + mypy --strict (see below) diff --git a/README.ru.md b/README.ru.md index ec3badef..5160ed94 100644 --- a/README.ru.md +++ b/README.ru.md @@ -28,8 +28,9 @@ scripts/own-check.sh --format human -- /путь/к/вашему/csharp/репо ``` Нужны Python 3.11+ и .NET SDK в `PATH` — ничего собирать, ничего ставить через -`pip install`. Есть и упакованный однокомандный CLI (`ownsharp check`) — сегодня -собирается и ставится локально, в nuget.org ещё не опубликован; см. +`pip install`. Есть и упакованный однокомандный CLI (`owen check`, пакет +`Owen.Cli`) — сегодня собирается и ставится локально, в nuget.org ещё не +опубликован; см. [`frontend/roslyn/OwnSharp.Cli/README.md`](frontend/roslyn/OwnSharp.Cli/README.md) и [`docs/notes/alpha-readiness.md`](docs/notes/alpha-readiness.md), gate **A**. diff --git a/action.yml b/action.yml index 23d1113e..89c62e12 100644 --- a/action.yml +++ b/action.yml @@ -1,4 +1,4 @@ -name: "Own.NET resource-leak check" +name: "Owen lifetime/resource check" description: >- Scan C# for lifetime/resource leaks the compiler cannot express — event/timer subscription leaks, undisposed IDisposable fields/locals, ignored Subscribe() @@ -29,7 +29,7 @@ inputs: required: false default: "true" python-version: - description: "Python version for the Own.NET core." + description: "Python version for the Owen core." required: false default: "3.13" dotnet-version: @@ -39,7 +39,7 @@ inputs: sarif-file: description: >- Where to write the SARIF log when format: sarif (default: - $RUNNER_TEMP/own-net.sarif). The chosen path is echoed back as the + $RUNNER_TEMP/owen.sarif). The chosen path is echoed back as the sarif-file output regardless. required: false default: "" @@ -54,7 +54,7 @@ outputs: runs: using: "composite" steps: - - name: Set up Python (Own.NET core) + - name: Set up Python (Owen core) uses: actions/setup-python@v5 with: python-version: ${{ inputs.python-version }} @@ -64,7 +64,7 @@ runs: with: dotnet-version: ${{ inputs.dotnet-version }} - - name: Own.NET leak check + - name: Owen leak check id: own shell: bash env: @@ -88,16 +88,16 @@ runs: # passed so the true exit code (0 clean / 1 findings / >=2 hard error) is # captured *after* the file is written; whether a finding fails the step # is the action's own fail-on-finding (default: let code scanning gate). - sarif="${OWN_SARIF_FILE:-$RUNNER_TEMP/own-net.sarif}" + sarif="${OWN_SARIF_FILE:-$RUNNER_TEMP/owen.sarif}" set +e "$check" --root "${{ github.action_path }}" --format sarif \ --severity "$OWN_SEVERITY" --fail-on-finding -- "$OWN_PATH" > "$sarif" rc=$? set -e echo "sarif-file=$sarif" >> "$GITHUB_OUTPUT" - echo "Own.NET wrote SARIF to $sarif ($(wc -c < "$sarif" 2>/dev/null || echo 0) bytes; own-check rc=$rc)" + echo "Owen wrote SARIF to $sarif ($(wc -c < "$sarif" 2>/dev/null || echo 0) bytes; own-check rc=$rc)" if [ "$rc" -ge 2 ]; then - echo "::error::Own.NET hard error (bad facts / drifted contract)" + echo "::error::Owen hard error (bad facts / drifted contract)" cat "$sarif" >&2 || true exit "$rc" fi @@ -107,7 +107,7 @@ runs: # fails the redirection (exit 1, outside own-check's 0/1/>=2 contract). # Surface it; never let it pass as a gated-off finding with no SARIF. if [ ! -s "$sarif" ]; then - echo "::error::Own.NET produced no SARIF at $sarif (own-check rc=$rc) — the run or its redirection failed" + echo "::error::Owen produced no SARIF at $sarif (own-check rc=$rc) — the run or its redirection failed" exit 1 fi if [ "$OWN_FAIL_ON_FINDING" = "true" ] && [ "$rc" -eq 1 ]; then diff --git a/docs/notes/alpha-readiness.md b/docs/notes/alpha-readiness.md index 8ab90640..ba6f290a 100644 --- a/docs/notes/alpha-readiness.md +++ b/docs/notes/alpha-readiness.md @@ -36,7 +36,7 @@ The bar for "showable": a person can reproduce the wow in ~3 minutes | | Item | Status (2026-06-27) | Gap to close | |---|------|--------------------|--------------| -| **A** | `dotnet tool` one-command CLI | ◑ **mostly built** (issue #202) — `OwnSharp.Cli` wraps extractor+core into one `dotnet tool install` → `ownsharp check `, proven install→check→findings on a clean ubuntu/windows runner in CI (`ownsharp-cli-smoke`). See [`frontend/roslyn/OwnSharp.Cli/README.md`](../../frontend/roslyn/OwnSharp.Cli/README.md). | Not published to nuget.org yet — today it's build-and-install-from-source only. Publishing (+ a real version scheme beyond `0.1.0`) is the remaining step. | +| **A** | `dotnet tool` one-command CLI | ◑ **mostly built** (issue #202) — `OwnSharp.Cli` (public facade: package `Owen.Cli`, command `owen`) wraps extractor+core into one `dotnet tool install` → `owen check `, proven install→check→findings on a clean ubuntu/windows runner in CI (`ownsharp-cli-smoke`). See [`frontend/roslyn/OwnSharp.Cli/README.md`](../../frontend/roslyn/OwnSharp.Cli/README.md). | Not published to nuget.org yet — today it's build-and-install-from-source only. Publishing (+ a real version scheme beyond `0.1.0`) is the remaining step. | | **B** | GitHub Action | ✅ **built** — `action.yml`: `path`/`severity`/`format` (`github` / `msbuild` / `human` / `sarif`), purple shield branding. Matches the "stupidly simple YAML" bar. | Publish to Marketplace; pin the 6-line usage in the README. | | **C** | SARIF / PR annotations | ✅ **built** — SARIF 2.1.0 + GitHub annotations + reachability/evidence (P-015). | — | | **D** | 5 core diagnostics | ✅ **built, well past** — OWN001/002/003, OWN014, DI001–005, POOL001–005, WPF001–005 (catalog). The comment's `SUB001/SUB002/TMR001/DISP001/DI001` all exist *semantically*; the `SUB/TMR/DISP` catalog rename is the deferred consolidation item, not new work. | (naming only) land the catalog rename with the OwnIR-v1/profile-label work. | @@ -58,13 +58,13 @@ suppression → "why not Sonar/CodeQL"), every step of that path now exists. front door, and now A have all closed too — what's left is publishing, not building:** -1. ~~a single `ownsharp check MyApp.sln` tool (**A**)~~ — **built**, not yet published to nuget.org; +1. ~~a single `owen check MyApp.sln` tool (**A**)~~ — **built**, not yet published to nuget.org; 2. ~~a wedge landing README + copy-paste quickstart (front door)~~ — **done**; 3. ~~three packaged case studies from finds we already have (**F**)~~ — **done**; 4. ~~one consolidated suppression / false-positive page (**G**)~~ — **done**. None of those is research; all are the difference between "interesting PoC" and -"people install it." Publishing `OwnSharp.Cli` to nuget.org is now the one item +"people install it." Publishing `Owen.Cli` to nuget.org is now the one item standing between here and the day 1–30 milestone being *literally* copy-paste for a stranger. @@ -87,7 +87,7 @@ until the .NET alpha above is delicious. Do not let the spike exceed 20%. ## 90-day shape (sequencing, not a schedule) -- **Days 1–30 — make the .NET alpha tasty:** publish `OwnSharp.Cli` to +- **Days 1–30 — make the .NET alpha tasty:** publish `Owen.Cli` to nuget.org (A/B/C/D/E/F/G and the README front door are all otherwise done). Suppression UX + bad/ok corpus polish continue as bug-driven follow-ups, not a blocking gate. diff --git a/docs/notes/owen-public-facade.md b/docs/notes/owen-public-facade.md new file mode 100644 index 00000000..7f779c8a --- /dev/null +++ b/docs/notes/owen-public-facade.md @@ -0,0 +1,145 @@ +# Owen: public facade rebrand + +This note records a **public-facing rebrand**, not an internal refactor. The +underlying engine, project names, namespaces, and diagnostic codes are all +unchanged; only what an external user *sees* — the package name, the CLI +command, help/error text, the SARIF tool identity, the cache directory, and +the Action's display name — changed to the public identity **Owen**. + +## Why + +The project is language-neutral at the OwnIR/core level (`ownlang/` takes +facts from any frontend that can produce them) and may eventually ship a +TypeScript frontend alongside the current C# one. Publishing the first real +package under a C#-specific name (`OwnSharp.Cli`, command `ownsharp`) would +have locked the public identity to a single-language framing the project +doesn't actually have. **Owen** is the product name; **this distribution +currently includes the .NET/C# frontend only** — that framing is stated +explicitly in the CLI's own `--help` output rather than left implicit. + +## Public identity implemented + +| Surface | Old | New | +|---|---|---| +| Product | (unnamed / "Own.NET" informally) | **Owen** | +| NuGet package ID | `OwnSharp.Cli` | **`Owen.Cli`** (confirmed unclaimed on nuget.org at time of writing — both `owen` and `owen.cli` returned 404 from the v3 flat-container API; still worth a final check immediately before an actual publish, since availability can change) | +| Tool command | `ownsharp` | **`owen`** | +| Action display name | `Own.NET resource-leak check` | **`Owen lifetime/resource check`** | +| SARIF `tool.driver.name` | `Own.NET` | **`Owen`** | +| Cache directory | `~/.ownsharp/core//` | **`~/.owen/core//`** | +| Preferred Python env var | `OWN_PYTHON` | **`OWEN_PYTHON`** (`OWN_PYTHON` still works, deprecated) | +| Default Action SARIF filename | `own-net.sarif` | **`owen.sarif`** | + +## What deliberately did NOT change (internal names) + +Per the guardrail this rebrand was scoped to: no mass rename. + +- The `OwnSharp.Cli` **project and namespace** — still `OwnSharp.Cli` in the + `.csproj`, C# namespace, and `.sln`. Only `PackageId` and + `ToolCommandName` (the two properties that actually control the public + package/command identity) changed. +- `AssemblyName` in `OwnSharp.Cli.csproj` stays `ownsharp` — it names the + internal `.dll` the `owen` shim launches, never typed or seen by a user, + so renaming it would add no user value (the csproj comment says so + explicitly, next to `ToolCommandName`). +- **`OwnSharp.Extractor`** (project, namespace, and its real output filename + `ownsharp-extract.dll`) — completely untouched. `CheckCommand.cs` + references that literal filename because it is the actual file that ships, + not a stale pre-rebrand reference. +- **`ownlang`** — the Python package name, its module names, its CLI + (`python -m ownlang ...`), and its `PYTHONPATH`/working-directory + conventions are all unchanged. Only the `"name"` string value inside the + SARIF `tool.driver` object (in `ownlang/ownir.py` and + `ownlang/diag_sarif.py`) changed from the literal `"Own.NET"` to `"Owen"` + — a metadata string, not a rename of anything importable. +- **`OwnIR`**, **`OWN001`** and every other diagnostic code, the Rust crates + under `rust/`, every `frontend/*` directory name, and all historical + docs/issues — untouched. This note does not retroactively edit history; + older notes that say "Own.NET" or "ownsharp" describe what was true when + they were written. +- `scripts/own-check.sh`/`.ps1` and `action.yml`'s internal call to + `scripts/own-check.sh` — unchanged. The Action's public *display name* and + *default SARIF filename* changed; what it runs under the hood did not. +- The GitHub repository itself (`PhysShell/Own.NET`) — not renamed in this + PR. `_SARIF_INFO_URI` in `ownlang/ownir.py`/`diag_sarif.py` still points at + `https://github.com/PhysShell/Own.NET`, which remains accurate. +- `audit/` and `scripts/{oracle_compare,mine_report}.py` still construct + synthetic test fixtures with a literal `"Own.NET"` driver name in a few + places. These are **test input fixtures** for those tools' own aggregation/ + comparison logic (arbitrary strings a fixture author chose), not assertions + about Owen's real emitted SARIF — `audit/` is explicitly documented + (`audit/README.md`, `AGENTS.md`) as decoupled from `ownlang` and consuming + `own-check` only through its CLI/SARIF surface, with active development + living in a separate repo. Left alone to avoid scope creep into a module + this PR has no reason to touch. + +## CLI contract additions + +- **`owen check `** is the public invocation. `--help` is + language-neutral at the product level ("Owen finds lifetime and + resource-contract bugs") while explicitly listing what this distribution + actually wires up today: + ``` + Included frontend: + .NET / C# (.cs, .csproj, .sln) + ``` + No plugin framework and no speculative TypeScript mention were added — + the help text does not claim support that doesn't exist yet. +- **Unsupported input fails explicitly.** Before this rebrand, pointing + `check` at a `.ts` file or an empty/non-C# directory silently printed + "0 findings" — indistinguishable from a genuinely clean C# scan. + `CheckCommand.HasSupportedInput` now checks every given path resolves to + something the included frontend can read (a `.cs`/`.csproj`/`.sln` file, + or a directory containing at least one `.cs` file anywhere under it) + *before* running the extractor, and exits **4** with an explicit message + if none do. This is a new, additive exit-code tier — it does not change + any of the existing 0/1/`>=2`/3 contract. +- **`OWEN_PYTHON`** is the preferred env var; the legacy **`OWN_PYTHON`** + name is still accepted as a temporary compatibility fallback (so existing + internal use — CI, scripts, muscle memory — doesn't break outright) and + prints a one-line deprecation note to stderr every time it's the variable + actually used to resolve Python. `OWEN_PYTHON` takes priority when both + are set. +- **Cache directory** moved to `~/.owen/core//`. `CoreVendor` reads + a plain existence check against the previous `~/.ownsharp/core//` + location first — if that exact version was already unpacked there by an + older install, it's reused in place rather than re-copied. This is a + fallback read, not a migration subsystem: the old location is never + written to, moved, or deleted by the new code. + +## Tests + +`ci.yml`'s `ownsharp-cli-smoke` job (job key kept as-is; only its *content* +changed — renaming the key isn't part of the public facade and would just +add unrelated churn) now pins, on both `ubuntu-latest` and `windows-latest`: +package ID (`Owen.Cli`), command name (`owen`), `--help` (Owen framing + +explicit included-frontend list + no TypeScript claim), `--version`, +unknown-command prefix (`owen:`), a leak sample and a clean negative control +both run from an installed, checkout-free location, the `Owen` SARIF driver +name, the exit-4 unsupported-input path (explicitly asserting the output +does **not** contain the literal clean-scan phrase `0 findings.`), +`OWEN_PYTHON`'s not-found path, and the legacy `OWN_PYTHON` fallback +(resolved successfully + its deprecation note). Two other pre-existing +`ci.yml` assertions that pinned the literal SARIF driver-name string +(`own-check-codescan`'s local structural-validation job, and +`tests/test_diag_sarif.py`/`tests/test_ownir.py`) were updated from +`"Own.NET"` to `"Owen"` to match — these are follow-on fixes made necessary +by the driver-name change, not scope creep; they were caught by +`python tests/run_tests.py` failing before the fix. + +## PR separation + +This is PR 1 of 3 for the release-readiness work: + +1. **This PR** — the public facade rebrand (Owen identity, no publish). +2. Phase 3 (NuGet release/package pipeline) — rebuilt against this PR once + merged, targeting `Owen.Cli`/`owen` instead of the pre-rebrand + `OwnSharp.Cli`/`ownsharp` identity its first draft (PR #244) used. +3. Phase 4 (GitHub Marketplace preparation) — rebuilt against this PR once + merged, targeting the `Owen lifetime/resource check` display name instead + of the pre-rebrand name its first draft (PR #245) used. + +No oracle remeasurement and no analyzer-semantics change are mixed into this +PR — the extractor, the core's detection logic, and every diagnostic +verdict are byte-for-byte unchanged; `python tests/run_tests.py`, `ruff`, +and `mypy` all stay green throughout. diff --git a/frontend/roslyn/OwnSharp.Cli/CheckCommand.cs b/frontend/roslyn/OwnSharp.Cli/CheckCommand.cs index 6bcbbfe3..c2ff138f 100644 --- a/frontend/roslyn/OwnSharp.Cli/CheckCommand.cs +++ b/frontend/roslyn/OwnSharp.Cli/CheckCommand.cs @@ -3,16 +3,26 @@ namespace OwnSharp.Cli; /// -/// `ownsharp check` — extract (bundled Roslyn extractor, in a child process) +/// `owen check` — extract (bundled Roslyn extractor, in a child process) /// -> facts.json -> the vendored core (system Python) -> render. Flags mirror /// scripts/own-check.sh 1:1; the exit-code contract is the same one (own-check /// comment): 0 clean, 1 findings, >=2 a hard error, plus --fail-on-finding. +/// Exit 4 is `owen`'s own: no path resolved to anything the currently +/// included frontend (.NET/C#) can analyze — see . /// internal static class CheckCommand { private static readonly HashSet ValidFormats = ["human", "github", "msbuild", "sarif"]; private static readonly HashSet ValidSeverities = ["error", "warning"]; + // The product (Owen) is language-neutral at the OwnIR/core level; this + // distribution currently wires up only the .NET/C# frontend. Naming the + // extensions here (instead of e.g. "just try everything and see") is + // what makes "unsupported input fails explicitly" possible at all -- a + // silent 0-finding scan on a .ts file or an empty directory would + // otherwise look identical to a genuinely clean C# scan. + private static readonly HashSet SupportedExtensions = [".cs", ".csproj", ".sln"]; + public static async Task RunAsync(string[] args) { string format; @@ -36,13 +46,13 @@ public static async Task RunAsync(string[] args) if (!ValidFormats.Contains(format)) { Console.Error.WriteLine( - $"ownsharp check: unknown --format '{format}' (choose: {string.Join(", ", ValidFormats)})"); + $"owen check: unknown --format '{format}' (choose: {string.Join(", ", ValidFormats)})"); return 2; } if (!ValidSeverities.Contains(severity)) { Console.Error.WriteLine( - $"ownsharp check: unknown --severity '{severity}' (choose: {string.Join(", ", ValidSeverities)})"); + $"owen check: unknown --severity '{severity}' (choose: {string.Join(", ", ValidSeverities)})"); return 2; } if (paths.Count == 0) @@ -50,6 +60,15 @@ public static async Task RunAsync(string[] args) paths.Add("."); } + if (!HasSupportedInput(paths, out var reason)) + { + Console.Error.WriteLine($"owen check: no supported input found — {reason}"); + Console.Error.WriteLine( + "Included frontend: .NET / C# (.cs, .csproj, .sln). " + + "This is not a clean scan: nothing was analyzed."); + return 4; + } + // Resolve Python FIRST: no point extracting facts just to fail on stage 2. ResolvedPython python; try @@ -134,23 +153,65 @@ private static string RequireValue(string[] args, ref int i, string flag) { if (i + 1 >= args.Length) { - throw new InvalidOperationException($"ownsharp check: {flag} requires a value"); + throw new InvalidOperationException($"owen check: {flag} requires a value"); } return args[++i]; } + /// True if at least one of resolves to + /// something the currently included frontend can analyze: a file whose + /// extension is in , or a directory that + /// recursively contains at least one .cs file. A path that doesn't + /// exist on disk at all is also unsupported (not silently skipped) -- the + /// point is to never let "nothing to analyze" print as "0 findings, + /// clean". + private static bool HasSupportedInput(IReadOnlyList paths, out string reason) + { + var problems = new List(); + foreach (var p in paths) + { + if (Directory.Exists(p)) + { + if (Directory.EnumerateFiles(p, "*.cs", SearchOption.AllDirectories).Any()) + { + reason = ""; + return true; + } + problems.Add($"'{p}' is a directory with no .cs files"); + } + else if (File.Exists(p)) + { + if (SupportedExtensions.Contains(Path.GetExtension(p))) + { + reason = ""; + return true; + } + problems.Add($"'{p}' has an unsupported extension ({Path.GetExtension(p)})"); + } + else + { + problems.Add($"'{p}' does not exist"); + } + } + reason = string.Join("; ", problems); + return false; + } + /// Stage 1: run the bundled extractor as a child process. All of its /// own output (build/run chatter, if any) goes to OUR stderr, keeping /// stdout clean for stage 2 — same as own-check.sh's `1>&2` on this stage. private static async Task RunExtractorAsync( IReadOnlyList paths, string factsPath, bool legacy, bool stats, bool bodyThrowEdges) { + // "ownsharp-extract.dll" is OwnSharp.Extractor's own real AssemblyName/output + // filename (internal project name, unchanged by the Owen public facade) — + // this is the file that actually ships, not a stale reference. var extractorDll = Path.Combine(AppContext.BaseDirectory, "ownsharp-extract.dll"); if (!File.Exists(extractorDll)) { Console.Error.WriteLine( - $"ownsharp: bundled extractor not found at '{extractorDll}' — a corrupt or " + - "incomplete tool install. Try `dotnet tool uninstall --global OwnSharp.Cli` and reinstall."); + $"owen: bundled extractor not found at '{extractorDll}' — a corrupt or " + + "incomplete tool install. Try `dotnet tool uninstall --global Owen.Cli` and reinstall."); return 2; } @@ -182,7 +243,7 @@ private static async Task RunExtractorAsync( } using var proc = Process.Start(psi) - ?? throw new InvalidOperationException("ownsharp: failed to start the extractor process"); + ?? throw new InvalidOperationException("owen: failed to start the extractor process"); var stdoutTask = proc.StandardOutput.ReadToEndAsync(); var stderrTask = proc.StandardError.ReadToEndAsync(); await proc.WaitForExitAsync().ConfigureAwait(false); @@ -199,8 +260,8 @@ private static async Task RunExtractorAsync( /// lookup is the reliable default; DOTNET_ROOT (set by some CI/sandboxed /// installs) is honored first when present. Deliberately NOT /// Process.GetCurrentProcess().MainModule — on Windows a `dotnet tool` - /// shim is a native apphost, so that would resolve to ownsharp.exe itself, - /// not the dotnet muxer. + /// shim is a native apphost, so that would resolve to owen.exe itself + /// (the ToolCommandName-based shim), not the dotnet muxer. private static string ResolveDotnetMuxer() { var root = Environment.GetEnvironmentVariable("DOTNET_ROOT"); @@ -245,7 +306,7 @@ private static async Task RunCoreAsync( psi.EnvironmentVariables["PYTHONPATH"] = cacheRoot; using var proc = Process.Start(psi) - ?? throw new InvalidOperationException("ownsharp: failed to start the Python core process"); + ?? throw new InvalidOperationException("owen: failed to start the Python core process"); await proc.WaitForExitAsync().ConfigureAwait(false); return proc.ExitCode; } diff --git a/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs b/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs index 52b3a31f..28c899d5 100644 --- a/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs +++ b/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs @@ -5,7 +5,12 @@ namespace OwnSharp.Cli; /// own nupkg under ownlang-core/ownlang/*.py, see the .csproj) into a /// stable, per-version cache directory outside the tool's own (versioned, /// nested) install path — and, per the design decision in issue #202, never -/// into the repository being analyzed. +/// into the repository being analyzed. Cache root renamed to +/// ~/.owen/core/<version>/ for the Owen public facade (see +/// docs/notes/owen-public-facade.md); a version already unpacked under the +/// previous ~/.ownsharp/core/<version>/ location is reused as-is +/// (a plain existence check, not a migration — the old location is never +/// written to or deleted). /// internal static class CoreVendor { @@ -17,10 +22,8 @@ internal static class CoreVendor /// public static string EnsureUnpacked() { - var cacheRoot = Path.Combine( - Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), - ".ownsharp", "core", ToolVersion.Current); - var destOwnlang = Path.Combine(cacheRoot, "ownlang"); + var userProfile = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); + var cacheRoot = Path.Combine(userProfile, ".owen", "core", ToolVersion.Current); var marker = Path.Combine(cacheRoot, ".unpacked"); if (File.Exists(marker)) @@ -28,12 +31,25 @@ public static string EnsureUnpacked() return cacheRoot; } + // Simple fallback read from the previous ~/.ownsharp location (same + // version already unpacked there by an older install of this same + // tool) — reuse it in place rather than re-copying, no migration + // subsystem. Only a same-version marker counts: a different version + // there is irrelevant (each version gets its own cache directory). + var legacyCacheRoot = Path.Combine(userProfile, ".ownsharp", "core", ToolVersion.Current); + var legacyMarker = Path.Combine(legacyCacheRoot, ".unpacked"); + if (File.Exists(legacyMarker)) + { + return legacyCacheRoot; + } + + var destOwnlang = Path.Combine(cacheRoot, "ownlang"); var sourceOwnlang = Path.Combine(AppContext.BaseDirectory, "ownlang-core", "ownlang"); if (!Directory.Exists(sourceOwnlang)) { throw new InvalidOperationException( - $"ownsharp: vendored core not found at '{sourceOwnlang}' — a corrupt or " + - "incomplete tool install. Try `dotnet tool uninstall --global OwnSharp.Cli` " + + $"owen: vendored core not found at '{sourceOwnlang}' — a corrupt or " + + "incomplete tool install. Try `dotnet tool uninstall --global Owen.Cli` " + "and reinstall."); } diff --git a/frontend/roslyn/OwnSharp.Cli/OwnSharp.Cli.csproj b/frontend/roslyn/OwnSharp.Cli/OwnSharp.Cli.csproj index da73dbbe..92880756 100644 --- a/frontend/roslyn/OwnSharp.Cli/OwnSharp.Cli.csproj +++ b/frontend/roslyn/OwnSharp.Cli/OwnSharp.Cli.csproj @@ -12,6 +12,14 @@ pieces (extractor -> core) the same way scripts/own-check.sh already does, so they ship as ONE `dotnet tool install`. + Public facade (docs/notes/owen-public-facade.md): the PROJECT stays + "OwnSharp.Cli" internally (not mass-renamed — that would be an + internal refactor, not what this is), but the PUBLISHED package ID, + command name, and all user-facing output are "Owen"/"owen". Product + "Owen" is language-neutral at the OwnIR/core level; this NuGet + package is specifically the .NET/C# frontend distribution of it — + see Program.cs's help text for the exact honest framing. + Packaging shape (design decision recorded in issue #202): - The Roslyn extractor (OwnSharp.Extractor, unmodified, P-013) is pulled in via ProjectReference below; its build output (dll + @@ -23,16 +31,22 @@ a bundled binary instead of running the extractor from source. - The Python core (ownlang/, zero-dependency pure Python, see ../../../pyproject.toml) is vendored as loose *.py content below - and unpacked to ~/.ownsharp/core// on first run; it is - executed by the machine's own Python (>=3.11), never embedded or - compiled. See Program.cs for the resolution/unpack/fail-fast - logic and the rejected alternatives on record in issue #202. + and unpacked to ~/.owen/core// on first run (falling back + to a previous ~/.ownsharp/core// if already unpacked + there — see CoreVendor.cs); it is executed by the machine's own + Python (>=3.11), never embedded or compiled. See Program.cs for + the resolution/unpack/fail-fast logic and the rejected + alternatives on record in issue #202. --> true - ownsharp - OwnSharp.Cli + + owen + Owen.Cli 0.1.0 - Own.NET's single command: `ownsharp check <path|.sln>` wraps the Roslyn extractor and the Python core (run on system Python) into one dotnet tool install. + Owen finds lifetime and resource-contract bugs. This distribution currently includes the .NET/C# frontend: `owen check <path|.sln>` wraps the Roslyn extractor and the Python core (run on system Python) into one dotnet tool install. diff --git a/frontend/roslyn/OwnSharp.Cli/Program.cs b/frontend/roslyn/OwnSharp.Cli/Program.cs index 56d7a76f..75b7fa4b 100644 --- a/frontend/roslyn/OwnSharp.Cli/Program.cs +++ b/frontend/roslyn/OwnSharp.Cli/Program.cs @@ -1,6 +1,9 @@ -// ownsharp — the single command for alpha gate A (issue #202). +// owen — the single command for alpha gate A (issue #202), public facade +// "Owen" (see docs/notes/owen-public-facade.md). The underlying project/ +// namespace stays OwnSharp.Cli internally -- this is a public-facing rename, +// not an internal refactor. // -// `ownsharp check ` wraps the two existing pipeline stages — +// `owen check ` wraps the two existing pipeline stages — // extractor -> core — into one `dotnet tool install`. This file only does // verb dispatch; the real work is in CheckCommand.cs / PythonResolver.cs / // CoreVendor.cs. No analysis logic lives here or anywhere in this project: @@ -22,20 +25,28 @@ if (args[0] != "check") { - Console.Error.WriteLine($"ownsharp: unknown command '{args[0]}'"); + Console.Error.WriteLine($"owen: unknown command '{args[0]}'"); Console.Error.WriteLine(HelpText()); return 2; } return await CheckCommand.RunAsync(args[1..]).ConfigureAwait(false); +// Product framing is deliberately language-neutral (Owen finds lifetime and +// resource-contract bugs; the OwnIR/core layer is not C#-specific) while the +// "Included frontend" line is explicit about what THIS distribution actually +// wires up today -- no plugin framework, no speculative TypeScript claim. static string HelpText() => """ - ownsharp — find lifetime/resource bugs in C# (Own.NET) + owen — finds lifetime and resource-contract bugs. + This distribution currently includes the .NET/C# frontend. + + Included frontend: + .NET / C# (.cs, .csproj, .sln) Usage: - ownsharp check [more paths...] [options] - ownsharp --version - ownsharp --help + owen check [more paths...] [options] + owen --version + owen --help Options (mirrors scripts/own-check.sh): --format {human|github|msbuild|sarif} finding surface (default: human) @@ -46,7 +57,11 @@ ownsharp check [more paths...] [options] --stats print flow-locals coverage to stderr --body-throw-edges opt-in: flag body-level (no-try) dispose-not-called-on-throw - Python: resolved via OWN_PYTHON, else `py -3` (Windows) / `python3` - (elsewhere); must be >=3.11. No auto-install — see the error message if - none is found. + Python: resolved via OWEN_PYTHON (OWN_PYTHON is a deprecated, temporary + fallback), else `py -3` (Windows) / `python3` (elsewhere); must be + >=3.11. No auto-install — see the error message if none is found. + + Input that doesn't match the included frontend (e.g. no .cs/.csproj/.sln + found anywhere given) fails explicitly (exit 4) rather than reporting a + clean scan. """; diff --git a/frontend/roslyn/OwnSharp.Cli/PythonResolver.cs b/frontend/roslyn/OwnSharp.Cli/PythonResolver.cs index a59cb9cf..d78c5d2a 100644 --- a/frontend/roslyn/OwnSharp.Cli/PythonResolver.cs +++ b/frontend/roslyn/OwnSharp.Cli/PythonResolver.cs @@ -13,9 +13,13 @@ internal sealed class PythonNotFoundException(string message) : Exception(messag internal sealed record ResolvedPython(string FileName, IReadOnlyList LeadingArgs); /// -/// Resolution order (design decision, issue #202): OWN_PYTHON env var -/// (used exactly as given, no fallback if it doesn't work — an explicit -/// override that fails is a configuration error, not a "keep guessing" case), +/// Resolution order (design decision, issue #202; env var renamed for the +/// Owen public facade, see docs/notes/owen-public-facade.md): +/// OWEN_PYTHON env var (used exactly as given, no fallback if it +/// doesn't work — an explicit override that fails is a configuration error, +/// not a "keep guessing" case); else the legacy OWN_PYTHON name (a +/// temporary compatibility fallback so existing internal use doesn't break — +/// prints a deprecation note to stderr whenever it's the one actually used); /// else the platform default (py -3 on Windows, python3 /// elsewhere). No auto-download, ever: a miss is a fast, actionable failure. /// @@ -26,19 +30,39 @@ internal static class PythonResolver public static ResolvedPython Resolve() { - var ownPython = Environment.GetEnvironmentVariable("OWN_PYTHON"); - if (!string.IsNullOrWhiteSpace(ownPython)) + var owenPython = Environment.GetEnvironmentVariable("OWEN_PYTHON"); + if (!string.IsNullOrWhiteSpace(owenPython)) { - var candidate = new ResolvedPython(ownPython, Array.Empty()); + var candidate = new ResolvedPython(owenPython, Array.Empty()); if (TryGetVersion(candidate, out var version) && IsSupported(version)) { return candidate; } throw new PythonNotFoundException( - $"ownsharp: OWN_PYTHON='{ownPython}' did not resolve to Python >={MinMajor}.{MinMinor} " + + $"owen: OWEN_PYTHON='{owenPython}' did not resolve to Python >={MinMajor}.{MinMinor} " + $"(found: {version ?? "not runnable"}). {InstallHint()}"); } + // Legacy fallback (temporary): OWN_PYTHON predates the Owen public + // facade and some existing/internal use still sets it. Honored so + // that doesn't silently break, but flagged every time it's the + // variable actually used, so it doesn't quietly become permanent. + var legacyOwnPython = Environment.GetEnvironmentVariable("OWN_PYTHON"); + if (!string.IsNullOrWhiteSpace(legacyOwnPython)) + { + var candidate = new ResolvedPython(legacyOwnPython, Array.Empty()); + if (TryGetVersion(candidate, out var version) && IsSupported(version)) + { + Console.Error.WriteLine( + "owen: OWN_PYTHON is deprecated — set OWEN_PYTHON instead (OWN_PYTHON is a " + + "temporary compatibility fallback and may be removed in a future release)."); + return candidate; + } + throw new PythonNotFoundException( + $"owen: OWN_PYTHON='{legacyOwnPython}' did not resolve to Python >={MinMajor}.{MinMinor} " + + $"(found: {version ?? "not runnable"}). {InstallHint()} (OWN_PYTHON is deprecated — use OWEN_PYTHON.)"); + } + var defaultCandidate = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? new ResolvedPython("py", ["-3"]) : new ResolvedPython("python3", Array.Empty()); @@ -64,8 +88,8 @@ public static ResolvedPython Resolve() } throw new PythonNotFoundException( - $"ownsharp: no Python >={MinMajor}.{MinMinor} found on PATH. {InstallHint()} " + - "(or set OWN_PYTHON to an interpreter's path)."); + $"owen: no Python >={MinMajor}.{MinMinor} found on PATH. {InstallHint()} " + + "(or set OWEN_PYTHON to an interpreter's path)."); } private static bool IsSupported(string? version) diff --git a/frontend/roslyn/OwnSharp.Cli/README.md b/frontend/roslyn/OwnSharp.Cli/README.md index 7c1deaf2..2a898297 100644 --- a/frontend/roslyn/OwnSharp.Cli/README.md +++ b/frontend/roslyn/OwnSharp.Cli/README.md @@ -1,8 +1,16 @@ -# ownsharp — the single command (alpha gate A, issue #202) - -`ownsharp check ` wraps the two existing pipeline stages — -the Roslyn extractor (`OwnSharp.Extractor`, P-013) and the Python core -(`ownlang/`) — into **one `dotnet tool install`**. Same pipeline +# owen — the single command (alpha gate A, issue #202) + +Public facade: **Owen**, package **Owen.Cli**, command **`owen`**. The +project/namespace stay `OwnSharp.Cli` internally — this is a public-facing +rename, not an internal refactor; see +[`docs/notes/owen-public-facade.md`](../../../docs/notes/owen-public-facade.md) +for the full rationale and what did/didn't change. + +Owen finds lifetime and resource-contract bugs. It is language-neutral at the +OwnIR/core level; **this distribution currently includes the .NET/C# +frontend only** — `owen check ` wraps the two existing +pipeline stages — the Roslyn extractor (`OwnSharp.Extractor`, P-013) and the +Python core (`ownlang/`) — into **one `dotnet tool install`**. Same pipeline [`scripts/own-check.sh`](../../../scripts/own-check.sh) already chains by hand; this is that, packaged. @@ -16,17 +24,24 @@ hand; this is that, packaged. build output (dll + `.deps.json`/`.runtimeconfig.json` + Roslyn dependencies) rides along in this tool's own pack payload because `PackAsTool` packs the full publish closure. `check` invokes it as a child - process (`dotnet exec /ownsharp-extract.dll ...`). + process (`dotnet exec /ownsharp-extract.dll ...` — the extractor's + own internal filename, unaffected by the public facade). - **The core is unmodified**, vendored as loose `*.py` content (see the - `.csproj`) and unpacked to `~/.ownsharp/core//` on first run — - never into the analyzed repo. It runs on the machine's own Python; nothing - is embedded, compiled, or downloaded. -- **Python resolution**: `OWN_PYTHON` env var (used exactly as given, no + `.csproj`) and unpacked to `~/.owen/core//` on first run (falling + back to a previous `~/.ownsharp/core//` if already unpacked there + by an older install — a plain reuse, not a migration) — never into the + analyzed repo. It runs on the machine's own Python; nothing is embedded, + compiled, or downloaded. +- **Python resolution**: `OWEN_PYTHON` env var (used exactly as given, no fallback — an explicit override that fails is a config error, not a - "keep guessing" case), else `py -3` (Windows) / `python3` (elsewhere), - version-checked to be `>=3.11`. No Python found → a fast, one-line, - actionable failure (`winget`/`apt`/`brew`/python.org, per OS) — **never** - an auto-download. + "keep guessing" case); `OWN_PYTHON` is honored as a temporary, deprecated + fallback (prints a note to stderr when it's the one actually used); else + `py -3` (Windows) / `python3` (elsewhere), version-checked to be `>=3.11`. + No Python found → a fast, one-line, actionable failure + (`winget`/`apt`/`brew`/python.org, per OS) — **never** an auto-download. +- **Unsupported input fails explicitly**: a path that isn't a `.cs`/`.csproj`/ + `.sln` file and isn't a directory containing any `.cs` file exits 4 with an + explicit message — never a silent "0 findings" clean scan. - **Rejected alternatives** (embedding a CPython runtime, self-contained PyInstaller binaries as the default, waiting for the Rust core, porting the core to C#) are on the record in the issue; do not re-litigate them here. @@ -37,15 +52,15 @@ Not published to nuget.org yet (P-013's Non-goals) — build and install from source: ```bash -dotnet pack frontend/roslyn/OwnSharp.Cli/OwnSharp.Cli.csproj -c Release -o /tmp/ownsharp-nupkg -dotnet tool install --global OwnSharp.Cli --version 0.1.0 --add-source /tmp/ownsharp-nupkg +dotnet pack frontend/roslyn/OwnSharp.Cli/OwnSharp.Cli.csproj -c Release -o /tmp/owen-nupkg +dotnet tool install --global Owen.Cli --version 0.1.0 --add-source /tmp/owen-nupkg -ownsharp check MyApp.sln # human output -ownsharp check . --format github --fail-on-finding # PR annotations, non-zero on a leak -ownsharp check . --format sarif > own.sarif # feed github/codeql-action/upload-sarif +owen check MyApp.sln # human output +owen check . --format github --fail-on-finding # PR annotations, non-zero on a leak +owen check . --format sarif > owen.sarif # feed github/codeql-action/upload-sarif ``` -Uninstall/upgrade: `dotnet tool uninstall --global OwnSharp.Cli`, then reinstall +Uninstall/upgrade: `dotnet tool uninstall --global Owen.Cli`, then reinstall as above (bump `--version` if you rebuilt with a new ``). ## Flags (mirror `scripts/own-check.sh` 1:1) @@ -60,29 +75,22 @@ as above (bump `--version` if you rebuilt with a new ``). | `--stats` | off | print flow-locals coverage to stderr | | `--body-throw-edges` | off | opt-in: flag body-level (no-`try`) dispose-not-called-on-throw | -Exit codes (same contract as `own-check.sh`/`.ps1`): the extractor stage's own -exit code propagates on a hard failure there; otherwise `0` clean / `1` -findings (only surfaced when `--fail-on-finding`) / `>=2` a core hard error -(bad facts, a drifted contract) always propagates; `3` is `ownsharp`'s own — -no usable Python was found. - -## Guardrails this project honors (no behaviour change, packaging only) - -- **No changes to `OwnSharp.Extractor`** — it is referenced, not edited. -- **No changes to `ownlang/`** — vendored byte-identical; "one checker" holds - literally, since the exact same core source renders every verdict. -- **`scripts/own-check.sh`/`.ps1` and `action.yml` are untouched** and keep - working exactly as before — this tool is a third surface alongside them, not - a replacement (P-013 §Scope). +Exit codes: `0` clean, `1` findings (only with `--fail-on-finding`), `>=2` a +core hard error (bad facts, a drifted contract), `3` no usable Python found, +`4` no supported input found (nothing matching the included frontend). ## CI proof `ownsharp-cli-smoke` in `.github/workflows/ci.yml` (matrix: `ubuntu-latest` + `windows-latest`) proves, on a clean runner: pack → `dotnet tool install ---global` → `ownsharp check` finds a real leak (`--fail-on-finding` exits 1, -`OWN001` in the output) → the timed install-to-findings window stays under a -regression ceiling → the no-Python path fails fast with the actionable -message. Both platforms matter here specifically, not just "more coverage": a -`dotnet tool` shim is a native apphost on Windows and a shell script on Unix — -genuinely different process-launch mechanics, so ubuntu-only would not have -proven the Windows path. +--global` → `owen --help`/`--version`/unknown-command → `owen check` finds a +real leak (`--fail-on-finding` exits 1, `OWN001` in the output) and stays +silent on clean code (exit 0) → the SARIF surface carries the `Owen` driver +name → unsupported input fails explicitly (exit 4, never a clean scan) → +`OWEN_PYTHON` (and the deprecated `OWN_PYTHON` fallback, with its +deprecation note) both resolve Python correctly → the no-Python path fails +fast with an actionable message → the timed install-to-findings window stays +under a regression ceiling. Both platforms matter here specifically, not +just "more coverage": a `dotnet tool` shim is a native apphost on Windows and +a shell script on Unix — genuinely different process-launch mechanics, so +ubuntu-only would not have proven the Windows path. diff --git a/frontend/roslyn/OwnSharp.Cli/ToolVersion.cs b/frontend/roslyn/OwnSharp.Cli/ToolVersion.cs index b6a593b4..be3fff77 100644 --- a/frontend/roslyn/OwnSharp.Cli/ToolVersion.cs +++ b/frontend/roslyn/OwnSharp.Cli/ToolVersion.cs @@ -4,8 +4,9 @@ namespace OwnSharp.Cli; /// /// The running tool's own version — doubles as the vendored-core cache key -/// (~/.ownsharp/core/<version>/), so a core mismatch between two -/// installed tool versions can never share a cache directory. +/// (~/.owen/core/<version>/, see ), so a +/// core mismatch between two installed tool versions can never share a cache +/// directory. /// internal static class ToolVersion { diff --git a/ownlang/diag_sarif.py b/ownlang/diag_sarif.py index ffdc44f1..e24ff87d 100644 --- a/ownlang/diag_sarif.py +++ b/ownlang/diag_sarif.py @@ -10,7 +10,7 @@ and projects its evidence through the SAME ``ownlang.evidence`` builders the OwnIR path uses (``relatedLocations`` for the unordered anchors, ``codeFlows`` for the ordered slice), so both paths speak one SARIF vocabulary. The log shape mirrors -``ownir.build_sarif``: one ``run`` whose ``tool.driver`` is Own.NET with a +``ownir.build_sarif``: one ``run`` whose ``tool.driver`` is Owen with a ``rules`` catalogue of the OWN codes present, and one ``result`` per diagnostic. """ @@ -70,7 +70,7 @@ def _result(d: Diagnostic, filename: str, severity: str) -> dict[str, Any]: def build_sarif(diags: list[Diagnostic], filename: str, severity: str = "error") -> dict[str, Any]: """Render flow diagnostics as a single SARIF 2.1.0 log: one ``run`` whose - ``tool.driver`` is Own.NET (with a ``rules`` catalogue of the OWN codes present + ``tool.driver`` is Owen (with a ``rules`` catalogue of the OWN codes present and their titles) and whose ``results`` carry each diagnostic's code, location, message and evidence slice. ``severity`` only sets each result's ``level``.""" codes = sorted({d.code for d in diags}) @@ -85,7 +85,7 @@ def build_sarif(diags: list[Diagnostic], filename: str, { "tool": { "driver": { - "name": "Own.NET", + "name": "Owen", "informationUri": _SARIF_INFO_URI, "rules": rules, }, diff --git a/ownlang/ownir.py b/ownlang/ownir.py index 390a6390..ff42d28e 100644 --- a/ownlang/ownir.py +++ b/ownlang/ownir.py @@ -465,7 +465,7 @@ def _sarif_result(f: Finding, severity: str) -> dict[str, Any]: def build_sarif(findings: list[Finding], severity: str = "error") -> dict[str, Any]: """Render the findings as a single SARIF 2.1.0 log: one `run` whose - `tool.driver` is Own.NET (with a `rules` catalogue of the OWN codes present and + `tool.driver` is Owen (with a `rules` catalogue of the OWN codes present and their titles) and whose `results` carry each finding's code, C# location, message and resource kind. `severity` is the same presentation choice as the other surfaces (it only sets each result's `level`). @@ -487,7 +487,7 @@ def build_sarif(findings: list[Finding], severity: str = "error") -> dict[str, A { "tool": { "driver": { - "name": "Own.NET", + "name": "Owen", "informationUri": _SARIF_INFO_URI, "rules": rules, "properties": {"ownirSchemaVersion": OWNIR_VERSION}, diff --git a/tests/test_diag_sarif.py b/tests/test_diag_sarif.py index 5a1290f0..42462aca 100644 --- a/tests/test_diag_sarif.py +++ b/tests/test_diag_sarif.py @@ -96,7 +96,7 @@ def expect(cond: bool, msg: str) -> None: expect(esc["$schema"].endswith("sarif-schema-2.1.0.json") and esc["version"] == "2.1.0", "SARIF log must declare the 2.1.0 schema/version") driver = esc["runs"][0]["tool"]["driver"] - expect(driver["name"] == "Own.NET", "tool driver must be Own.NET") + expect(driver["name"] == "Owen", "tool driver must be Owen") expect([r["id"] for r in driver["rules"]] == ["OWN015"], f"rules catalogue must list the codes present: {driver['rules']}") diff --git a/tests/test_ownir.py b/tests/test_ownir.py index f37b5619..f915d76e 100644 --- a/tests/test_ownir.py +++ b/tests/test_ownir.py @@ -3084,7 +3084,7 @@ def _bcl(body: list) -> list: fails.append(f"SARIF envelope wrong: version={sf.get('version')!r}") driver = sf["runs"][0]["tool"]["driver"] checks += 1 - if driver.get("name") != "Own.NET": + if driver.get("name") != "Owen": fails.append(f"SARIF tool.driver.name wrong: {driver.get('name')!r}") checks += 1 if [(r["id"], r["shortDescription"]["text"]) for r in driver["rules"]] != \ From 3585bf8766164fb2ce2fcc298e0889849ac84dae Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 12 Jul 2026 13:19:24 +0000 Subject: [PATCH 2/5] fix(owen): close two Codex-flagged holes in the rebrand (PR #246) - CoreVendor's legacy ~/.ownsharp fallback trusted a bare .unpacked marker's existence keyed only by version. The CLI's own doesn't change every time the vendored core's content does - this rebrand's own SARIF-driver-name change is the concrete proof: same 0.1.0, different core content - so a machine with a pre-rebrand OwnSharp.Cli install's ~/.ownsharp/core/0.1.0/ cache could silently serve stale content (the old "Own.NET" driver name) to the new Owen.Cli 0.1.0 install. Reproduced locally: seeded a legacy cache with the pre-rebrand driver name and confirmed `owen check --format sarif` served it unchanged before this fix. Fixed by keying the marker on a SHA-256 fingerprint of the bundled ownlang/*.py files' names+content instead of bare existence - same location, both the new ~/.owen and legacy ~/.ownsharp fallback checks now only trust a cache whose marker matches the content this install actually bundles. Still the "simple fallback read, not a migration subsystem" shape requested - content-aware instead of version-only. Verified both directions locally: a stale-content legacy cache is now correctly rejected (fresh unpack to ~/.owen), and a legitimate matching-content legacy cache is still reused in place without an unnecessary ~/.owen copy. - CheckCommand.HasSupportedInput's directory walk used a plain SearchOption.AllDirectories, which throws on a locked/permission- denied subdirectory, while the extractor's own Expand() walk (OwnSharp.Extractor/Program.cs) uses EnumerationOptions { IgnoreInaccessible = true } specifically to tolerate that and keep scanning. The new preflight check could have crashed owen check on a directory tree the extractor itself would have handled fine. Fixed by using the identical EnumerationOptions the extractor already relies on. docs/notes/owen-public-facade.md updated to record both corrections in place, without erasing the original description of the intended (simpler, buggier) fallback shape. --- docs/notes/owen-public-facade.md | 32 +++++++-- frontend/roslyn/OwnSharp.Cli/CheckCommand.cs | 13 +++- frontend/roslyn/OwnSharp.Cli/CoreVendor.cs | 69 ++++++++++++++------ 3 files changed, 87 insertions(+), 27 deletions(-) diff --git a/docs/notes/owen-public-facade.md b/docs/notes/owen-public-facade.md index 7f779c8a..3d4c8581 100644 --- a/docs/notes/owen-public-facade.md +++ b/docs/notes/owen-public-facade.md @@ -93,19 +93,37 @@ Per the guardrail this rebrand was scoped to: no mass rename. or a directory containing at least one `.cs` file anywhere under it) *before* running the extractor, and exits **4** with an explicit message if none do. This is a new, additive exit-code tier — it does not change - any of the existing 0/1/`>=2`/3 contract. + any of the existing 0/1/`>=2`/3 contract. **Correction (review, PR #246):** + the directory walk in this preflight check now uses the same + `EnumerationOptions { RecurseSubdirectories = true, IgnoreInaccessible = + true }` the extractor's own `Expand()` walk uses (`OwnSharp.Extractor/ + Program.cs`) — the first version used a plain `SearchOption.AllDirectories`, + which throws on a locked/permission-denied subdirectory instead of + tolerating it and continuing, regressing a directory scan the extractor + itself would have handled fine. - **`OWEN_PYTHON`** is the preferred env var; the legacy **`OWN_PYTHON`** name is still accepted as a temporary compatibility fallback (so existing internal use — CI, scripts, muscle memory — doesn't break outright) and prints a one-line deprecation note to stderr every time it's the variable actually used to resolve Python. `OWEN_PYTHON` takes priority when both are set. -- **Cache directory** moved to `~/.owen/core//`. `CoreVendor` reads - a plain existence check against the previous `~/.ownsharp/core//` - location first — if that exact version was already unpacked there by an - older install, it's reused in place rather than re-copied. This is a - fallback read, not a migration subsystem: the old location is never - written to, moved, or deleted by the new code. +- **Cache directory** moved to `~/.owen/core//`. `CoreVendor` checks + the previous `~/.ownsharp/core//` location first — if that exact + version was already unpacked there by an older install, it's reused in + place rather than re-copied. This is a fallback read, not a migration + subsystem: the old location is never written to, moved, or deleted by the + new code. **Correction (review, PR #246):** the first version of this + fallback trusted a bare `.unpacked` marker file's *existence* — but the + CLI's own `` doesn't change every time the vendored core's + content does (this rebrand's own SARIF-driver-name change is the concrete + proof: same `0.1.0`, different core content), so an existence-only check + could silently reuse a stale, differently-vendored core from a machine + that had run a pre-rebrand `OwnSharp.Cli` install. `CoreVendor` now writes + a SHA-256 fingerprint of the bundled `ownlang/*.py` files as the marker's + content (both locations) and only trusts a cache whose marker's hash + matches the source this install would unpack right now — same "simple, + not a migration subsystem" shape, just content-aware instead of + version-only. ## Tests diff --git a/frontend/roslyn/OwnSharp.Cli/CheckCommand.cs b/frontend/roslyn/OwnSharp.Cli/CheckCommand.cs index c2ff138f..02954abb 100644 --- a/frontend/roslyn/OwnSharp.Cli/CheckCommand.cs +++ b/frontend/roslyn/OwnSharp.Cli/CheckCommand.cs @@ -158,6 +158,17 @@ private static string RequireValue(string[] args, ref int i, string flag) return args[++i]; } + // Mirrors the extractor's own Expand() walk (Program.cs): IgnoreInaccessible + // tolerates a locked/permission-denied subdirectory mid-walk instead of + // throwing. Without this, a preflight check using plain + // SearchOption.AllDirectories could crash on a tree the extractor itself + // would have scanned around just fine (review, PR #246). + private static readonly EnumerationOptions DirectoryWalkOptions = new() + { + RecurseSubdirectories = true, + IgnoreInaccessible = true, + }; + /// True if at least one of resolves to /// something the currently included frontend can analyze: a file whose /// extension is in , or a directory that @@ -172,7 +183,7 @@ private static bool HasSupportedInput(IReadOnlyList paths, out string re { if (Directory.Exists(p)) { - if (Directory.EnumerateFiles(p, "*.cs", SearchOption.AllDirectories).Any()) + if (Directory.EnumerateFiles(p, "*.cs", DirectoryWalkOptions).Any()) { reason = ""; return true; diff --git a/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs b/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs index 28c899d5..39b78d0a 100644 --- a/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs +++ b/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs @@ -1,3 +1,6 @@ +using System.Security.Cryptography; +using System.Text; + namespace OwnSharp.Cli; /// @@ -8,9 +11,10 @@ namespace OwnSharp.Cli; /// into the repository being analyzed. Cache root renamed to /// ~/.owen/core/<version>/ for the Owen public facade (see /// docs/notes/owen-public-facade.md); a version already unpacked under the -/// previous ~/.ownsharp/core/<version>/ location is reused as-is -/// (a plain existence check, not a migration — the old location is never -/// written to or deleted). +/// previous ~/.ownsharp/core/<version>/ location is reused as-is — +/// a plain fallback read, not a migration (the old location is never written +/// to or deleted) — but ONLY when its content fingerprint still matches the +/// source this install actually bundles (see ). /// internal static class CoreVendor { @@ -22,37 +26,46 @@ internal static class CoreVendor /// public static string EnsureUnpacked() { + var sourceOwnlang = Path.Combine(AppContext.BaseDirectory, "ownlang-core", "ownlang"); + if (!Directory.Exists(sourceOwnlang)) + { + throw new InvalidOperationException( + $"owen: vendored core not found at '{sourceOwnlang}' — a corrupt or " + + "incomplete tool install. Try `dotnet tool uninstall --global Owen.Cli` " + + "and reinstall."); + } + // A same-version marker alone does NOT prove the cached files match what + // THIS install bundles (review, PR #246): the CLI's own can + // stay unchanged across a content-only re-vendor (exactly what shipping + // the Owen SARIF-driver-name rename without a version bump would do), + // and the legacy ~/.ownsharp fallback below is a second location that + // can hold same-version-but-different-content data from a genuinely + // different install lineage (a pre-rebrand tool). A content fingerprint + // closes both holes with the same simple mechanism, no migration + // subsystem: trust a cache only when its marker's hash equals the hash + // of the source this install would unpack right now. + var fingerprint = Fingerprint(sourceOwnlang); + var userProfile = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); var cacheRoot = Path.Combine(userProfile, ".owen", "core", ToolVersion.Current); var marker = Path.Combine(cacheRoot, ".unpacked"); - if (File.Exists(marker)) + if (File.Exists(marker) && File.ReadAllText(marker) == fingerprint) { return cacheRoot; } // Simple fallback read from the previous ~/.ownsharp location (same - // version already unpacked there by an older install of this same - // tool) — reuse it in place rather than re-copying, no migration - // subsystem. Only a same-version marker counts: a different version - // there is irrelevant (each version gets its own cache directory). + // version, same content, already unpacked there by an older install) + // — reuse it in place rather than re-copying. var legacyCacheRoot = Path.Combine(userProfile, ".ownsharp", "core", ToolVersion.Current); var legacyMarker = Path.Combine(legacyCacheRoot, ".unpacked"); - if (File.Exists(legacyMarker)) + if (File.Exists(legacyMarker) && File.ReadAllText(legacyMarker) == fingerprint) { return legacyCacheRoot; } var destOwnlang = Path.Combine(cacheRoot, "ownlang"); - var sourceOwnlang = Path.Combine(AppContext.BaseDirectory, "ownlang-core", "ownlang"); - if (!Directory.Exists(sourceOwnlang)) - { - throw new InvalidOperationException( - $"owen: vendored core not found at '{sourceOwnlang}' — a corrupt or " + - "incomplete tool install. Try `dotnet tool uninstall --global Owen.Cli` " + - "and reinstall."); - } - Directory.CreateDirectory(destOwnlang); foreach (var file in Directory.EnumerateFiles(sourceOwnlang, "*.py")) { @@ -62,7 +75,25 @@ public static string EnsureUnpacked() // Write the marker LAST: an interrupted copy (killed process, full disk) // leaves no marker, so the next run redoes the unpack instead of running // against a half-written core. - File.WriteAllText(marker, ToolVersion.Current); + File.WriteAllText(marker, fingerprint); return cacheRoot; } + + /// SHA-256 over every vendored *.py file's name and content, + /// sorted by filename for determinism — a cheap, self-contained way to tell + /// "this cache holds exactly this source" apart from "this cache exists and + /// the version number happens to match". + private static string Fingerprint(string sourceOwnlang) + { + using var sha = SHA256.Create(); + using var buffer = new MemoryStream(); + foreach (var file in Directory.EnumerateFiles(sourceOwnlang, "*.py").OrderBy(f => f, StringComparer.Ordinal)) + { + var nameBytes = Encoding.UTF8.GetBytes(Path.GetFileName(file)); + buffer.Write(nameBytes); + var contentBytes = File.ReadAllBytes(file); + buffer.Write(contentBytes); + } + return Convert.ToHexString(sha.ComputeHash(buffer.ToArray())); + } } From 9e2a758bac0a584dd91cabf2d45ae9e988855e8a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 12 Jul 2026 13:49:27 +0000 Subject: [PATCH 3/5] Owen facade: content-addressed cache, extractor-owned exit 4, case-insensitive extensions Addresses review round 3 on PR #246 (three findings, all with committed CI regression tests, not just local verification): 1. Stale-cache pollution on a fingerprint mismatch: the round-2 fix overwrote the destination file-by-file and rewrote the marker to match the new source, but never removed a file the new source no longer had, so a removed-upstream module survived as an orphan while the marker claimed the (polluted) result was valid. CoreVendor now uses a content-addressed layout (~/.owen/core///) with no separate marker -- a fingerprint mismatch is a different path, never an in-place overwrite -- and publishes atomically (build into a temp sibling, verify the written destination's own fingerprint matches the source, then Directory.Move once). The legacy fallback now fingerprints the actual legacy destination content and only trusts it on an exact match, rejecting extra/missing/modified files instead of patching them. 2. HasSupportedInput's cheap directory check disagreed with the extractor's real expansion rules (bin/obj/generated/vendor skips), so a directory containing only skipped files passed the CLI preflight but produced zero real extractor inputs -- the exact silent-clean-scan bug exit 4 exists to prevent. Per review, the CLI no longer tries to duplicate the extractor's skip-list; the extractor itself now checks Expand(...).Distinct() and returns exit 4 with an explicit message when it's empty, and the CLI/own-check.sh's existing exit-code propagation carries that through unchanged. 3. SupportedExtensions now compares with StringComparer.OrdinalIgnoreCase so Foo.CS/App.CSPROJ are recognized like their lowercase spellings. ci.yml gains committed regression coverage (both OSes) for all of the above: matching/extra-file/modified-content legacy cache reuse and rejection, skipped-only and generated-only directories, an empty .csproj, a .sln with no projects, skipped-files-plus-real-source, an inaccessible subtree alongside a readable source, and an uppercase extension. docs/notes/owen-public-facade.md updated to describe the round-3 design. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01SSXTDuh1ZHdQc4QqmYwshw --- .github/workflows/ci.yml | 156 +++++++++++++++++ docs/notes/owen-public-facade.md | 133 +++++++++++---- frontend/roslyn/OwnSharp.Cli/CheckCommand.cs | 68 ++++---- frontend/roslyn/OwnSharp.Cli/CoreVendor.cs | 158 ++++++++++++------ frontend/roslyn/OwnSharp.Cli/ToolVersion.cs | 9 +- frontend/roslyn/OwnSharp.Extractor/Program.cs | 18 ++ 6 files changed, 423 insertions(+), 119 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2bd80500..5f7ee029 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2129,3 +2129,159 @@ jobs: echo "$out" | grep -qi "OWN_PYTHON is deprecated" || { echo "FAIL: expected a deprecation note when OWN_PYTHON is used"; exit 1; } echo "$out" | grep -q "OWN001" || { echo "FAIL: expected OWN001 in the output even via the legacy var"; exit 1; } + - name: Seed a same-content legacy cache and confirm it is reused (no ~/.owen copy) + # Review, PR #246 -- the fallback must trust DESTINATION content, not a + # marker's say-so. This is the legitimate case: exact byte-for-byte + # match with the just-packed core, reused in place. + run: | + rm -rf "$HOME/.owen" "$HOME/.ownsharp" + mkdir -p "$HOME/.ownsharp/core/0.1.0/ownlang" + cp ownlang/*.py "$HOME/.ownsharp/core/0.1.0/ownlang/" + driver=$(owen check "$RUNNER_TEMP/owen-sample" --format sarif | python3 -c "import json,sys; print(json.load(sys.stdin)['runs'][0]['tool']['driver']['name'])") + [ "$driver" = "Owen" ] || { echo "FAIL: driver name '$driver' via the legacy cache"; exit 1; } + if [ -d "$HOME/.owen" ]; then + echo "FAIL: ~/.owen was created despite a legitimate matching legacy cache (fallback not reused in place)"; exit 1 + fi + echo "OK: matching legacy cache reused in place, no unnecessary copy" + - name: Seed a legacy cache with an EXTRA (removed) file and confirm it is REJECTED + # The exact reproduction from review: an old cache holds a file the new + # source no longer has. A version-only marker would have missed this; + # the content fingerprint must not. + run: | + rm -rf "$HOME/.owen" "$HOME/.ownsharp" + mkdir -p "$HOME/.ownsharp/core/0.1.0/ownlang" + cp ownlang/*.py "$HOME/.ownsharp/core/0.1.0/ownlang/" + echo "# stale leftover module" > "$HOME/.ownsharp/core/0.1.0/ownlang/removed_module.py" + driver=$(owen check "$RUNNER_TEMP/owen-sample" --format sarif | python3 -c "import json,sys; print(json.load(sys.stdin)['runs'][0]['tool']['driver']['name'])") + [ "$driver" = "Owen" ] || { echo "FAIL: driver name '$driver'"; exit 1; } + [ -d "$HOME/.owen" ] || { echo "FAIL: expected a fresh ~/.owen unpack (extra-file legacy cache should have been rejected)"; exit 1; } + if find "$HOME/.owen" -name "removed_module.py" | grep -q .; then + echo "FAIL: the stale extra file leaked into the fresh cache"; exit 1 + fi + echo "OK: extra-file legacy cache correctly rejected, fresh cache is clean" + - name: Seed a legacy cache with MODIFIED content (same filenames, different bytes) and confirm rejection + # Same version, same file SET, different CONTENT -- the same-version + # class of bug this whole fingerprint mechanism exists to catch. + run: | + rm -rf "$HOME/.owen" "$HOME/.ownsharp" + mkdir -p "$HOME/.ownsharp/core/0.1.0/ownlang" + cp ownlang/*.py "$HOME/.ownsharp/core/0.1.0/ownlang/" + echo "# tampered" >> "$HOME/.ownsharp/core/0.1.0/ownlang/ownir.py" + driver=$(owen check "$RUNNER_TEMP/owen-sample" --format sarif | python3 -c "import json,sys; print(json.load(sys.stdin)['runs'][0]['tool']['driver']['name'])") + [ "$driver" = "Owen" ] || { echo "FAIL: driver name '$driver'"; exit 1; } + [ -d "$HOME/.owen" ] || { echo "FAIL: expected a fresh ~/.owen unpack (modified-content legacy cache should have been rejected)"; exit 1; } + echo "OK: modified-content legacy cache correctly rejected" + - name: Directory containing only skipped files (bin/obj) -- exit 4, not a silent clean scan + run: | + rm -rf "$HOME/.owen" "$HOME/.ownsharp" + mkdir -p "$RUNNER_TEMP/owen-skip-only/bin" "$RUNNER_TEMP/owen-skip-only/obj" + cat > "$RUNNER_TEMP/owen-skip-only/bin/Ignored.cs" <<'EOF' + public class X { public void M() { var s = new System.IO.MemoryStream(); } } + EOF + cat > "$RUNNER_TEMP/owen-skip-only/obj/AlsoIgnored.cs" <<'EOF' + public class Y { public void M() { var s = new System.IO.MemoryStream(); } } + EOF + set +e + out=$(owen check "$RUNNER_TEMP/owen-skip-only" 2>&1) + rc=$? + set -e + echo "$out" + [ "$rc" -eq 4 ] || { echo "FAIL: expected exit 4 (bin/obj-only directory), got $rc"; exit 1; } + echo "$out" | grep -qi "no supported input" || { echo "FAIL: expected an explicit unsupported-input message"; exit 1; } + - name: Directory containing only generated files (*.g.cs) -- exit 4 + run: | + mkdir -p "$RUNNER_TEMP/owen-generated-only" + cat > "$RUNNER_TEMP/owen-generated-only/Foo.g.cs" <<'EOF' + public class Gen { public void M() { var s = new System.IO.MemoryStream(); } } + EOF + set +e + out=$(owen check "$RUNNER_TEMP/owen-generated-only" 2>&1) + rc=$? + set -e + echo "$out" + [ "$rc" -eq 4 ] || { echo "FAIL: expected exit 4 (generated-only directory), got $rc"; exit 1; } + echo "$out" | grep -qi "no supported input" || { echo "FAIL: expected an explicit unsupported-input message"; exit 1; } + - name: Empty .csproj (no .cs files in its directory) -- exit 4 + run: | + mkdir -p "$RUNNER_TEMP/owen-empty-proj" + cat > "$RUNNER_TEMP/owen-empty-proj/Empty.csproj" <<'EOF' + + net8.0 + + EOF + set +e + out=$(owen check "$RUNNER_TEMP/owen-empty-proj/Empty.csproj" 2>&1) + rc=$? + set -e + echo "$out" + [ "$rc" -eq 4 ] || { echo "FAIL: expected exit 4 (empty .csproj), got $rc"; exit 1; } + - name: .sln with no usable projects -- exit 4 + run: | + mkdir -p "$RUNNER_TEMP/owen-empty-sln" + cat > "$RUNNER_TEMP/owen-empty-sln/Empty.sln" <<'EOF' + Microsoft Visual Studio Solution File, Format Version 12.00 + # no Project( lines at all + EOF + set +e + out=$(owen check "$RUNNER_TEMP/owen-empty-sln/Empty.sln" 2>&1) + rc=$? + set -e + echo "$out" + [ "$rc" -eq 4 ] || { echo "FAIL: expected exit 4 (.sln with no projects), got $rc"; exit 1; } + - name: Skipped files plus one real source -- the real one is still found, exit 1 + run: | + mkdir -p "$RUNNER_TEMP/owen-mixed-skip/bin" "$RUNNER_TEMP/owen-mixed-skip/src" + cat > "$RUNNER_TEMP/owen-mixed-skip/bin/Ignored.cs" <<'EOF' + public class X { public void M() { var s = new System.IO.MemoryStream(); } } + EOF + cat > "$RUNNER_TEMP/owen-mixed-skip/src/Real.cs" <<'EOF' + using System.IO; + public class Leaky { public void Run() { var s = new MemoryStream(); s.WriteByte(1); } } + EOF + set +e + out=$(owen check "$RUNNER_TEMP/owen-mixed-skip" --fail-on-finding 2>&1) + rc=$? + set -e + echo "$out" + [ "$rc" -eq 1 ] || { echo "FAIL: expected exit 1 (real source found despite bin/ noise), got $rc"; exit 1; } + echo "$out" | grep -q "OWN001" || { echo "FAIL: expected OWN001"; exit 1; } + - name: Inaccessible subtree plus one readable source -- tolerated, real source still found + # On these hosted runners the job does not run as an all-powerful admin/ + # root account, so chmod/icacls genuinely restricts access (unlike a + # root-based local sandbox, where this same scenario cannot be exercised). + run: | + mkdir -p "$RUNNER_TEMP/owen-mixed-locked/locked" "$RUNNER_TEMP/owen-mixed-locked/readable" + cat > "$RUNNER_TEMP/owen-mixed-locked/locked/Secret.cs" <<'EOF' + public class X { public void M() { var s = new System.IO.MemoryStream(); } } + EOF + cat > "$RUNNER_TEMP/owen-mixed-locked/readable/Real.cs" <<'EOF' + using System.IO; + public class Leaky { public void Run() { var s = new MemoryStream(); s.WriteByte(1); } } + EOF + chmod 000 "$RUNNER_TEMP/owen-mixed-locked/locked" 2>/dev/null || true + set +e + out=$(owen check "$RUNNER_TEMP/owen-mixed-locked" --fail-on-finding 2>&1) + rc=$? + set -e + chmod 755 "$RUNNER_TEMP/owen-mixed-locked/locked" 2>/dev/null || true + echo "$out" + if echo "$out" | grep -qi "UnauthorizedAccess\|Unhandled exception"; then + echo "FAIL: crashed on the locked subdirectory instead of tolerating it"; exit 1 + fi + echo "$out" | grep -q "OWN001" || { echo "FAIL: expected the readable source's leak to still be found"; exit 1; } + - name: Uppercase extensions are accepted case-insensitively (Foo.CS) + run: | + mkdir -p "$RUNNER_TEMP/owen-uppercase" + cat > "$RUNNER_TEMP/owen-uppercase/Leak.CS" <<'EOF' + using System.IO; + public class Leaky { public void Run() { var s = new MemoryStream(); s.WriteByte(1); } } + EOF + set +e + out=$(owen check "$RUNNER_TEMP/owen-uppercase/Leak.CS" --fail-on-finding 2>&1) + rc=$? + set -e + echo "$out" + [ "$rc" -eq 1 ] || { echo "FAIL: expected exit 1 for uppercase .CS extension, got $rc"; exit 1; } + echo "$out" | grep -q "OWN001" || { echo "FAIL: expected OWN001"; exit 1; } + - name: Clean up cache state left by the edge-case tests above + run: rm -rf "$HOME/.owen" "$HOME/.ownsharp" diff --git a/docs/notes/owen-public-facade.md b/docs/notes/owen-public-facade.md index 3d4c8581..14d74241 100644 --- a/docs/notes/owen-public-facade.md +++ b/docs/notes/owen-public-facade.md @@ -87,43 +87,76 @@ Per the guardrail this rebrand was scoped to: no mass rename. the help text does not claim support that doesn't exist yet. - **Unsupported input fails explicitly.** Before this rebrand, pointing `check` at a `.ts` file or an empty/non-C# directory silently printed - "0 findings" — indistinguishable from a genuinely clean C# scan. - `CheckCommand.HasSupportedInput` now checks every given path resolves to - something the included frontend can read (a `.cs`/`.csproj`/`.sln` file, - or a directory containing at least one `.cs` file anywhere under it) - *before* running the extractor, and exits **4** with an explicit message - if none do. This is a new, additive exit-code tier — it does not change - any of the existing 0/1/`>=2`/3 contract. **Correction (review, PR #246):** - the directory walk in this preflight check now uses the same - `EnumerationOptions { RecurseSubdirectories = true, IgnoreInaccessible = - true }` the extractor's own `Expand()` walk uses (`OwnSharp.Extractor/ - Program.cs`) — the first version used a plain `SearchOption.AllDirectories`, - which throws on a locked/permission-denied subdirectory instead of - tolerating it and continuing, regressing a directory scan the extractor - itself would have handled fine. + "0 findings" — indistinguishable from a genuinely clean C# scan. The + explicit-failure behavior now has two layers, split across the two + components that each know a different half of the answer (review, PR + #246 round 2): `CheckCommand.HasSupportedInput` does only the CHEAP, + obvious check — an existing file with a recognized extension, or an + existing directory — *before* running the extractor at all, catching a + bare `.ts` file or a nonexistent path for free. `SupportedExtensions` is + compared with `StringComparer.OrdinalIgnoreCase` (review, PR #246 round + 2: `Foo.CS`/`App.CSPROJ` are the same file kind as their lowercase + spellings, on Windows/macOS filesystems and to MSBuild itself). The CLI + does **not** also try to duplicate the extractor's directory-walk skip + rules to predict whether a directory will actually yield anything — a + directory containing only `bin/`, `obj/`, or generated (`.g.cs`, + `.Designer.cs`, `.AssemblyInfo.cs`) files passed the CLI's cheap check + fine but produced zero real extractor inputs, recreating the exact + silent-clean-scan bug this exit tier exists to prevent. Duplicating the + extractor's skip-list in the CLI would just drift from it over time + (review, PR #246 round 2 explicit instruction: "do not duplicate the + complete extractor expansion rules in the CLI"). Instead, the extractor + itself is now the sole authority on "found nothing after expansion": + `OwnSharp.Extractor/Program.cs` checks `Expand(rawInputs).Distinct()` + immediately after computing it and returns a new exit code **4** with an + explicit message if that list is empty — a `.csproj`/`.sln` with no + usable source resolves the same way, since project/solution resolution + is text/glob-based, not full MSBuild evaluation (see the `ProjectCsFiles` + comment). `CheckCommand.RunAsync`'s existing `extractRc != 0` early + return propagates that 4 unchanged, and `own-check.sh`'s existing `set + -e` does the same for the script/Action path — no extra code needed on + either side beyond the extractor's own check. This is a new, additive + exit-code tier; it does not change any of the existing 0/1/`>=2`/3 + contract. - **`OWEN_PYTHON`** is the preferred env var; the legacy **`OWN_PYTHON`** name is still accepted as a temporary compatibility fallback (so existing internal use — CI, scripts, muscle memory — doesn't break outright) and prints a one-line deprecation note to stderr every time it's the variable actually used to resolve Python. `OWEN_PYTHON` takes priority when both are set. -- **Cache directory** moved to `~/.owen/core//`. `CoreVendor` checks - the previous `~/.ownsharp/core//` location first — if that exact - version was already unpacked there by an older install, it's reused in - place rather than re-copied. This is a fallback read, not a migration - subsystem: the old location is never written to, moved, or deleted by the - new code. **Correction (review, PR #246):** the first version of this - fallback trusted a bare `.unpacked` marker file's *existence* — but the - CLI's own `` doesn't change every time the vendored core's - content does (this rebrand's own SARIF-driver-name change is the concrete - proof: same `0.1.0`, different core content), so an existence-only check - could silently reuse a stale, differently-vendored core from a machine - that had run a pre-rebrand `OwnSharp.Cli` install. `CoreVendor` now writes - a SHA-256 fingerprint of the bundled `ownlang/*.py` files as the marker's - content (both locations) and only trusts a cache whose marker's hash - matches the source this install would unpack right now — same "simple, - not a migration subsystem" shape, just content-aware instead of - version-only. +- **Cache directory** moved to `~/.owen/core///`, + where `fingerprint` is a SHA-256 over every vendored file's name and + content (length-prefixed encoding, so e.g. name `"ab"` + content `"c"` + can't collide with name `"a"` + content `"bc"` by bare concatenation). + **Correction (review, PR #246 round 3):** the round-2 design (a + version-keyed directory with a separate marker file holding a source + fingerprint) had a gap the reviewer's reproduction demonstrated directly: + on a fingerprint mismatch, the fix copied new files over the existing + destination with `overwrite: true`, then rewrote the marker to match the + new source — but a file the new source no longer has (e.g. a module + deleted upstream) was never removed from the destination, so it survived + as an orphan while the rewritten marker now claimed the (polluted) + directory was valid. Round 3 removes the marker concept entirely and + makes cache identity content-addressed: the fingerprint *is* the last + path segment, so a content change is a different path, never an + in-place overwrite of an existing one. Publication into a fresh path is + atomic — build into a `.tmp-` sibling directory, recompute the + fingerprint of what was actually written there, confirm it equals the + source fingerprint, and only then `Directory.Move` it to the final + fingerprint-named path — so a reader can only ever observe nothing, or a + fully-written self-verified copy, never a partial one (a crash or a lost + race with a concurrent `owen` process mid-copy just leaves a harmless + orphaned temp directory that nothing consults). The previous flat + `~/.ownsharp/core//` location (pre-rebrand layout, no + fingerprint segment) is still checked first and used in place without + copying — but only after fingerprinting what is *actually on disk* there + right now and confirming it equals the current source fingerprint; a + destination with extra, missing, or modified files fails that check and + falls through to a fresh unpack instead of being trusted or patched. + This is still a plain fallback *read*: the legacy location is never + written to, moved, or deleted by this code — content-addressing didn't + change that guardrail, only how "is this destination actually still + correct" gets decided. ## Tests @@ -145,6 +178,44 @@ does **not** contain the literal clean-scan phrase `0 findings.`), by the driver-name change, not scope creep; they were caught by `python tests/run_tests.py` failing before the fix. +**Added in review round 3** — the reviewer's stated concern was that both +bugs (stale-cache pollution, cheap-preflight/extractor disagreement) were +"sufficiently non-obvious that a future cleanup could reintroduce [them] +while the ordinary fresh-install smoke remains green," so both +reproductions from the review became committed `ci.yml` steps rather than +staying local manual verification. All of the below ran locally first +(pack -> isolated-feed install -> exercise -> confirm exit code/output) +before being encoded as assertions: +- **Matching legacy cache is reused in place** — the legitimate case: a + byte-for-byte-identical legacy `~/.ownsharp/...` unpack is used without + copying (`~/.owen` is asserted to *not* get created). +- **Extra (removed-upstream) file in the legacy cache is rejected** — the + exact reproduction from the review: an old cache holds a file the new + source no longer has. A fresh, clean `~/.owen` unpack must result, with + the stale file absent from it. +- **Modified-content legacy cache (same filenames, different bytes) is + rejected** — same version, same file set, different content: the + same-version class of bug the fingerprint exists to catch generally, not + just the removed-file case. +- **Directory containing only skipped files** (`bin/`/`obj/`) and + **directory containing only generated files** (`*.g.cs`) both assert + exit 4 with an explicit "no supported input" message — the CLI's cheap + preflight passes these (a directory that exists), and only the + extractor's own zero-expanded-input check catches them. +- **Empty `.csproj`** (no `Compile` items resolve to any `.cs` file) and + **`.sln` with no `Project(` lines** both assert exit 4 the same way. +- **Skipped files alongside one real source file** asserts exit 1 with + `OWN001` still found — proving the skip rules don't over-reject a + directory that has genuine content alongside the noise. +- **An inaccessible subtree alongside one readable source file** (`chmod + 000` on a subdirectory, meaningful on the hosted runners' non-root + accounts unlike this session's root-based local sandbox) asserts no + crash and the readable source's finding is still reported — the + `EnumerationOptions { IgnoreInaccessible = true }` tolerance from review + round 2, now with a real permission-denied subdirectory to exercise it. +- **Uppercase extension (`Leak.CS`)** asserts exit 1 with `OWN001` found — + the `StringComparer.OrdinalIgnoreCase` fix from review round 3. + ## PR separation This is PR 1 of 3 for the release-readiness work: diff --git a/frontend/roslyn/OwnSharp.Cli/CheckCommand.cs b/frontend/roslyn/OwnSharp.Cli/CheckCommand.cs index 02954abb..de6ab957 100644 --- a/frontend/roslyn/OwnSharp.Cli/CheckCommand.cs +++ b/frontend/roslyn/OwnSharp.Cli/CheckCommand.cs @@ -7,8 +7,13 @@ namespace OwnSharp.Cli; /// -> facts.json -> the vendored core (system Python) -> render. Flags mirror /// scripts/own-check.sh 1:1; the exit-code contract is the same one (own-check /// comment): 0 clean, 1 findings, >=2 a hard error, plus --fail-on-finding. -/// Exit 4 is `owen`'s own: no path resolved to anything the currently -/// included frontend (.NET/C#) can analyze — see . +/// Exit 4 means no analyzable input: either this preflight's cheap check +/// rejects every path outright (), or every +/// path existed but the EXTRACTOR's own expansion (which alone knows its +/// skip rules — bin/obj, generated, vendor trees) found nothing after +/// filtering, in which case the extractor itself returns 4 and this command +/// simply propagates it (review, PR #246: the CLI must not duplicate the +/// extractor's expansion/skip rules to guess that outcome itself). /// internal static class CheckCommand { @@ -18,10 +23,14 @@ internal static class CheckCommand // The product (Owen) is language-neutral at the OwnIR/core level; this // distribution currently wires up only the .NET/C# frontend. Naming the // extensions here (instead of e.g. "just try everything and see") is - // what makes "unsupported input fails explicitly" possible at all -- a - // silent 0-finding scan on a .ts file or an empty directory would - // otherwise look identical to a genuinely clean C# scan. - private static readonly HashSet SupportedExtensions = [".cs", ".csproj", ".sln"]; + // what makes the CHEAP half of "unsupported input fails explicitly" + // possible: an obviously-wrong bare file (.ts, no extension at all, + // nonexistent path) is rejected here without spinning up the extractor + // at all. Case-insensitive (review, PR #246): Windows/macOS filesystems + // commonly are, and MSBuild itself treats `Foo.CS`/`App.CSPROJ` as the + // same file kinds as their lowercase spellings. + private static readonly HashSet SupportedExtensions = + new(StringComparer.OrdinalIgnoreCase) { ".cs", ".csproj", ".sln" }; public static async Task RunAsync(string[] args) { @@ -158,24 +167,19 @@ private static string RequireValue(string[] args, ref int i, string flag) return args[++i]; } - // Mirrors the extractor's own Expand() walk (Program.cs): IgnoreInaccessible - // tolerates a locked/permission-denied subdirectory mid-walk instead of - // throwing. Without this, a preflight check using plain - // SearchOption.AllDirectories could crash on a tree the extractor itself - // would have scanned around just fine (review, PR #246). - private static readonly EnumerationOptions DirectoryWalkOptions = new() - { - RecurseSubdirectories = true, - IgnoreInaccessible = true, - }; - - /// True if at least one of resolves to - /// something the currently included frontend can analyze: a file whose - /// extension is in , or a directory that - /// recursively contains at least one .cs file. A path that doesn't - /// exist on disk at all is also unsupported (not silently skipped) -- the - /// point is to never let "nothing to analyze" print as "0 findings, - /// clean". + /// True if at least one of is CHEAPLY, + /// OBVIOUSLY plausible input for the currently included frontend: an + /// existing file whose extension is in , + /// or an existing directory. Deliberately NOT a full expansion — a + /// directory is accepted here even if every .cs file under it + /// turns out to be skipped (bin/obj, generated, vendor) once the + /// extractor actually walks it; duplicating that skip-list in the CLI + /// would drift from the extractor's real rules (review, PR #246). The + /// extractor itself is the sole authority on "found nothing after + /// expansion" and returns exit 4 for that case (Program.cs) — this + /// preflight only catches the cheaper "obviously not C# at all" case + /// (nonexistent path, or a bare file with the wrong extension) without + /// paying for an extractor invocation. private static bool HasSupportedInput(IReadOnlyList paths, out string reason) { var problems = new List(); @@ -183,14 +187,10 @@ private static bool HasSupportedInput(IReadOnlyList paths, out string re { if (Directory.Exists(p)) { - if (Directory.EnumerateFiles(p, "*.cs", DirectoryWalkOptions).Any()) - { - reason = ""; - return true; - } - problems.Add($"'{p}' is a directory with no .cs files"); + reason = ""; + return true; } - else if (File.Exists(p)) + if (File.Exists(p)) { if (SupportedExtensions.Contains(Path.GetExtension(p))) { @@ -198,11 +198,9 @@ private static bool HasSupportedInput(IReadOnlyList paths, out string re return true; } problems.Add($"'{p}' has an unsupported extension ({Path.GetExtension(p)})"); + continue; } - else - { - problems.Add($"'{p}' does not exist"); - } + problems.Add($"'{p}' does not exist"); } reason = string.Join("; ", problems); return false; diff --git a/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs b/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs index 39b78d0a..47e2491c 100644 --- a/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs +++ b/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs @@ -6,15 +6,35 @@ namespace OwnSharp.Cli; /// /// Unpacks the vendored ownlang/ Python source (packed into this tool's /// own nupkg under ownlang-core/ownlang/*.py, see the .csproj) into a -/// stable, per-version cache directory outside the tool's own (versioned, -/// nested) install path — and, per the design decision in issue #202, never -/// into the repository being analyzed. Cache root renamed to -/// ~/.owen/core/<version>/ for the Owen public facade (see -/// docs/notes/owen-public-facade.md); a version already unpacked under the -/// previous ~/.ownsharp/core/<version>/ location is reused as-is — -/// a plain fallback read, not a migration (the old location is never written -/// to or deleted) — but ONLY when its content fingerprint still matches the -/// source this install actually bundles (see ). +/// stable, content-addressed cache directory outside the tool's own +/// (versioned, nested) install path — and, per the design decision in issue +/// #202, never into the repository being analyzed. +/// +/// Layout: ~/.owen/core/<version>/<fingerprint>/ownlang/, where +/// fingerprint is a SHA-256 over every vendored file's name and +/// content (see ). Content-addressing (review, PR +/// #246) closes a hole a plain version-keyed marker had: the CLI's own +/// <Version> does not change every time the vendored core's +/// content does — this rebrand's own SARIF-driver-name change is the +/// concrete proof, same 0.1.0, different core content — so a +/// version-only cache could either serve stale content on a mismatch, or +/// (worse) get overwritten file-by-file in place, leaving files the new +/// source no longer has stranded alongside the new ones with a marker that +/// then claims the (polluted) result matches. Content-addressing sidesteps +/// both: a fingerprint mismatch is a *different path*, never an overwrite of +/// an existing one, and publication into that path is atomic (build in a +/// temp sibling, then once +/// fully written and verified) so a reader never observes a partial write. +/// +/// A version already unpacked under the previous flat +/// ~/.ownsharp/core/<version>/ location (pre-rebrand layout, no +/// fingerprint) is used in place — without copying — but ONLY after +/// recomputing a fingerprint over what is actually on disk there and +/// confirming it matches the source this install bundles right now; a +/// destination with extra, missing, or modified files fails that check and +/// falls through to a fresh unpack. This is still a plain fallback *read*, +/// not a migration subsystem: the legacy location is never written to, +/// moved, or deleted by this code. /// internal static class CoreVendor { @@ -34,66 +54,106 @@ public static string EnsureUnpacked() "incomplete tool install. Try `dotnet tool uninstall --global Owen.Cli` " + "and reinstall."); } - // A same-version marker alone does NOT prove the cached files match what - // THIS install bundles (review, PR #246): the CLI's own can - // stay unchanged across a content-only re-vendor (exactly what shipping - // the Owen SARIF-driver-name rename without a version bump would do), - // and the legacy ~/.ownsharp fallback below is a second location that - // can hold same-version-but-different-content data from a genuinely - // different install lineage (a pre-rebrand tool). A content fingerprint - // closes both holes with the same simple mechanism, no migration - // subsystem: trust a cache only when its marker's hash equals the hash - // of the source this install would unpack right now. - var fingerprint = Fingerprint(sourceOwnlang); + var sourceFiles = SortedPyFiles(sourceOwnlang); + var fingerprint = Fingerprint(sourceFiles); var userProfile = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); - var cacheRoot = Path.Combine(userProfile, ".owen", "core", ToolVersion.Current); - var marker = Path.Combine(cacheRoot, ".unpacked"); + var versionRoot = Path.Combine(userProfile, ".owen", "core", ToolVersion.Current); + var finalRoot = Path.Combine(versionRoot, fingerprint); + var finalOwnlang = Path.Combine(finalRoot, "ownlang"); - if (File.Exists(marker) && File.ReadAllText(marker) == fingerprint) + // Content-addressed: existence at the fingerprint-named path IS proof of + // validity (see the atomic-publish note below) -- no separate marker to + // go stale or drift from the directory's actual contents. + if (Directory.Exists(finalOwnlang)) { - return cacheRoot; + return finalRoot; } - // Simple fallback read from the previous ~/.ownsharp location (same - // version, same content, already unpacked there by an older install) - // — reuse it in place rather than re-copying. - var legacyCacheRoot = Path.Combine(userProfile, ".ownsharp", "core", ToolVersion.Current); - var legacyMarker = Path.Combine(legacyCacheRoot, ".unpacked"); - if (File.Exists(legacyMarker) && File.ReadAllText(legacyMarker) == fingerprint) + // Legacy fallback: verify the LEGACY DESTINATION's actual content, not a + // marker file's say-so (review, PR #246) -- a marker only proves "an + // unpack happened here once", never that nothing since removed, added, + // or modified a file in that directory. + var legacyOwnlang = Path.Combine(userProfile, ".ownsharp", "core", ToolVersion.Current, "ownlang"); + if (Directory.Exists(legacyOwnlang)) { - return legacyCacheRoot; + var legacyFiles = SortedPyFiles(legacyOwnlang); + var legacyFingerprint = Fingerprint(legacyFiles); + if (legacyFingerprint == fingerprint) + { + return Path.Combine(userProfile, ".ownsharp", "core", ToolVersion.Current); + } } - var destOwnlang = Path.Combine(cacheRoot, "ownlang"); - Directory.CreateDirectory(destOwnlang); - foreach (var file in Directory.EnumerateFiles(sourceOwnlang, "*.py")) + // Fresh unpack: build into a temp sibling, verify the DESTINATION's own + // fingerprint matches the source before anything else can observe it, + // then publish with a single atomic rename. A reader can only ever see + // either nothing at finalOwnlang, or a fully-written, self-verified copy + // -- never a partial one (crash/kill/full-disk mid-copy just leaves an + // orphaned temp directory next to it, harmless and never consulted). + var tempRoot = Path.Combine(versionRoot, $".tmp-{Guid.NewGuid():N}"); + var tempOwnlang = Path.Combine(tempRoot, "ownlang"); + try { - var dest = Path.Combine(destOwnlang, Path.GetFileName(file)); - File.Copy(file, dest, overwrite: true); + Directory.CreateDirectory(tempOwnlang); + foreach (var file in sourceFiles) + { + File.Copy(file, Path.Combine(tempOwnlang, Path.GetFileName(file)), overwrite: true); + } + var writtenFiles = SortedPyFiles(tempOwnlang); + var writtenFingerprint = Fingerprint(writtenFiles); + if (writtenFingerprint != fingerprint) + { + throw new InvalidOperationException( + $"owen: internal error -- the core copy at '{tempOwnlang}' does not match " + + "its source fingerprint after writing. Not publishing it; try reinstalling."); + } + + if (Directory.Exists(finalOwnlang)) + { + // Lost a race with a concurrent `owen` process that published the + // same fingerprint first -- their content is provably identical + // (same fingerprint), so just use it. + return finalRoot; + } + Directory.CreateDirectory(finalRoot); + Directory.Move(tempOwnlang, finalOwnlang); + return finalRoot; + } + finally + { + if (Directory.Exists(tempRoot)) + { + try { Directory.Delete(tempRoot, recursive: true); } catch (IOException) { /* best-effort cleanup */ } + } } - // Write the marker LAST: an interrupted copy (killed process, full disk) - // leaves no marker, so the next run redoes the unpack instead of running - // against a half-written core. - File.WriteAllText(marker, fingerprint); - return cacheRoot; } - /// SHA-256 over every vendored *.py file's name and content, - /// sorted by filename for determinism — a cheap, self-contained way to tell - /// "this cache holds exactly this source" apart from "this cache exists and - /// the version number happens to match". - private static string Fingerprint(string sourceOwnlang) + private static List SortedPyFiles(string dir) => + Directory.EnumerateFiles(dir, "*.py").OrderBy(f => Path.GetFileName(f), StringComparer.Ordinal).ToList(); + + /// SHA-256 over every file's name and content, each explicitly + /// length-prefixed (review, PR #246) so two different (name, content) sets + /// can never hash identically by having their bytes merely concatenate the + /// same way -- e.g. name "ab" + content "c" vs. name "a" + content "bc" + /// would collide under bare concatenation; an 8-byte length prefix on each + /// field rules that out. Sorted by filename first (by the caller) for a + /// fingerprint that doesn't depend on enumeration order. + private static string Fingerprint(IReadOnlyList files) { using var sha = SHA256.Create(); using var buffer = new MemoryStream(); - foreach (var file in Directory.EnumerateFiles(sourceOwnlang, "*.py").OrderBy(f => f, StringComparer.Ordinal)) + using var writer = new BinaryWriter(buffer, Encoding.UTF8, leaveOpen: true); + foreach (var file in files) { var nameBytes = Encoding.UTF8.GetBytes(Path.GetFileName(file)); - buffer.Write(nameBytes); + writer.Write((long)nameBytes.Length); + writer.Write(nameBytes); var contentBytes = File.ReadAllBytes(file); - buffer.Write(contentBytes); + writer.Write((long)contentBytes.Length); + writer.Write(contentBytes); } + writer.Flush(); return Convert.ToHexString(sha.ComputeHash(buffer.ToArray())); } } diff --git a/frontend/roslyn/OwnSharp.Cli/ToolVersion.cs b/frontend/roslyn/OwnSharp.Cli/ToolVersion.cs index be3fff77..07cb2750 100644 --- a/frontend/roslyn/OwnSharp.Cli/ToolVersion.cs +++ b/frontend/roslyn/OwnSharp.Cli/ToolVersion.cs @@ -3,10 +3,11 @@ namespace OwnSharp.Cli; /// -/// The running tool's own version — doubles as the vendored-core cache key -/// (~/.owen/core/<version>/, see ), so a -/// core mismatch between two installed tool versions can never share a cache -/// directory. +/// The running tool's own version — the first path segment of the +/// vendored-core cache key (~/.owen/core/<version>/<fingerprint>/, +/// see ), so a core mismatch between two installed +/// tool versions can never share a cache directory even before the +/// fingerprint segment is considered. /// internal static class ToolVersion { diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index 00a142e0..d3d57d69 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -462,6 +462,24 @@ static string Rel(string path) => var inputs = Expand(rawInputs).Distinct().ToList(); +// Every raw input resolved to nothing analyzable — every path either doesn't +// exist, or every .cs file under it was filtered out by IsSkipped (bin/obj, +// generated, vendor trees; review, PR #246). Distinct from the "no inputs +// GIVEN at all" usage error above (exit 2): this is "inputs were given, but +// none of them contain anything this frontend can read" — the same shape a +// caller (owen check, own-check.sh) must not read as "0 findings, clean". +// Exit 4 deliberately matches the CLI's own no-supported-input contract +// (CheckCommand.cs) so a caller never needs to special-case which layer +// caught it; own-check.sh/action.yml propagate any non-zero extractor exit +// as-is (no python-core stage 2 is reached), same as the existing exit-2 path. +if (inputs.Count == 0) +{ + Console.Error.WriteLine( + "extractor: no supported input found — every given path either does not exist " + + "or contains no .cs file after skipping bin/obj/generated/vendor trees."); + return 4; +} + static bool IsHandler(ExpressionSyntax rhs) => rhs is IdentifierNameSyntax || rhs is MemberAccessExpressionSyntax; From f555f6c75009df5d977e6a8e3bfb430a6cb43a03 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 12 Jul 2026 13:58:40 +0000 Subject: [PATCH 4/5] Owen facade: address CodeRabbit round on PR #246 - ci.yml: use `python` instead of `python3` in the CLI smoke job's SARIF driver-name assertions -- actions/setup-python does not expose a python3 alias on windows-latest, so these would have broken the Windows leg. - ci.yml: the inaccessible-subtree test is now Linux-only (chmod 000 does not reliably lock out the job's own account on Windows) and actually proves the lockout worked -- asserts exit 1 and that the output names Real.cs but never Secret.cs, instead of a bare "OWN001 somewhere" check that would pass just as well if the locked file got analyzed too. - CoreVendor.cs: close the narrow TOCTOU window between the Directory.Exists(finalOwnlang) race check and Directory.Move -- a concurrent `owen` process publishing the same fingerprint in between now resolves the same way the earlier check already did (their content is provably identical), instead of surfacing Move's IOException. - CoreVendor.cs: Fingerprint hashes the MemoryStream directly (rewound) instead of copying it via ToArray() first. - owen-public-facade.md: language tag on the --help excerpt's code fence. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01SSXTDuh1ZHdQc4QqmYwshw --- .github/workflows/ci.yml | 29 ++++++++++++++-------- docs/notes/owen-public-facade.md | 2 +- frontend/roslyn/OwnSharp.Cli/CoreVendor.cs | 19 +++++++++++--- 3 files changed, 36 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5f7ee029..3c38b8bf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2076,7 +2076,7 @@ jobs: - name: owen check --format sarif -- Owen-branded SARIF driver name run: | out=$(owen check "$RUNNER_TEMP/owen-sample" --format sarif) - echo "$out" | python3 -c " + echo "$out" | python -c " import json, sys d = json.load(sys.stdin) n = d['runs'][0]['tool']['driver']['name'] @@ -2137,7 +2137,7 @@ jobs: rm -rf "$HOME/.owen" "$HOME/.ownsharp" mkdir -p "$HOME/.ownsharp/core/0.1.0/ownlang" cp ownlang/*.py "$HOME/.ownsharp/core/0.1.0/ownlang/" - driver=$(owen check "$RUNNER_TEMP/owen-sample" --format sarif | python3 -c "import json,sys; print(json.load(sys.stdin)['runs'][0]['tool']['driver']['name'])") + driver=$(owen check "$RUNNER_TEMP/owen-sample" --format sarif | python -c "import json,sys; print(json.load(sys.stdin)['runs'][0]['tool']['driver']['name'])") [ "$driver" = "Owen" ] || { echo "FAIL: driver name '$driver' via the legacy cache"; exit 1; } if [ -d "$HOME/.owen" ]; then echo "FAIL: ~/.owen was created despite a legitimate matching legacy cache (fallback not reused in place)"; exit 1 @@ -2152,7 +2152,7 @@ jobs: mkdir -p "$HOME/.ownsharp/core/0.1.0/ownlang" cp ownlang/*.py "$HOME/.ownsharp/core/0.1.0/ownlang/" echo "# stale leftover module" > "$HOME/.ownsharp/core/0.1.0/ownlang/removed_module.py" - driver=$(owen check "$RUNNER_TEMP/owen-sample" --format sarif | python3 -c "import json,sys; print(json.load(sys.stdin)['runs'][0]['tool']['driver']['name'])") + driver=$(owen check "$RUNNER_TEMP/owen-sample" --format sarif | python -c "import json,sys; print(json.load(sys.stdin)['runs'][0]['tool']['driver']['name'])") [ "$driver" = "Owen" ] || { echo "FAIL: driver name '$driver'"; exit 1; } [ -d "$HOME/.owen" ] || { echo "FAIL: expected a fresh ~/.owen unpack (extra-file legacy cache should have been rejected)"; exit 1; } if find "$HOME/.owen" -name "removed_module.py" | grep -q .; then @@ -2167,7 +2167,7 @@ jobs: mkdir -p "$HOME/.ownsharp/core/0.1.0/ownlang" cp ownlang/*.py "$HOME/.ownsharp/core/0.1.0/ownlang/" echo "# tampered" >> "$HOME/.ownsharp/core/0.1.0/ownlang/ownir.py" - driver=$(owen check "$RUNNER_TEMP/owen-sample" --format sarif | python3 -c "import json,sys; print(json.load(sys.stdin)['runs'][0]['tool']['driver']['name'])") + driver=$(owen check "$RUNNER_TEMP/owen-sample" --format sarif | python -c "import json,sys; print(json.load(sys.stdin)['runs'][0]['tool']['driver']['name'])") [ "$driver" = "Owen" ] || { echo "FAIL: driver name '$driver'"; exit 1; } [ -d "$HOME/.owen" ] || { echo "FAIL: expected a fresh ~/.owen unpack (modified-content legacy cache should have been rejected)"; exit 1; } echo "OK: modified-content legacy cache correctly rejected" @@ -2246,9 +2246,14 @@ jobs: [ "$rc" -eq 1 ] || { echo "FAIL: expected exit 1 (real source found despite bin/ noise), got $rc"; exit 1; } echo "$out" | grep -q "OWN001" || { echo "FAIL: expected OWN001"; exit 1; } - name: Inaccessible subtree plus one readable source -- tolerated, real source still found - # On these hosted runners the job does not run as an all-powerful admin/ - # root account, so chmod/icacls genuinely restricts access (unlike a - # root-based local sandbox, where this same scenario cannot be exercised). + # On these hosted Linux runners the job does not run as an all-powerful + # admin/root account, so chmod genuinely restricts access (unlike a + # root-based local sandbox, where this same scenario cannot be exercised, + # and unlike Windows runners, where chmod 000 does not reliably lock out + # the job's own account -- review, PR #246: restricted to Linux so the + # assertions below are actually exercising the locked path, not silently + # passing because both files got analyzed). + if: runner.os == 'Linux' run: | mkdir -p "$RUNNER_TEMP/owen-mixed-locked/locked" "$RUNNER_TEMP/owen-mixed-locked/readable" cat > "$RUNNER_TEMP/owen-mixed-locked/locked/Secret.cs" <<'EOF' @@ -2258,17 +2263,21 @@ jobs: using System.IO; public class Leaky { public void Run() { var s = new MemoryStream(); s.WriteByte(1); } } EOF - chmod 000 "$RUNNER_TEMP/owen-mixed-locked/locked" 2>/dev/null || true + chmod 000 "$RUNNER_TEMP/owen-mixed-locked/locked" set +e out=$(owen check "$RUNNER_TEMP/owen-mixed-locked" --fail-on-finding 2>&1) rc=$? set -e - chmod 755 "$RUNNER_TEMP/owen-mixed-locked/locked" 2>/dev/null || true + chmod 755 "$RUNNER_TEMP/owen-mixed-locked/locked" echo "$out" if echo "$out" | grep -qi "UnauthorizedAccess\|Unhandled exception"; then echo "FAIL: crashed on the locked subdirectory instead of tolerating it"; exit 1 fi - echo "$out" | grep -q "OWN001" || { echo "FAIL: expected the readable source's leak to still be found"; exit 1; } + [ "$rc" -eq 1 ] || { echo "FAIL: expected exit 1 (the readable source's leak), got $rc"; exit 1; } + echo "$out" | grep -q "Real.cs" || { echo "FAIL: readable source was not analyzed"; exit 1; } + if echo "$out" | grep -q "Secret.cs"; then + echo "FAIL: the inaccessible source was unexpectedly analyzed -- chmod 000 did not actually lock it out, so this run proves nothing"; exit 1 + fi - name: Uppercase extensions are accepted case-insensitively (Foo.CS) run: | mkdir -p "$RUNNER_TEMP/owen-uppercase" diff --git a/docs/notes/owen-public-facade.md b/docs/notes/owen-public-facade.md index 14d74241..819bb44a 100644 --- a/docs/notes/owen-public-facade.md +++ b/docs/notes/owen-public-facade.md @@ -79,7 +79,7 @@ Per the guardrail this rebrand was scoped to: no mass rename. language-neutral at the product level ("Owen finds lifetime and resource-contract bugs") while explicitly listing what this distribution actually wires up today: - ``` + ```text Included frontend: .NET / C# (.cs, .csproj, .sln) ``` diff --git a/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs b/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs index 47e2491c..074bd56e 100644 --- a/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs +++ b/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs @@ -116,8 +116,20 @@ public static string EnsureUnpacked() // (same fingerprint), so just use it. return finalRoot; } - Directory.CreateDirectory(finalRoot); - Directory.Move(tempOwnlang, finalOwnlang); + try + { + Directory.CreateDirectory(finalRoot); + Directory.Move(tempOwnlang, finalOwnlang); + } + catch (IOException) when (Directory.Exists(finalOwnlang)) + { + // Narrower version of the same race (review, PR #246): a concurrent + // process created finalOwnlang between the check above and this + // Move. Same reasoning -- their content is provably identical + // (same fingerprint), so just use it instead of surfacing the + // IOException Move throws for an existing destination. + return finalRoot; + } return finalRoot; } finally @@ -154,6 +166,7 @@ private static string Fingerprint(IReadOnlyList files) writer.Write(contentBytes); } writer.Flush(); - return Convert.ToHexString(sha.ComputeHash(buffer.ToArray())); + buffer.Position = 0; + return Convert.ToHexString(sha.ComputeHash(buffer)); } } From 5f8e9ef7de2ec6240d896aea8213dcf5fb61affe Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 12 Jul 2026 15:17:16 +0000 Subject: [PATCH 5/5] Owen facade: verify the CURRENT cache's actual content, not just its existence Round-4 review finding: a hit at the content-addressed ~/.owen/core///ownlang path was trusted on Directory.Exists alone, and the two concurrent-publisher race checks around the atomic move made the same assumption. A path name is only a claim, not proof -- a destination already sitting at the "right" fingerprint could be modified, corrupted, or hand-assembled after publication and would then be served as valid. Every current-path hit now recomputes the fingerprint over what is actually on disk and only trusts an exact match, the same standard the legacy-cache fallback already held itself to. A mismatch quarantines the invalid destination (atomic rename to a .invalid- sibling, then best-effort recursive delete -- no in-place delete a concurrent reader could observe mid-way) and falls through to the same temp-directory + atomic-move rebuild a fresh unpack uses. Added the round-4 regression to ci.yml: create the current cache, tamper ownir.py and add a stale_module.py directly under the fingerprint-named path, and assert the second run rejects it, rebuilds clean, and the stale file is gone from whatever cache directory actually got used. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01SSXTDuh1ZHdQc4QqmYwshw --- .github/workflows/ci.yml | 24 +++++ docs/notes/owen-public-facade.md | 25 +++++ frontend/roslyn/OwnSharp.Cli/CoreVendor.cs | 101 +++++++++++++++++---- 3 files changed, 131 insertions(+), 19 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3c38b8bf..b6d13f7c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2292,5 +2292,29 @@ jobs: echo "$out" [ "$rc" -eq 1 ] || { echo "FAIL: expected exit 1 for uppercase .CS extension, got $rc"; exit 1; } echo "$out" | grep -q "OWN001" || { echo "FAIL: expected OWN001"; exit 1; } + - name: A tampered CURRENT (fingerprint-named) cache is rejected and rebuilt + # Review, PR #246 round 4 -- the earlier fixes verified a LEGACY cache's + # actual content, but a hit at the current ~/.owen/core/// + # path itself was still trusted on existence alone. Reproduction: run once + # to create it, then tamper the file content directly under that exact + # fingerprint-named path (not the legacy location) and add a stale extra + # file, then confirm the second run rejects it, rebuilds cleanly, and the + # stale file is gone from whatever cache directory actually got used. + run: | + rm -rf "$HOME/.owen" "$HOME/.ownsharp" + owen check "$RUNNER_TEMP/owen-sample" --fail-on-finding > /dev/null 2>&1 || true + cache_dir=$(find "$HOME/.owen/core" -mindepth 2 -maxdepth 2 -type d) + [ -n "$cache_dir" ] || { echo "FAIL: first run did not create a current-cache directory"; exit 1; } + echo "# tampered" >> "$cache_dir/ownlang/ownir.py" + echo "# stale leftover module" > "$cache_dir/ownlang/stale_module.py" + driver=$(owen check "$RUNNER_TEMP/owen-sample" --format sarif | python -c "import json,sys; print(json.load(sys.stdin)['runs'][0]['tool']['driver']['name'])") + [ "$driver" = "Owen" ] || { echo "FAIL: driver name '$driver' after rebuild"; exit 1; } + if find "$HOME/.owen" -name "stale_module.py" | grep -q .; then + echo "FAIL: the stale extra file survived under a used cache directory"; exit 1 + fi + if grep -q "# tampered" "$cache_dir/ownlang/ownir.py" 2>/dev/null; then + echo "FAIL: the tampered file content is still being served from the original path"; exit 1 + fi + echo "OK: tampered current-cache destination rejected and rebuilt clean" - name: Clean up cache state left by the edge-case tests above run: rm -rf "$HOME/.owen" "$HOME/.ownsharp" diff --git a/docs/notes/owen-public-facade.md b/docs/notes/owen-public-facade.md index 819bb44a..22a336a1 100644 --- a/docs/notes/owen-public-facade.md +++ b/docs/notes/owen-public-facade.md @@ -157,6 +157,23 @@ Per the guardrail this rebrand was scoped to: no mass rename. written to, moved, or deleted by this code — content-addressing didn't change that guardrail, only how "is this destination actually still correct" gets decided. + **Correction (review, PR #246 round 4):** round 3 still trusted a hit at + the CURRENT fingerprint-named path (`Directory.Exists(finalOwnlang)`) on + existence alone — the concurrent-publisher race checks did too. But a + path name is only ever a claim; nothing stopped a destination already + living at the "right" fingerprint from being modified, corrupted, or + hand-assembled after the fact, and that content would then be served as + if it were still the exact bytes the fingerprint names. Every current- + path hit (the initial check, and both concurrent-publisher checks around + the atomic move) now recomputes the fingerprint over what is actually on + disk there and only trusts it on an exact match, exactly like the legacy + fallback already did. A mismatch quarantines the invalid destination — + an atomic rename to a `.invalid-` sibling (so no concurrent reader + ever observes an in-place delete mid-way), then a best-effort recursive + delete of the renamed copy — and falls through to the same temp- + directory-plus-atomic-move rebuild a fresh unpack uses; since the source + didn't change, the rebuild lands back at the identical fingerprint-named + path, now holding a verified copy. ## Tests @@ -215,6 +232,14 @@ before being encoded as assertions: round 2, now with a real permission-denied subdirectory to exercise it. - **Uppercase extension (`Leak.CS`)** asserts exit 1 with `OWN001` found — the `StringComparer.OrdinalIgnoreCase` fix from review round 3. +- **A tampered CURRENT (fingerprint-named) cache is rejected and rebuilt** + (review round 4) — runs once to create `~/.owen/core///`, + tampers `ownlang/ownir.py`'s bytes and adds an `ownlang/stale_module.py` + directly under that exact path (not the legacy location the earlier + tests already covered), then asserts the second run's SARIF driver is + still `Owen`, `stale_module.py` is gone from whatever cache directory + actually got used, and `ownir.py` no longer contains the tampered + content anywhere under `~/.owen`. ## PR separation diff --git a/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs b/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs index 074bd56e..29b16c9d 100644 --- a/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs +++ b/frontend/roslyn/OwnSharp.Cli/CoreVendor.cs @@ -35,6 +35,18 @@ namespace OwnSharp.Cli; /// falls through to a fresh unpack. This is still a plain fallback *read*, /// not a migration subsystem: the legacy location is never written to, /// moved, or deleted by this code. +/// +/// A hit at the CURRENT (fingerprint-named) path is verified the same way +/// (review, PR #246 round 4) — the path's name is only ever a claim, not +/// proof; something could have modified, corrupted, or hand-assembled a +/// directory that happens to sit at the "right" fingerprint since it was +/// published. is recomputed over what is actually +/// there on every hit (both the initial existence check and the +/// concurrent-publisher race checks further down) and only trusted on an +/// exact match; a mismatch quarantines the invalid destination (an atomic +/// rename out of the way, then best-effort delete — never an in-place +/// delete a concurrent reader could observe mid-way) and falls through to +/// the same temp-directory + atomic-move rebuild used for a fresh unpack. /// internal static class CoreVendor { @@ -62,12 +74,21 @@ public static string EnsureUnpacked() var finalRoot = Path.Combine(versionRoot, fingerprint); var finalOwnlang = Path.Combine(finalRoot, "ownlang"); - // Content-addressed: existence at the fingerprint-named path IS proof of - // validity (see the atomic-publish note below) -- no separate marker to - // go stale or drift from the directory's actual contents. + // Content-addressed cache hit: verify the DESTINATION's actual content, + // not just its existence at the fingerprint-named path (review, PR #246 + // round 4) -- a directory living under the "right" path is not proof it + // still holds the exact bytes that path name claims; only recomputing + // the fingerprint over what is actually there is. A mismatch means this + // path is invalid -- content-addressing has no business trusting it (it + // is not a "different, still-valid" cache the way a different + // fingerprint would be) -- quarantine it and fall through to rebuild. if (Directory.Exists(finalOwnlang)) { - return finalRoot; + if (DestinationMatches(finalOwnlang, fingerprint)) + { + return finalRoot; + } + QuarantineInvalidDestination(finalRoot); } // Legacy fallback: verify the LEGACY DESTINATION's actual content, not a @@ -75,14 +96,9 @@ public static string EnsureUnpacked() // unpack happened here once", never that nothing since removed, added, // or modified a file in that directory. var legacyOwnlang = Path.Combine(userProfile, ".ownsharp", "core", ToolVersion.Current, "ownlang"); - if (Directory.Exists(legacyOwnlang)) + if (Directory.Exists(legacyOwnlang) && DestinationMatches(legacyOwnlang, fingerprint)) { - var legacyFiles = SortedPyFiles(legacyOwnlang); - var legacyFingerprint = Fingerprint(legacyFiles); - if (legacyFingerprint == fingerprint) - { - return Path.Combine(userProfile, ".ownsharp", "core", ToolVersion.Current); - } + return Path.Combine(userProfile, ".ownsharp", "core", ToolVersion.Current); } // Fresh unpack: build into a temp sibling, verify the DESTINATION's own @@ -111,10 +127,17 @@ public static string EnsureUnpacked() if (Directory.Exists(finalOwnlang)) { - // Lost a race with a concurrent `owen` process that published the - // same fingerprint first -- their content is provably identical - // (same fingerprint), so just use it. - return finalRoot; + // Possibly lost a race with a concurrent `owen` process that + // published the same fingerprint first -- but only trust that if + // ITS destination actually verifies (review, PR #246 round 4). + // Existence proves nothing about a path anyone (or anything) + // could have written to since; "same fingerprint-named path" is + // not the same claim as "same, provably identical content". + if (DestinationMatches(finalOwnlang, fingerprint)) + { + return finalRoot; + } + QuarantineInvalidDestination(finalRoot); } try { @@ -125,10 +148,16 @@ public static string EnsureUnpacked() { // Narrower version of the same race (review, PR #246): a concurrent // process created finalOwnlang between the check above and this - // Move. Same reasoning -- their content is provably identical - // (same fingerprint), so just use it instead of surfacing the - // IOException Move throws for an existing destination. - return finalRoot; + // Move. Same verification requirement as above -- only accept it + // if it actually matches; otherwise quarantine it and retry the + // move once with our own already-verified tempOwnlang copy. + if (DestinationMatches(finalOwnlang, fingerprint)) + { + return finalRoot; + } + QuarantineInvalidDestination(finalRoot); + Directory.CreateDirectory(finalRoot); + Directory.Move(tempOwnlang, finalOwnlang); } return finalRoot; } @@ -144,6 +173,40 @@ public static string EnsureUnpacked() private static List SortedPyFiles(string dir) => Directory.EnumerateFiles(dir, "*.py").OrderBy(f => Path.GetFileName(f), StringComparer.Ordinal).ToList(); + /// True only if every .py file actually on disk under + /// right now fingerprints to + /// (review, PR #246 round 4). This + /// is the sole source of truth for "is this destination still valid" -- + /// a directory's location (even a content-addressed, fingerprint-named + /// one) is only ever a claim about what was published there once, never + /// proof of what is there now. + private static bool DestinationMatches(string ownlangDir, string expectedFingerprint) => + Fingerprint(SortedPyFiles(ownlangDir)) == expectedFingerprint; + + /// Moves an invalid cache destination out of the way of a rebuild + /// (review, PR #246 round 4). Renames first -- an atomic same-volume + /// rename can't be observed half-done the way an in-place recursive + /// delete could -- then best-effort deletes the renamed copy; a failure + /// there just leaves inert garbage that is never consulted again (the + /// quarantined name is never re-derived by ), + /// same reasoning as the orphaned-temp-directory cleanup above. + private static void QuarantineInvalidDestination(string invalidRoot) + { + var quarantined = $"{invalidRoot}.invalid-{Guid.NewGuid():N}"; + try + { + Directory.Move(invalidRoot, quarantined); + } + catch (IOException) + { + // Lost a race with something else already handling this exact path + // (e.g. a concurrent process's own quarantine of the same invalid + // directory) -- nothing more to do; the caller re-checks fresh. + return; + } + try { Directory.Delete(quarantined, recursive: true); } catch (IOException) { /* best-effort cleanup */ } + } + /// SHA-256 over every file's name and content, each explicitly /// length-prefixed (review, PR #246) so two different (name, content) sets /// can never hash identically by having their bytes merely concatenate the