Repository navigation
Close the triaged findings from the nightly review and docs-sync runs - #406
Merged
Merged
Conversation
- The legacy hash forwarder followed a hash like #///evil.com off-origin, because an empty first segment skipped the docs branch and joined back into a protocol-relative URL. It now goes to the site root, and the hostile cases are in the test table. - The site .htaccess named this project's host in a file every build copies verbatim, so a deploy for another origin 301-redirected that origin's visitors here. The canonical host now comes from the build's origin; an origin under a path drops the rule and upgrades to HTTPS on whichever host answered. - The SSRF pre-check accepted a host as soon as one resolved address was usable, which holds only while the guarded connect callback filters the rest. A proxied request is resolved by the proxy, so it now requires every address to pass. - The desktop uninstaller deleted %SystemRoot%\System32\logs once it was empty. Only the nodepilot-*.log files are ours; the directory is shared. - docs/images/og-image.png is re-encoded losslessly at 340,185 bytes, below the 367,397 it had before the commit that claimed to shrink it.
- DocumentationCountsTests pinned one of the README's three activity counts; the other two drifted to 29 while the guarded one stayed at 27. Both are pinned now. - The CLAUDE.md guard table gains the TestRunSettingsTests row, so a seventh test project points at the coverage.runsettings comment it invalidates. - expectOk now prefers the server's message over the status text, a contract nothing asserted. Four cases cover it, plus a toast case for a refused alerting delete.
Supersedes the rule added one commit earlier. Author and committer stay Sev7eNup, so that avatar is first on every commit; a Claude, Codex or Dependabot trailer may follow it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vite.site.config.ts is type-checked under tsconfig.node.json, which knows neither Node's built-ins nor __dirname, so the file I/O broke the docs-ui build. It now sits in scripts/htaccess.mjs next to the prerender plugin, which is there for the same reason, with a .d.mts for the config's import. The test reads the file through Vite's ?raw instead of node:fs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| /// pre-check is the only address filter left. Demanding that every resolved address pass | ||
| /// keeps a name that mixes a routable record with a link-local one out. | ||
| /// </param> | ||
| public static void ValidateUrl(IConfiguration config, string url, bool requireAllAddresses) |
This was referenced Sep 22, 2026
Closed
Sev7eNup
added a commit
that referenced
this pull request
Sep 27, 2026
…eck) Automated documentation-drift sync. Both claims were verified against the checked-out code on main, not inferred from a diff. CLAUDE.md - markdown exception lists the skipped CI jobs (from #409) The sentence enumerates which jobs a markdown-only PR skips, and named Frontend, Desktop and E2E. #409 added the `database` job, gated on the same `needs.changes.outputs.code == 'true'` as those three, so it skips with them. Database is added to the list. docs/claude-reference.md - RestApi:AllowedHosts address rule (from #406) The row is specifically about targets that go through a proxy, and stated that one usable address suffices when a host resolves to several, citing NetworkGuard.AssertAnyAddressUsable and the per-address re-check in EnforceConnect. #406 inverted exactly that case: the method is now AssertAddressesUsable, and for a proxied destination every resolved address must pass, because the proxy resolves the destination DNS itself and EnforceConnect never sees those addresses. A host whose DNS mixes a routable record with a link-local one is now rejected when proxied, so the row told an operator the opposite of what the guard does. The one-address rule is scoped to direct targets and the proxied rule spelled out, naming the requireAllAddresses flag and RestApiHttpClientProvider.UsesProxyForDestination. Text only; no code, config or test changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0122Ukd5gDBxgLcUPeCPQn7W
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Works through the open triage issues. Each item below is verified against the code; findings that turned out to be already fixed on main, or that ask for an editorial decision, are listed at the end instead of guessed at.
Fixed
#395 — open redirect in the legacy hash forwarder (high). A hash like
#///evil.comleft an empty first segment, which skipped thedocs/branch and joined back into a protocol-relative URL thatlocation.replacefollowed off-origin. It now goes to the site root. The hostile cases are inlegacy-redirect.test.ts.#395 — hardcoded canonical host in
.htaccess(medium). The file ships as a static asset, so a deploy for another origin 301-redirected that origin's visitors towww.nodepilot.run— and a 301 outlives the fix. The host now comes from the build's origin. An origin under a path (a Pages project site) names no canonical host, so that rule is dropped and the HTTPS upgrade keeps whichever host answered. Newhtaccess.test.ts.#395 — untested error-reporting change (medium).
expectOkprefers the server's message over the status text; nothing asserted it. Four cases cover the contract, plus a case that a refused alerting delete raises the toast with the server's message.#385 — SSRF pre-check on the proxied path (medium). Accepting a host as soon as one resolved address is usable holds only while the guarded connect callback filters the rest. A proxied request is resolved by the proxy, so
ValidateUrlnow requires every address to pass when the request will be proxied. Both call sites pass it;WebhookNotificationSinkincluded, which had no destination check at all.#385 — uninstaller deletes a System32 directory (medium).
%SystemRoot%\System32\logsis shared with anything that runs with System32 as its working directory. Only thenodepilot-*.logfiles are removed now.#379 — og-image grew in the commit that claimed to shrink it. Re-encoded losslessly at 340,185 bytes, below the 367,397 it had before that commit. Pixels are identical.
#404 — two README activity counts sat outside the guard. Both are pinned now, which is what let them drift to 29 while the guarded one stayed at 27.
#392 —
TestRunSettingsTestsmissing from the guard table. Added, so a seventh test project points at thecoverage.runsettingscomment it invalidates.Already fixed on main
#379 —
np-inputis gone (the class now only appears in a test asserting it is not used), the duration-trend options query is already gated withenabled: !query.data, and the misplaced DTO summary from #375 is back on its own record.Left for a decision
#400, #391, #386 and the remaining items of #375 and #379 ask for authoring or a product call — manual E2E parts, AV exclusion priorities and residual risk, whether
Stats:Rollup:Enabledbelongs in the admin UI, and whether to run theDatabaseIntegrationcategory in CI. Each issue states why, and none is a factual drift.Noticed in passing
FileWatcherTriggerSourceTests.DisposeAsync_StopsHealthProbefailed on two of the stale PR runs as a timing flake. Not touched here.