Skip to content

Make adoption diagnostics share-safe by default - #1102

Merged
jeffhuber merged 4 commits into
mainfrom
codex/1084-share-safe
Sep 21, 2026
Merged

jeffhuber merged 4 commits into
mainfrom
codex/1084-share-safe

Conversation

@jeffhuber

@jeffhuber jeffhuber commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Closes #1084.

Hosted and adoption doctor output could expose config, checkout, executable, LaunchAgent, workflow, and packaged-template paths when copied into an issue or adoption report. This change keeps the existing report fields and check IDs while making adoption, supervised-pilot, hosted-builder, and orchestrator postures share-safe by default. JSON states local_paths: redacted; --share-safe makes that policy explicit for maintained prompts, while --include-local-paths returns the exact legacy report.as_dict() object for private local debugging.

The redactor covers nested values and keys; path-bearing config, executable, workflow-path, template-path, file, and directory fields; authority-bearing and local file: URIs; absolute, home-relative, Unicode-rooted, relative, Windows drive, rooted Windows, and UNC spellings. Non-file URLs, including URL query paths and SSH URLs, and OWNER/REPO slugs remain readable. Text, JSON, remediation, and config-load error surfaces use the same boundary. Embedded paths are replaced without truncating the diagnostic prose that follows them, including when paths contain spaces. No cloud schema or upload path changes.

The standalone materializer now includes doctor_checks/share_safe.py, and the committed package inventory is regenerated. A scratch standalone tree imports code_mower.doctor successfully, and both the wheel and sdist contain the module.

Maintained install, quickstart, launch-surface, and orchestrator prompt commands request --share-safe explicitly and document the local-debug opt-in.

Validation at exact head 3b589d523c32b6b6cb517f8ca0f162559d56c5dc:

  • 246 doctor and supervised-pilot tests passed with 244 subtests.
  • 13 package materializer/manifest/standalone tests passed with 252 subtests.
  • Standalone materialization and isolated code_mower.doctor import passed.
  • Source wheel and sdist built successfully; both contain doctor_checks/share_safe.py.
  • Ruff, repository privacy scan, and diff hygiene passed.

This PR remains draft pending new independent exact-head Codex and Claude audits plus the full CI/gate matrix.

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Code Mower exact-head Codex audit — BLOCKED

Exact head: 0bbccddbb6f18769f08455044ac125a0a5a90a7c

Findings: P0 0 / P1 2 / P2 0

  1. [P1] The standalone package materializer omits the new required module. doctor_checks/__init__.py imports doctor_checks.share_safe, but src/code_mower/package_manifest.py does not include src/code_mower/doctor_checks/share_safe.py. The repository's inventory guard fails at test_package_materializer_includes_internal_package_seams. A direct rehearsal confirms the impact: after code_mower.package --output-dir <temp>, the file is absent and importing code_mower.doctor from that materialized tree raises ModuleNotFoundError: No module named 'code_mower.doctor_checks.share_safe'. Add the module to the materializer manifest and regenerate/verify the committed package inventory so generated standalone packages remain runnable.

  2. [P1] --share-safe still emits several valid local path forms verbatim. The regex boundary only recognizes authority-free file:///... URIs and a subset of absolute ASCII-rooted paths. The following synthetic inputs remain visible: file://server/share/private/workflow.yml, file://localhost/Users/alice/private.yml, confidential/config.yml, .github/workflows/private-audit.yml, and rooted Windows \Users\alice\private.yml; /秘密/config.yml leaks its first path segment. These forms can occur in the report's known config/executable/workflow/template fields, nested details, messages, remediations, and errors, so the output is not yet share-safe by construction. Preserve https://... and OWNER/REPO, but redact path-bearing fields and complete the URI/POSIX/Windows matrix with positive and negative fixtures.

Verification performed against the immutable head:

  • 56 focused share-safe/output/registry/supervised-pilot tests passed.
  • A real hosted/orchestrator JSON run redacted this checkout, and --include-local-paths restored the exact legacy top-level key set and values.
  • Source wheel/sdist build passed, while the standalone materialization/import rehearsal failed as described above.
  • Full doctor-focused discovery exposed the package inventory failure.
  • Diff inspection confirmed no cloud schema or upload-boundary changes.

The needs-codex-audit label should remain until both P1s are fixed and a new exact head is reviewed.

@github-actions

github-actions Bot commented Sep 21, 2026

Copy link
Copy Markdown

Claude audit (merge-authority lane)

Head SHA: 0bbccddbb6f18769f08455044ac125a0a5a90a7c
Verdict: BLOCKED
Review details remain in the local audit artifact.
Publication workflow: .github/workflows/local-audit-publication.yml at 052d49452605eaf668fb07f385673fe5e25b303f

@jeffhuber

jeffhuber commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

P1 fix evidence

Exact head: 3b589d523c32b6b6cb517f8ca0f162559d56c5dc

Both findings from the exact-head Codex audit are addressed:

  1. The standalone source manifest and regenerated committed inventory include src/code_mower/doctor_checks/share_safe.py. A scratch materialization contains the file and imports code_mower.doctor successfully. The built wheel and sdist also contain the module.
  2. Share-safe rendering now covers authority-bearing and local file: URIs, relative and dot-relative paths, Unicode POSIX roots, Windows drive/drive-relative/rooted/UNC paths, and ambiguous values in known path-bearing fields. Non-file URLs and OWNER/REPO remain intact. --include-local-paths is regression-tested as exact equality with the legacy report.as_dict() payload.
  3. The exact-head Claude finding is addressed: replacing an embedded path no longer truncates the diagnostic text after it. Quoted paths and unquoted multi-part paths containing spaces remain hidden while following prose is preserved.

Validation:

  • 246 passed, 244 subtests passed across tests/test_doctor_*.py and tests/test_supervised_pilot_contract.py
  • 13 passed, 252 subtests passed for package materializer/manifest/standalone selections
  • Ruff: passed
  • repository privacy scan: passed
  • git diff --check: passed
  • wheel and sdist build: passed
  • standalone materialization/import rehearsal: passed

The PR remains draft with needs-codex-audit and needs-claude-audit for new exact-head review. It has not been merged.

@github-actions

github-actions Bot commented Sep 21, 2026

Copy link
Copy Markdown

Claude audit (merge-authority lane)

Head SHA: 6c012942771c3c98a47fb1ea47f5e07209fa6d62
Verdict: BLOCKED
Review details remain in the local audit artifact.
Publication workflow: .github/workflows/local-audit-publication.yml at 052d49452605eaf668fb07f385673fe5e25b303f

@github-actions

github-actions Bot commented Sep 21, 2026

Copy link
Copy Markdown

Claude audit (merge-authority lane)

Head SHA: 3b589d523c32b6b6cb517f8ca0f162559d56c5dc
Verdict: PASS
Review details remain in the local audit artifact.
Publication workflow: .github/workflows/local-audit-publication.yml at 052d49452605eaf668fb07f385673fe5e25b303f

@jeffhuber

Copy link
Copy Markdown
Contributor Author

Code Mower exact-head Codex audit — PASS

Exact head: 3b589d523c32b6b6cb517f8ca0f162559d56c5dc

Findings: P0 0 / P1 0 / P2 0

The whole patch satisfies issue #1084. The two prior P1s are resolved: doctor_checks/share_safe is present in the standalone materializer, committed inventory, wheel, and sdist; and the share-safe boundary covers the previously escaping authority-bearing file: URIs, relative and dot-relative paths, Unicode POSIX roots, Windows drive/drive-relative/rooted/UNC forms, nested/path-bearing fields, config errors, text, JSON, and remediation surfaces while preserving non-file URLs and OWNER/REPO. The later Claude truncation concern is resolved: quoted and unquoted paths with spaces are hidden while following diagnostic prose remains intact. --include-local-paths returns exact report.as_dict() legacy payload equality, including the original top-level key set. No cloud schema or upload boundary changed.

Independent validation from a clean detached exact-head clone: 246 doctor/supervised-pilot tests passed with 244 subtests; 14 package/materializer/manifest tests passed with 641 subtests; standalone materialization plus isolated code_mower.doctor import passed; wheel and sdist builds passed and both contain doctor_checks/share_safe.py; adversarial path/URL/slug checks, Ruff, privacy scan, and diff hygiene passed. CodeRabbit reviewed all 12 changed files with 0 findings. The live PR head was rechecked immediately before publication.

@jeffhuber
jeffhuber marked this pull request as ready for review September 21, 2026 15:46
@jeffhuber
jeffhuber merged commit 4a6148c into main Sep 21, 2026
19 checks passed
@jeffhuber
jeffhuber deleted the codex/1084-share-safe branch September 21, 2026 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

builder:codex Code Mower generated label claude-audit-done

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v1.6.0: provide share-safe adoption diagnostics

1 participant