fix(security): harden local analysis boundaries by zhangjinzan1 · Pull Request #35 · QoderAI/better-harness

zhangjinzan1 · 2026-07-31T06:52:48Z

Summary

make the cloc direct-entrypoint check canonical and filesystem-safe for spaced or symbolic installation paths
reject Checkup source patches when either the source path or backup tree contains a symbolic-link component or resolves outside its authorized root
expose Secret Scan coverage as complete, partial, or failed, including nested symbolic links and safety-sensitive read skips
redact GitLab personal access tokens and userinfo credentials from hierarchical URLs across HTTP, database, SSH, percent-encoded, and username/token-only forms
add regression tests, tighten the traceability spec, and record the maintainer-requested task-scope rule in AGENTS.md

Motivation and context

These fixes close confirmed reliability and local security boundary failures:

scripts/cloc/cli.mjs compared non-canonical paths, so valid direct invocations through spaced or equivalent symbolic paths could exit successfully without running the CLI.
Checkup validated the source target but could still write backups through a symbolic-link backup root outside the authorized workspace.
Secret Scan could silently ignore nested symbolic links or safety-sensitive read skips and report complete clean coverage.
Session-safe summaries covered only HTTP(S)/FTP user:password@ URLs and retained credentials in database, SSH, percent-encoded, or token-only userinfo forms.
User-visible behavior alone was being treated as authorization to edit release metadata.

No host support, scoring, report-model, Preview, release automation, package manifest, or changelog behavior is changed.

Test plan

Node 22.20.0 npm test — 1031/1031 passed
Focused regression suite — 118/118 passed
npm run pack:verify — npm 359 entries, runtime zip 382 entries
node --test test/doc-link-graph.test.mjs — 6/6 passed
cd docs && npm ci && npm run build — English and Simplified Chinese builds passed
Secret Scan over all 10 final changed files — complete coverage, 0 findings, 0 errors

Compatibility and downstream impact

Secret Scan machine consumers may receive exit code 3 when scanning is incomplete. Exit code 0 remains complete-and-clean; exit code 2 remains findings at or above the configured threshold.
Source and backup paths intentionally reject symbolic-link components instead of following them.
URL userinfo is redacted more broadly in review-safe text.
No dependency or package-manifest changes.

Risk and rollback

Main risk is stricter behavior for workflows that previously relied on symbolic-link mutation or backup paths, or ignored incomplete Secret Scan coverage.
Canonical entrypoint behavior and symlink fixtures are covered by macOS/Linux local tests and the repository's Windows/macOS/Linux CI matrix.
Rollback is a single revert of this PR; no migration or persistent schema change is involved.

AI assistance

AI-assisted implementation and review were used. The amended commit contains the repository-required single Co-authored-by line.

Checklist

I reviewed AGENTS.md and the traceability spec.
I added regression coverage for every requested-change thread.
I ran the complete local verification gates.
I kept CHANGELOG.md, version files, and package metadata out of the final diff.
I did not include credentials, tokens, private keys, passwords, or personal filesystem paths.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

phodal

Requesting changes on head 3f6a1e9. The overall direction and traceability spec are sound, but the current implementation still leaves three security/privacy boundary failures and one silent CLI reliability failure. Each issue below was reproduced against this exact commit even though the focused tests and CI pass.

Please address all four threads and add regression coverage for each scenario. Also amend the commit with the repository-required single Co-authored-by line using the actual AI agent/model, then rerun npm test, npm run pack:verify, the docs build, and the final changed-file secret scan.

phodal · 2026-07-31T11:38:09Z


  if (stat.isSymbolicLink()) {
    stats.skippedFiles += 1;
+    stats.errors.push(`${absPath}: symbolic-link scan targets are not inspected`);


[P1] Record nested symbolic links as incomplete coverage

This branch is reached only when collectFiles() is called directly with a symbolic-link path. During directory traversal, the Dirent loop handles only directories and regular files, so nested symbolic links are silently ignored. A directory containing only a symlink currently reports coverageStatus: \"complete\", scannedFiles: 0, no errors, and exits 0.

Please record nested links as skipped/incomplete (and do the same for safety-related read skips), then add a regression that scans a directory containing a symbolic link rather than passing the link as the explicit CLI path.

phodal · 2026-07-31T11:38:09Z

  const relativeBackup = backupName(sourceRef, options.now);
  const backupPath = path.join(backupRoot, relativeBackup);
  await mkdir(path.dirname(backupPath), { recursive: true });
+  await assertSafeSourceTarget(resolved);


[P1] Validate the backup write path as well as the source path

assertSafeSourceTarget(resolved) protects the source file, but backupPath is still created and written without canonical containment or component-level symlink checks. If .better-harness-checkup-backups is a symlink to a directory outside the workspace, this patch succeeds and copyFile() writes the backup outside the authorized root.

Please create/validate the backup tree without following symbolic-link components, verify its real path remains under the canonical workspace, and add a regression for a symlinked backup root.

phodal · 2026-07-31T11:38:09Z

    .replace(/\b(api[_-]?key|access[_-]?token|auth[_-]?token|password|secret)\s*[:=]\s*[^\s,;]+/giu, "$1=<redacted>")
    .replace(/\b(?:sk|ghp|github_pat|xox[abprs])[-_][A-Za-z0-9_-]{8,}\b/giu, "<secret>")
+    .replace(/\bglpat-[A-Za-z0-9_-]{20,}\b/giu, "<secret>")
+    .replace(/\b((?:https?|ftp):\/\/)[^\s/@:]+:[^\s/@]+@/giu, "$1<redacted>@")


[P1] Redact userinfo credentials for non-HTTP URL schemes

This pattern only covers HTTP(S)/FTP URLs with a user:password@ form. Common credential-bearing values such as postgresql://db-user:db-password@... and ssh://build-user:ssh-password@... keep both credentials in the review-safe result; later path redaction removes only the host/path.

Please redact userinfo for all supported hierarchical URL schemes (including username/token-only forms where applicable) and add cases for database URLs, SSH URLs, and percent-encoded userinfo.

phodal · 2026-07-31T11:38:09Z

 }

-if (process.argv[1] && import.meta.url.endsWith(process.argv[1].replaceAll("\\", "/"))) {
+if (process.argv[1] && path.resolve(process.argv[1]) === path.resolve(fileURLToPath(import.meta.url))) {


[P2] Canonicalize both entrypoint paths before comparing them

path.resolve() is lexical and does not resolve symbolic links. On macOS, invoking this exact file through the /tmp alias while the module resolves under /private/tmp still produces no output and exits 0. The current spaced-path test places only the analyzed --cwd under a spaced path; the CLI file itself remains at the normal repository path.

Please compare canonical real paths and add a regression where the CLI installation path itself contains spaces and is invoked through an equivalent symlinked path.

Close the remaining PR QoderAI#35 review gaps by canonicalizing cloc entrypoints, treating nested and safety-related scan skips as incomplete, containing Checkup backups, and redacting hierarchical URL userinfo. Also record the maintainer-requested task-scope rule and remove the proactive changelog entry. Implements AC-1 through AC-6 in docs/specs/2026-07-31-security-reliability-boundaries.md. Validated with Node 22.20.0 npm test (1031 tests), npm run pack:verify, the docs build, doc-link tests, and a complete changed-file secret scan. Co-authored-by: Codex (GPT 5.6 Sol) <codex@openai.com>

phodal requested a review from Copilot July 31, 2026 06:58

Copilot AI reviewed Jul 31, 2026

phodal requested changes Jul 31, 2026

View reviewed changes

phodal force-pushed the fix/security-reliability-boundaries branch from 3f6a1e9 to e7b326e Compare July 31, 2026 13:48

phodal merged commit 56c7d14 into QoderAI:main Jul 31, 2026
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(security): harden local analysis boundaries - #35

fix(security): harden local analysis boundaries#35
phodal merged 1 commit into
QoderAI:mainfrom
zhangjinzan1:fix/security-reliability-boundaries

zhangjinzan1 commented Jul 31, 2026 •

edited by phodal

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

phodal left a comment

Uh oh!

phodal Jul 31, 2026

Uh oh!

phodal Jul 31, 2026

Uh oh!

phodal Jul 31, 2026

Uh oh!

phodal Jul 31, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

zhangjinzan1 commented Jul 31, 2026 • edited by phodal Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation and context

Test plan

Compatibility and downstream impact

Risk and rollback

AI assistance

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

phodal left a comment

Choose a reason for hiding this comment

Uh oh!

phodal Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

phodal Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

phodal Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

phodal Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zhangjinzan1 commented Jul 31, 2026 •

edited by phodal

Loading