Repository navigation
Align browser onboarding with account API permission parity - #659
Conversation
Codier
left a comment
Codier
left a comment
There was a problem hiding this comment.
Review A and dedicated security at d59c2420b24e4e93d819c3f983cbade6f2af0c1b. Changes requested. Factory KimiK3 was explicitly authorized by the user after the prior Factory Opus budget failure. This fresh retry completed the original A round independently; failed substituted runs are preserved but do not count as reviews. All findings, evidence, checks and limitations are retained.
Reviewer A
Factory KimiK3, high reasoning/high autonomy; session2fc7fe87-818c-4c68-b9c9-fdb2cd210c8d.
Intent: Independent general reviewer A (retry) for API508 PR #659: verify the onboarding/docs slice against frozen base 8f9085b, confirm the writer's test/fixture claims, check actual integration and boundaries, assess the stale older-head evidence snapshot against the new root 8f9085b verification, and check the release-prep documents against the new requirement that rollout needs full browser-session access-token invalidation rather than refresh-only retirement.
Verdict: changes_requested.
Findings
A1 P2: docs/releases/api508-runbook.md:38
The release-prep artifacts do not yet encode the newly stated rollout requirement of full browser-session access-token invalidation. The rollback section permits keeping the revocation feed 'until issued access and refresh authority are revoked or expire', so a gate check could be satisfied by refresh-family retirement plus natural access-token expiry. The evidence manifest's nearest acceptance field is revocationFeedRetention (null); no field requires demonstrated invalidation of outstanding access tokens at or after logout/retirement. The packaged guide already admits the gap: a legacy refresh-only receipt means already-issued access can survive until expiry, and a hypothetical access_tokens_revoked receipt remains unconfirmed. Because resource requests are Bearer-only without DPoP, an unexpired access token stays usable until expiry under the current contract, so this gate cannot be waived as implied. Prerequisite is server-side support plus measured evidence through the real gateway; this finding asks for the gate and manifest field to be added to these repository-only NOT_READY documents, not for any live test. Impact if left as-is: a rollout review consuming these artifacts could pass with refresh-only retirement evidence, contrary to the stated requirement.
Evidence: docs/releases/api508-runbook.md:38 ('revoked or expire under the verified protocol'); docs/releases/api508-evidence.json acceptance block has 21 null fields, none covering access-token invalidation (revocationFeedRetention at line 100 is the closest); docs/browser-login.md:65 ('A legacy refresh-only receipt means already-issued access can survive until expiry') and :108 ('Unknown revoke receipts, including hypothetical access_tokens_revoked:true, remain unconfirmed'); docs/browser-login.md:24 ('The resource contract uses Bearer without DPoP; a copied valid access token is accepted by that policy').
A2 P3: docs/browser-login.md:135
The slice retains the pre-rerun evidence snapshot and should be updated to cite root's new 8f9085b verification without fabricating cohort or acceptance. The packaged guide still says native renewal evidence keeps its 1c16839 attribution and 'requires current-head verification', and reports Linux locked refusal at 85 ms; root has since run exactly that current-head verification at the frozen dependency 8f9085b (Mac and isolated Debian/GNOME native 10,295-byte one-rotation, fresh-process read, restart and logout; Linux locked refusal 80 ms; 51 renewal tests at the frozen head) plus 11 public CLI/API synthetic billing cases at CLI 8f9085b / API 9190 covering no-inactive-env refresh and a free zero-credit account after rotation. The same staleness appears in the evidence manifest's two 1c16839 entries ('not reconciled 8f9085b'), the runbook readiness-matrix platform row, and the PR body's Limits paragraph. The update should add the new artifact identifiers and hashes, keep their stated limits (synthetic issuer/retirement/storage, no live ceremony, ledger, gateway, platform or cohort acceptance), and leave candidate-head/package and all other null gates null, since d59c242 differs from 8f9085b and this slice changes no auth code. As written, the release artifacts misstate which named checks still lack current-head evidence.
Evidence: docs/browser-login.md:5 and :135; docs/releases/api508-evidence.json:37-47 (api507-native-renewal-1c16839-result.json, linux-native-refresh-1c16839-result.json with 'Older source head, not reconciled8f9085b'); docs/releases/api508-runbook.md:13; PR #659 body Limits: 'native root evidence is still attributed to its actual older heads (83d6 custody and 1c16839 renewal), not this reconciled head'. Superseding evidence: RUN/API-507-root-8f9085b-native-verification.md and RUN/API-507-root-joint-refresh-verification.md.
A3 P3: scripts/postinstall.js:187
The new direct-entry guard compares import.meta.url to pathToFileURL(process.argv[1]) without resolving symlinks, so invoking the script through a symlinked path silently skips all onboarding (no tips, no skill offer) while still exiting 0. The standard npm lifecycle invocation is unaffected: node scripts/postinstall.js from the package directory prints the non-TTY tips as expected. The failure mode is bounded and failure-tolerant by design, but it is invisible; comparing realpaths (or keeping the guard but logging nothing differently) would remove the edge. No test covers the symlinked invocation.
Evidence: Reproduced locally: npm_config_global=true node scripts/postinstall.js </dev/null prints tips; npm_config_global=true node /private/tmp/api508-guard-check/postinstall-link.js </dev/null (symlink to the same file) prints nothing, exit 0. Guard at scripts/postinstall.js:187-191.
Checks
{
"command": "git merge-base --is-ancestor 8f9085b..d59c242; git rev-parse HEAD; git status --short",
"result": "Base 8f9085b is an ancestor of the exact head d59c242; working tree clean, detached at the reviewed head"
}
{
"command": "GET api.github.com/repos/nansen-ai/nansen-cli/pulls/659 (+ /reviews, /check-runs)",
"result": "Draft PR, base hulk/api-507-session-refresh at 8f9085b, head d59c242, author hulk-linus[bot], 39 files +962/-142, 0 comments, 0 review comments, 0 reviews; only check run is Aikido Security: check code = skipped (draft). No CI pass exists or is claimed"
}
{
"command": "npm test",
"result": "89 files passed, 3507 passed, 9 skipped, 122.38s; matches the writer's claim exactly; pretest changeset check passed (exit 0)"
}
{
"command": "npx vitest run src/tests/browser-onboarding-routes.test.js src/tests/postinstall.test.js",
"result": "158 passed (2 files, 7.89s); matches the writer's focused-test claim"
}
{
"command": "npx vitest run src/tests/package.test.js",
"result": "2 passed standalone (3.86s); packs the tarball, installs with --omit=optional, stubs fetch for all children, asserts packaged recovery doc and absence of docs/releases/api508-evidence.json; only network is npm registry dependency install"
}
{
"command": "npm run lint; git diff --check 8f9085b..d59c242; node scripts/check-changeset.js",
"result": "eslint clean, no whitespace errors, changeset validation exit 0; major changeset retained and patch changeset added"
}
{
"command": "node comparison of src/tests/fixtures/api508-admitted-routes.json vs /Users/dipeng/Nansen/worktrees/api505-research-root-9190/docs/contracts/api505-browser-session-research.json",
"result": "Route sets identical, 58/58, no diff either direction; frozen contract sha256 4c0032ef6d73d622a927147a66f0756adfbd667446ede1e9b556d1ba6c5709f3 matches the fixture's recorded sourceSha256"
}
{
"command": "node count of src/tests/fixtures/api508-skill-commands.json",
"result": "133 literal examples, 23 conditional skills, exactly one non-admitted example (research portfolio defi); matches the writer's 133/23 claims and the plan's skill table"
}
{
"command": "grep scan of the 23 conditional skills for nansen agent/trade/wallet/alerts/web/mcp literals and shell loops",
"result": "No key-only command-family literals in any conditional skill; only two embedded loops (sm-cross-chain-flows CHAINS, wallet-clustering REFERENCE per-chain loop), both matching the expansion tests and admitted routes"
}
{
"command": "DO_NOT_TRACK=1 NANSEN_API_KEY=synthetic node src/index.js research token not-a-command",
"result": "Exits 0 with success:true and data.error 'Unknown subcommand'; confirms the exit-0 hazard is real and the acceptance test's data/error and nonempty-calls assertions are what reject it"
}
{
"command": "Read/traced scripts/postinstall.js, src/auth-credentials.js, src/cli.js login + auth paths, package.json files list",
"result": "Postinstall classifies via shared offline resolveCredential/assertUsableSelection, never opens native storage or network during classification, requires explicit y for the free account check, spawns node <packaged src/index.js> account --pretty with no secret argv; isLoggedIn removed with no remaining refs; packaged imports resolve with optional deps omitted; login --human persists an injected env key without prompting (src/cli.js:1214-1216); auth status keeps not_checked_no_prompt offline semantics"
}
{
"command": "Symlink probe of the postinstall entry guard",
"result": "npm-style relative invocation prints tips; symlinked-path invocation prints nothing and exits 0 (finding A3)"
}
Limitations
No live gateway, deployed builds, controlled accounts, ledger before/after, headroom, revocation propagation, Windows, desktop/daemon-write cancellation, or published v2-compatible recovery version exists to verify; all such gates remain null in the manifest and were not exercised
Dependency review of API507 at 8f9085b is still pending and was not redone here per assignment; renewal/custody behavior was trusted as the frozen base except where this slice's docs make claims about it
The new root 8f9085b native and joint evidence supersedes the older-head qualifier for the named checks only; it is synthetic (fixture issuer/retirement/storage), not live ceremony, ledger, or platform acceptance, and was read as supplied evidence, not rerun by me
The 158 focused tests and the 58-route fixture are dispatch/output evidence through real parser/handlers/NansenAPI.request with locally captured transport; they do not prove server admission, entitlement, or billing
The fixture pins the API505 contract at 9190ae3d; bcbae source equivalence is recorded in the contract docs and was not independently rerun
CI is absent on the stacked base by workflow design (main-only PR filter); the only check on the head is Aikido skipped; this is recorded, not treated as a pass
The standalone package test's only network activity is npm registry dependency installation; no product, issuer, retirement, or analytics traffic left the machine in any check I ran, and no existing user credentials were read by the tested paths (non-TTY branches return before config reads)
Finding A1 reflects a requirement stated after this head was written; assessing whether server support for access-token invalidation exists is out of scope and no live test was run or authorized
Dedicated security
Claude Code Opus5, requested high reasoning; session5030e6b8-4963-432d-badd-f2c721bc093a. SDK does not independently expose effort.
Intent: API-508 onboarding slice on frozen API-507 base 8f9085b: replace postinstall's ad-hoc apiKey sniff with the shared offline credential resolver, gate the free account check behind an explicit yes, align README/help/schema/packaged recovery docs with fresh browser login, environment precedence, automatic renewal and the major selected-key-402 no-autopay change, convert 23 direct-data skills to conditional browser/key guidance while 10 mixed/excluded skills keep their API-key requirement, and add a repository-only NOT_READY release manifest and runbook plus route/postinstall/package regression tests.
Verdict: changes_requested.
Findings
S1 P2: skills/nansen-token-research/SKILL.md:6
23 shipped skills drop the machine-readable requires.env: [NANSEN_API_KEY] and primaryEnv: NANSEN_API_KEY openclaw fields (the metadata published by .github/workflows/sync-clawhub.yml), leaving only prose that hedges 'when browser admission and the documented platform cohort are enabled'. At the sha256-pinned API505 contract both gates are false with requires_both_flags true, and this PR's own docs/releases/api508-evidence.json records every deployment/flag/cohort field null, so no browser session can serve any of these 58 routes in any environment today. The only credential state the metadata now permits in practice is 'none' — and that is exactly the state in which src/api.js:783 sets mayAutoPay (this.selection.kind === 'anonymous'), so a 402 on any of these research routes is auto-signed and settled from the default x402 wallet, up to NANSEN_X402_MAX_AMOUNT (default $1) per payment, repeated per call. Several of these 23 skills loop over chains and address lists. Their new Authentication paragraph asserts 'login does not purchase credits' and never mentions that running them with no credential at all is the one path that does spend wallet funds, so the hidden-payment-fallback boundary the rest of this PR is careful about is reopened for exactly the skills whose declared key requirement was removed. Keep the declared key requirement (or declare an equivalent credential precondition) until both server gates and a named cohort exist, and state the anonymous-x402 spend consequence in the skill text.
Evidence: Metadata removal verified in all 23 skills via git diff 8f9085b d59c242 -- skills/; 23 conditional / 10 key-only split confirmed against src/tests/browser-onboarding-routes.test.js:16-18 and ls skills (34 dirs = 23 + 10 + nansen-mpp-payment). Frozen contract flags read from /Users/dipeng/Nansen/worktrees/api505-research-root-9190/docs/contracts/api505-browser-session-research.json: {"BROWSER_SESSION_ACCOUNT_ENABLED":false,"BROWSER_SESSION_RESEARCH_ENABLED":false}, requires_both_flags=true (file sha256 4c0032ef6d73d622a927147a66f0756adfbd667446ede1e9b556d1ba6c5709f3, byte-identical to the pin in src/tests/fixtures/api508-admitted-routes.json). Measured at head with an isolated HOME and a local stub returning 402: node src/index.js research token top-tokens --limit 1 with no credential takes the anonymous auto-pay branch and only falls through to 'Sign the paymentRequirements below ... --x402-payment-signature' because the isolated home has no wallet; the identical command with NANSEN_API_KEY set is refused with 'Automatic wallet payment is available only without a selected credential.' Prerequisites: a funded default wallet (created by the first-class nansen trade flow) and no key/session. Material limit: openclaw/ClawHub enforcement semantics for requires.env live outside this repository and were not exercised.
S2 P3: src/tests/browser-onboarding-routes.test.js:17
The consistency guard that is supposed to keep key-only guidance on mixed-scope skills is inverted. conditional is derived as every skill having at least one admitted example, and the test then asserts those skills' frontmatter must NOT contain NANSEN_API_KEY. A skill that documents both admitted and excluded routes therefore lands in the key-free set and the test actively requires its key requirement to be dropped, instead of failing. nansen-defi-positions is only protected by a hardcoded filename exclusion on this line, not by the admitted/excluded data. Derive the key-free set as skills where every inventoried example is admitted, and fail any skill with a mixed set. Secondary: the excluded-family assertion matches /beta/, which never matches the real /api/v1beta1/ prefix used by src/api.js:1860-2008; it is harmless today only because the fixture is a byte-exact sha256 pin of the frozen contract, not because the regex screens anything.
Evidence: Proven in a throwaway rsync copy of the worktree (tracked files untouched): appending nansen research portfolio defi --wallet $ADDR to skills/nansen-wallet-deep-dive/SKILL.md plus the matching fixture row {expectedRoutes:['POST /api/v1/portfolio/defi-holdings'], admitted:false} left the skill in conditional with no NANSEN_API_KEY in its frontmatter, and npx vitest run src/__tests__/browser-onboarding-routes.test.js reported 141 passed / 1 file passed. Current per-skill fixture audit shows no mixed skill exists at this head (only nansen-defi-positions has an admitted:false row), so this is a latent guard defect, not a live mislabel.
S3 P3: scripts/postinstall.js:187
The new direct-invocation guard import.meta.url === pathToFileURL(process.argv[1]).href compares a realpath-resolved module URL against an unresolved argv path, so any install prefix reached through a symlinked path component silently skips main() entirely. Every piece of onboarding this slice adds then disappears: the shared-resolver credential classification, the 'Run nansen login for browser approval, or use NANSEN_API_KEY directly' tip, and the 'Browser login requires a supported OS credential store and enabled server admission' precondition warning. This matters most for the users the slice targets (pnpm, npm link, symlinked global prefixes), who get a silent install with no credential guidance at all. The new packaged assertion cannot detect it: src/tests/package.test.js asserts expect(onboarding).toBe('') on execFileSync stdout while every postinstall line is written to stderr via log(), and its tmpDir from os.tmpdir() is itself symlinked on macOS (/var/folders -> /private/var/folders), so the assertion passes whether or not main() ran. Compare realpath-resolved paths (or keep an explicit opt-out) and assert on captured stderr content. The --omit=optional import-resolution value of that same test is unaffected, because the ../src/auth-credentials.js import is evaluated before the guard.
Evidence: Measured at head with an isolated HOME and npm_config_global=true: node /Users/dipeng/Nansen/worktrees/api508-onboarding-security-d59c242/scripts/postinstall.js prints the four non-TTY tip lines; ln -s <same dir> $L && node $L/scripts/postinstall.js prints nothing; invoking the relative path from inside the symlinked cwd prints the tips again (argv[1] relative, resolved against cwd). src/auth-credentials.js imports only node builtins, so the postinstall import itself resolves cleanly without the pinned optional native dependencies.
S4 P3: docs/releases/api508-evidence.json:96
The release manifest and the packaged guide both understate what evidence now exists at the frozen base. docs/releases/api508-evidence.json records native renewal only at the older 1c16839 (line 37) and leaves acceptance.exactHeadNativeRenewal null; docs/browser-login.md:5 states that native renewal evidence 'retains its original API5071c16839 attribution, not the reconciled base'. Root's independently supplied verification at the frozen base now records, at 8f9085b: 51 renewal tests, macOS arm64 and isolated Debian12 x64 GNOME Secret Service native 10,295-byte one-rotation runs with fresh-process read after daemon restart and logout retirement/deletion, an 80 ms locked-collection refusal, plus eleven public CLI/API cases at CLI 8f9085b / API 9190 covering one rotation, no refresh of an inactive environment-key account, and a zero-credit account after rotation. The drift is in the safe direction, but the manifest is the artifact the runbook directs an operator to read for the exact-head native gate, so it currently misreports the state of a named gate. Re-attribute those two records to the reconciled base and keep every deployment, controlled-account, ledger, cohort, propagation, Windows/daemon-cancellation and published-recovery-version field null: the new root runs are still synthetic issuer/storage/gateway with local-only transport and establish no live ceremony, no accepted platform and no cohort.
Evidence: docs/releases/api508-evidence.json lines 37-48 and 96; docs/browser-login.md line 5 and the trailing 1c16839 paragraph; docs/releases/api508-runbook.md readiness row 'Platform custody/renewal: older83d6 ... older1c16839'. Superseding source: RUN/API-507-root-8f9085b-native-verification.md and RUN/API-507-root-joint-refresh-verification.md (supplied to this review by root, not read from a peer report or a running review). PR #659 body independently acknowledges the older-head attribution as a stated limit, which is why this is reported as a required record update rather than a false claim.
Checks
Confirmed exact head: git rev-parse HEAD = d59c242; gh pr view 659 --json headRefOid = d59c242…, base branch hulk/api-507-session-refresh; full delta reviewed against 8f9085b (39 files, +962/-142).
Read CLAUDE.md/AGENTS.md, RUN/API-503.md, API-508.md, API-508-implementation-plan.md context, API-508-plan-review.md, API-508-implementation-handoff.md, API-508-review-focus.md, the PR body, and root's two supplied 8f9085b verification documents. No peer review reports, API-507 security output or running API-507A material was opened.
npm test: 3507 passed / 9 skipped across 89 files (125s), matching the writer's claim exactly. npm run lint: clean. pretest changeset validation passed. package-lock.json touched by npm install was restored; worktree left clean with no tracked edits, commits, pushes or GitHub writes.
Admission contract verified byte-exactly: src/tests/fixtures/api508-admitted-routes.json pins sha256 4c0032ef…09f3 and sourcePath docs/contracts/api505-browser-session-research.json; shasum -a 256 of the frozen API505 file matches, and set-difference of the 58 method/path pairs in both directions is empty. All 58 are /api/v1 data reads — no execution, wallet, agent, portfolio, web, v1beta1 or internal route is present.
Per-skill admitted/excluded audit of the 133-row command fixture: no skill in the 23-skill key-free set contains an excluded example at this head; nansen-defi-positions is the only skill with an excluded row and correctly retains NANSEN_API_KEY, as do agent-guide, web-searcher, web-fetcher, smart-alerts, alerts-webhook-listener, trading, limit-orders, wallet-manager and wallet-keychain-migration. Grepped all 23 key-free skills for non-nansen research invocations: only nansen login appears, so no top-level perp/trade/agent/web/alerts/mcp helper is advertised as browser-reachable.
Postinstall credential classification traced through the real entry point: src/auth-credentials.js imports only node builtins (no optional native binding), resolveCredential/assertUsableSelection read config metadata only, and postinstall prints kind-derived strings only — no credential object, key, account id or token. Verified the global/local guard, the non-TTY tips-only path, and that the account child is spawned only on an explicit y/yes (default [y/N] no).
Login dispatch re-traced at src/cli.js:1206-1212: plain nansen login and --no-browser always route to browserLoginFn regardless of NANSEN_API_KEY; only explicit --human/--api-key reach the legacy key path, so the reworded RATE_LIMITED resolution ('retry explicit key setup: nansen login --human') is on the correct branch. No --device flag exists.
Ran real public commands with an injected local-only fetch and isolated HOME: --help (new AUTHENTICATION block rendered), auth status (offline JSON, effective vs saved distinguished, key masked, no outbound auth call), and the 402 comparison for anonymous vs selected-key credential selection. No production, paid, live-browser, issuer, retirement or analytics traffic was emitted; DO_NOT_TRACK=1 and NANSEN_NO_TELEMETRY=1 set throughout; no existing user credentials were read or modified (test-isolation.js also redirects HOME to a temp dir for the suite).
Packaging boundary checked: package.json files ships docs/browser-login.md and excludes docs/releases/**; the runbook's relative recovery link resolves to the existing '## Offline recovery without native locking' heading; the postinstall anonymous branch's 'See docs/browser-login.md in this package' reference is satisfied by a packaged file; no packaged file references docs/releases.
Rollback/recovery guidance reviewed: journal stop-gate added before manual editing, v2 tombstone preserved with no forced downgrade, no invented published recovery version (rollback.v2CompatiblePublishedRecoveryVersion and every acceptance field remain null), admission-restriction and revocation-feed/retention steps kept separate from distribution rollback, and 'a main push is not a harmless way to revert documentation' retained. Changesets policy preserved (existing major retained, new patch added); no workflow, CI target or trusted-publishing change in the delta.
Limitations
No live gateway, deployed issuer/API/consumer build, controlled account, real browser ceremony, before/after ledger, revocation propagation measurement, headroom measurement, Windows support, desktop prompt cancellation or OS login/PAM/reboot evidence was produced or is claimed by this review; all probing used local-only transports against synthetic responses.
Native credential-store behavior was not re-exercised here. The reconciled-base native and joint results come from root's supplied documents, which are themselves synthetic-issuer/storage/gateway runs; native compatibility remains a minority-platform question and the latency deferral persists.
openclaw/ClawHub enforcement semantics for requires.env and primaryEnv are defined outside this repository and were not executed, so S1's blast radius depends on how the consuming harness treats a skill with no declared credential requirement; the CLI-side anonymous x402 auto-pay branch itself was verified directly.
The authservice device-flow contract at 3e08f9e76a3fd785df9f641f640234e5a87e00a4 was not accessible from this worktree, so the README/docs claims about supported sign-in methods ('email/password and Google supported; employee-gated audiences require Google; Apple deferred') were not verified against the pinned issuer document.
Dependency review of API-507 is still pending and was deliberately not repeated; only this slice's own claims, boundaries and integration against the frozen base were verified. Base-branch CI is main-only and no stacked run exists; absent CI is recorded as a gap, not a pass, and no bypass was attempted.
The --help path still performs a background openapi.json fetch (observed as an intercepted request). That is pre-existing behavior outside this delta and was not assessed.
Orchestrator qualifications
Root qualifications for API508 general A
Full KimiK3 standalone report read. It independently completed the original A round on unchanged d59c242; failed earlier model-substituted runs are not accepted. All3findings,12checks and8limitations are retained.
A1 is a valid newly required release-gate addition. Do not accept its broader implication that server code only supports natural access expiry: authservice357 and API1966 implement complete-family revocation, with synthetic original/rotatedA200->401 and unrelatedB200->200 proof. Their current GitHub heads remain unchanged and unmerged. Live propagation remains unproven. The existing CLI correctly recognizes access_revocation:{status:recorded,coverage:complete_family,propagation:pending,version:1} as recorded_pending. Its rejection of an invented access_tokens_revoked:true boolean is correct and must remain. Source src/auth-device.js:162-167 and docs/browser-login.md:108 explain this distinction. Requiring all-token invalidation before rollout does not justify claiming immediate server propagation from an acknowledgement. Add explicit manifest/runbook gates and accurately distinguish local removal, recorded/pending and observed API rejection. Natural expiry alone must not count as logout invalidation acceptance.
A2 overlapsS4 and the root snapshot update. Add new8f artifact hashes with original provenance; after refresh repairs reconcile dependency then rerun or establish exact bounded source equivalence. Never rename old1c evidence as8f. Actual native store proofs use synthetic issuer/retirement; joint transport uses controlled storage/gateway. Leave live/cohort/package acceptance null until proven.
A3 overlapsS3. Keep direct-import silence and failure-tolerant installation while canonicalizing entry paths. RootR1 and securityS1/S2 remain independent findings despite not appearing in A; do not discard minority security findings because A did not duplicate them. See API-508-security-root-qualification-notes.md for source-backed configured-key injection and route-guard repairs.
Root qualifications for API508 security at d59c242
Full standalone report read; preserve all four findings and six limitations. General A has now completed independently without this report; consolidated repair can proceed.
S1 identifies a real guidance/eligibility concern: removing the environment requirement permits skill loading without a selected credential, and existing anonymous x402 can spend wallet funds. The finding does not establish a new CLI payment fallback or a deployed funded transaction. A null rollout field and default-false contract flags do not prove browser admission is disabled in every actual environment. The branch is an unpublished NOT_READY draft with distribution gated. Preserve those qualifications, not a universal live-state claim.
Root consulted official OpenClaw docs https://docs.openclaw.ai/tools/skills and https://docs.openclaw.ai/tools/skills-config on2026-09-19. requires.env controls skill eligibility; primaryEnv associates per-skill apiKey configuration with an environment name. They have different roles. Removing primaryEnv may independently break configured-key injection; writer must verify this and preserve conventional-key configuration compatibility. A saved browser session cannot be represented as a made-up OpenClaw environment requirement. Before any research, instructions should require an explicitly selected usable credential, explain offline status is unverified, use an explicitly requested free account check where necessary, and stop on anonymous/invalid/expired/uncertain selection. State anonymous wallet payment is separate and requires explicit user intent; do not silently convert auth failures to anonymous. Preserve browser onboarding and conventional keys without inventing metadata support or changing the CLI anonymous-payment policy. Assess whether keeping the rollout-time environment gate is needed until an accepted cohort exists, while documenting the distinction from the eventual browser-enabled skill behavior.
S2 is a concrete latent guard weakness. Root read the test: any admitted example currently qualifies the entire skill, with hand-coded exclusions, and the /beta/ expression misses /v1beta1/. Repair derivation to use complete per-skill scope and a negative mixed-scope mutation. Do not conflate optional primaryEnv with required env when checking frontmatter.
S3 is a concrete symlink entrypoint/package-test weakness. Compare canonical entry paths without running main on module import. Assert actual stderr tips for global non-TTY direct and symlinked invocations, and silence for local install/import. Keep the optional-dependency import check and no-install-failure property.
S4 is acknowledged stale snapshot metadata. Add new separately hashed8f9085b artifacts; never re-attribute the old1c16839 files. The native root runs use actual platform storage with synthetic issuer; the joint run uses controlled storage/gateway. Those are distinct proofs. acceptance.exactHeadNativeRenewal must distinguish frozen dependency proof from final candidate/package proof. After API507 repairs change the head, rerun or establish bounded exact-source equivalence and retain every live/cohort/recovery gate. docs and PR metadata should match.
Pinned issuer contracts are available outside this reviewer checkout at /Users/dipeng/Nansen/worktrees/api504-session-final-3e08f9e. Root read docs/device-authorization-flow.md lines100-133 at that exact source. It explicitly documents email/password and Google, Apple deferred under SUPER-16109, and Google-only verified employee sign-in for employee-gated audiences. This closes the reviewer’s source-read gap for the wording, not live sign-in acceptance.
Root R1: preserve configured-key injection
Official source retrieved through gh from openclaw/openclaw src/skills/runtime/env-overrides.ts, blob80a9e42211b4896ec1de57d5614502ec16ada358, SHA256812b0eb4332c390e2117916a4d3e3ec28c0bd9ecd5a2a09df26e029a5022cd7b, retained as openclaw-env-overrides-20260919.ts. applySkillConfigEnvOverrides only maps skillConfig.apiKey when normalizedPrimaryEnv exists; both direct and snapshot paths forward the metadata primaryEnv. Removing primaryEnv from the23 skills therefore prevents this supported configured-key mapping in current official source. Existing explicit environment variables are a separate unaffected path. Restore primaryEnv independently of whether requires.env stays mandatory, and update the route test to parse the relevant field instead of forbidding every NANSEN_API_KEY mention in frontmatter. This is source-backed compatibility evidence, not an executed installed OpenClaw integration test. No local OpenClaw executable is available. Source at verified commit: https://github.com/openclaw/openclaw/blob/10b13070c4afb9f4c5ecefab01212ef5511246d3/src/skills/runtime/env-overrides.ts#L151 .
Root independently passed158focused tests and verified all8original manifest hashes. New DPoP-signature verification passed11CLI/API cases at8f9085b, including original-key signature verification and tampered-signature rejection, still synthetic and not candidate/staging acceptance. No release/deployment/approval is implied.
|
Consolidated first-round onboarding repairs pushed at
The reviewed head was AI execution metadata
|
|
Exact dependency reconciliation completed at Both conflicts retain API508 onboarding, primaryEnv/credential instructions and every-scope route guards, symlink/stderr package assertions, plus API507 recovery text and pre-import/minimal-child fail-closed guards. Package dependency installation has the existing narrow npm registry exception and bounded timeout. Auth-state/device/login/telemetry and custody/API components are byte-identical to the exact dependency; no new runtime changes beyond its merge. Full tests: 3537 passed,9 skipped across90 files,122.58s, exit0.294 focused integration/package/guard/renewal checks and38 separate telemetry cases pass; lint, changeset/status and diff pass. Initial full run's10mocked telemetry failures came from my runner exporting DO_NOT_TRACK; corrected runner-only opt-out, preserved guards, then reran successfully. Logs retain that failed attempt. No live/paid/browser/analytics/keyring probes or user credentials; registry-only package dependency access. All24 artifact hashes verify;16historical records unchanged. New a25 proof is root133 focused each Node20/24, actual nativeMac/Linux synthetic renewal/restart/logout, and11joint CLI/API fixture cases with actual original-key ES256 signature verification/tamper rejection. These are dependency evidence, not candidate/package/live/cohort acceptance. All23acceptance gates plus candidate/rollback records remain unchanged/null where missing. A1 full-token logout gate, A2/S4 source freshness, A3/S3 symlink/stderr, S1/rootR1 optional primaryEnv and explicit credential prerequisite, and S2 complete-scope/v1beta1 exclusions are preserved. Instructions do not add machine payment enforcement. API507's B clean code verdict is not approval; proxy availability, native legacy-write compatibility, latency/cache growth, v2-compatible recovery and external-daemon limits remain. Prior security unauthorized synthetic production POST/401 is disclosed without a no-server-write assertion; its scripts were not rerun, and old stronger-guard failures are not relabeled. Next is assigned API508 B/security and root final verification. Full-session original/rotated/copied invalidation through deployed services, ledger/accounts, candidate native/platform/cohort, gateway/headroom and published compatible recovery remain gates. No approval, merge, retarget, workflow change, release, deploy, publication or ticket closure. Writer is returning rather than waiting or self-reviewing. AI execution metadataOpenAI
|
Codier
left a comment
Codier
left a comment
There was a problem hiding this comment.
Review B and bounded security repair verification at 956764b207fcc91498cea45c8ba91d70e31e538a, against a25bc511f53379ad619eb18b994311b78c2e86bb. Changes requested for security S5-S7. General B reports zero code findings and material acceptance limits. Every finding, check and limitation below is retained. No approval or rollout acceptance.
Reviewer B
Factory DeepSeek V4 Pro, max reasoning/high autonomy; session 40aded61-c114-4c1c-a199-cf20cd069e79. Same-round bounded retry after failed original session and failed same-session resume. No permission bypass; optional scratch deletion was not retried. Failed turns do not count as completed reviews.
Intent: Independent final general B review of PR659 API508 onboarding at exact head 956764b against exact base a25bc51, retry of the assigned B round after the original provider session failed. Scope: the complete 39-file onboarding delta, every changed production path and live caller, the intended user journey, dead or redundant code, and preservation of all prior A/security findings and root qualifications. Read-only operations and existing tests only, under the checkout fail-closed network guard with proxies stripped.
Verdict: changes_requested.
Findings
No findings.
Checks
Exact refs verified in the review worktree: git rev-parse HEAD equals 956764b, merge parents are 4f374e3 and a25bc51, the real base a25bc51 is an ancestor, and the working tree is clean with no stray tarballs.
Delta inventory confirmed at exactly 39 files versus base: 2 changesets, README.md, docs/browser-login.md, docs/releases/api508-evidence.json, docs/releases/api508-runbook.md, scripts/postinstall.js, 25 SKILL.md files, src/cli.js, src/schema.json, and 5 test or fixture files. Production changes are confined to scripts/postinstall.js, src/cli.js and src/schema.json, 1294 insertions and 123 deletions.
Live PR659 state read directly from GitHub: draft true, baseRefName hulk/api-507-session-refresh at a25bc51, headRefOid 956764b, reviewDecision CHANGES_REQUESTED from Codier at d59c242, zero inline review comments, state OPEN. Matches the reconciled readback artifacts.
Full guarded npm test run at exact head by this review on Node22.23.2 with src/tests/fixtures/network-guard.cjs preloaded via NODE_OPTIONS before imports, ambient proxies and NODE_USE_ENV_PROXY stripped, DO_NOT_TRACK left unset per the correct telemetry-test contract: 3537 passed, 9 skipped across 90 files, exit 0. This independently reproduces the writer reconciliation claim at this exact head.
Focused onboarding suites run under the same guard: browser-onboarding-routes.test.js, postinstall.test.js, network-guard.test.js and package.test.js, 163 passed, 8.08s, exit 0, matching the independent root source-verification result for this head.
npm run lint clean, git diff --check base..head clean, changeset status valid with the major bump preserved and the patch changeset present.
Scanned the full production delta for dead or redundant code: the old postinstall isLoggedIn ad-hoc apiKey sniff and its readFileSync import are gone with no remaining isLoggedIn references anywhere in src; every import and constant in scripts/postinstall.js is used; the src/cli.js change is confined to help text plus one RATE_LIMITED recovery string on the legacy key-setup branch.
Live-caller tracing of scripts/postinstall.js: wired by package.json postinstall as node scripts/postinstall.js, global-only via npm_lifecycle_event and npm_config_global, failure-tolerant main().catch with exit 0, entry guard now compares realpathSync(process.argv[1]) against realpathSync(fileURLToPath(import.meta.url)) so direct and symlinked invocations behave identically, and the guard plus try/catch keep import and missing-entry invocations silent.
Classifier tracing of scripts/postinstall.js: resolveCredential and assertUsableSelection in src/auth-credentials.js import only node builtins and read config metadata only, no storage or network is opened during classification, explicit y/yes is required before spawning node src/index.js account --pretty with no secrets in argv, and default consent is [y/N].
src/cli.js login dispatch re-read at source: plain login and --no-browser always route to browserLoginFn regardless of NANSEN_API_KEY, only explicit --human or --api-key reach legacy key setup, --no-browser plus --human is refused, and the changed RATE_LIMITED resolution sits on the legacy branch only.
All four dependency production modules src/auth-state.js, src/auth-device.js, src/auth-login.js and src/telemetry.js compared by git blob id against a25bc51: byte-identical, confirming the reconciliation claim that no onboarding runtime change was smuggled in beyond the documented merge.
Skill audit: 23 conditional direct-data skills carry exactly the required Authentication prose phrases (selected usable credential, offline status first, explicit free account check on request, stop on anonymous/invalid/expired/uncertain/failed auth, anonymous x402 as separate explicit-intent paid workflow, rollout pending), keep optional primaryEnv NANSEN_API_KEY with no requiredEnv, while the 10 key-only skills agent-guide, defi-positions, web-searcher, web-fetcher, smart-alerts, alerts-webhook-listener, trading, limit-orders, wallet-manager and wallet-keychain-migration all retain requires.env NANSEN_API_KEY.
Route guard strength verified in passing tests: supportsBrowserSkill requires every inventoried example admitted and every referenced nansen command family within research/login/auth/account, the negative mixed-scope mutation throws, requiredEnv and primaryEnv are parsed independently, the excluded-route regex now covers v1beta1, and 133 literal examples including embedded scripts and chain loops are traced against the 58-route admitted fixture that pins the API505 contract head 9190ae3d with sha256 4c0032ef6d73d622a927147a66f0756adfbd667446ede1e9b556d1ba6c5709f3.
Packaging verification by the passed package test: tarball installs with optional native bindings omitted, help works, direct and symlinked global-invocation stderr carries the onboarding tips while local install and import stay silent, documented recovery heading and skills ship in the package, docs/releases is excluded, and the earlier root unpublished-tarball native proof at this exact head qualifies the writer snapshot missing-package statement.
Prior disposition preservation spot-verified in source: A1 is encoded as the runbook full-session invalidation gate plus evidence fullSessionAccessTokenInvalidation and measuredRevocationPropagationAndDurability acceptance fields and the docs/browser-login.md rollout paragraph; A2 and S4 are addressed by new a25bc51 evidence records with fresh hashes while 1c16839 and 8f9085b records retain original attribution; A3 and S3 are fixed by the realpath guard and stderr-content assertions; S1 and rootR1 are implemented as qualified with optional primaryEnv restored and machine-enforcement distinction documented in the runbook; S2 is fixed by complete-scope derivation with negative mutations.
Limitations
The code delta is clean but all 23 acceptance gates in docs/releases/api508-evidence.json remain null: no published candidate version or integrity, no accepted cohort, no controlled accounts, canonical UID mapping or before/after ledger, no deployed issuer/API/consumer/gateway digests or flag snapshots, no public local or remote browser walkthrough, no denied-load or latency acceptance, and no v2-compatible published recovery binary. Material missing verification blocks approval even with zero code findings.
The user-required full-session access-token invalidation before rollout is unproven: real logout must reject original, rotated and copied access tokens through the deployed issuer, gateway and API with measured propagation, unaffected-session controls, durable records and restart or outage fail-closed behavior. Local removal, the recorded/pending complete_family receipt and natural expiry are explicitly not substitutes, and no acceptance evidence exists yet.
Verification provenance: this review ran the full and focused suites only on Node22.23.2 at the exact merged head. The Node20 and Node24 focused results, native macOS arm64 and isolated Debian12/GNOME renewal or restart or logout runs, and the eleven joint CLI/API DPoP cases are attributed to the dependency head a25bc51 by the root documents, and I did not rerun them on the merged candidate. Native keyring behavior was not exercised by this review per containment.
The unpublished tarball native Mac and isolated Debian proof from the root at this exact head qualifies the writer snapshot package statement only. It does not establish published, cohort or live acceptance, and it used synthetic issuer, retirement and recreation with owned temporary storage, once per platform.
Skill credential guidance is model instruction, not machine enforcement. An agent that ignores skill instructions has no execution-level credential gate and can still reach the anonymous x402 automatic wallet payment branch per call, including inside chain loops. This is the documented, root-qualified residual of original S1 with the draft distribution-gated in an unpublished NOT_READY branch, and I could not execute the openclaw harness semantics locally because no OpenClaw runtime exists on this machine.
The earlier security review's unauthorized synthetic production refresh POST and its 401 response remain disclosed and were not rerun. No server-side no-write claim is inferred, and historical blocked-attempt failures are not relabeled by later green runs.
Minority and platform findings from the dependency rounds remain open and are preserved: mandatory-native legacy-write compatibility disagreement, proxy CONNECT and multiple-request refusals without safe retry, unaccepted native read latency, cache isolation stranding unbounded old files, external credential daemon outstanding writes preventing physical erasure proof, and v2 persistence after first attempted rotation with no tested pre-v2 downgrade or published compatible recovery.
The npm package installation inside the package test used the explicit narrow registry.npmjs.org exception under NANSEN_TEST_PACKAGE_INSTALL with a 45-second bounded install; the installed CLI fetch was locally captured. Guards are process-level and are not OS-level egress isolation.
Release metadata fields for the candidate reviews and review gates are intentionally null in the evidence manifest and are not passed gates. The live PR remains a draft with only the original Codier CHANGES_REQUESTED review at d59c242; assigned final-head B and bounded security verification are the disposition that was outstanding when this review ran, and the prior B provider failure is treated as a retry of the same round, not a completed review.
Dedicated security repair verification
Claude Code Opus 5, requested high reasoning; session 87db0ef8-6739-4d54-ad02-af1fd5c2f1cb. Observed model claude-opus-5; SDK does not independently expose effort. Scope is prior repairs and merge consequences, not a third general review.
Intent: Bounded security repair verification of API-508 onboarding at the exact merge head 956764b (parents 4f374e3 API-508 repair and a25bc51 exact API-507 dependency), covering this reviewer's prior S1-S4 and root R1, the security-relevant general findings A1-A3, and the direct consequences of the dependency merge. Not a third general review and not a review of the unchanged API-507 authentication owner.
Verdict: changes_requested.
Findings
S5 P3: skills/nansen-token-research/SKILL.md:18
The new credential prerequisite (identical in all 23 conditional skills, and restated in docs/releases/api508-runbook.md:54) tells the agent to run nansen auth status first and then 'Stop research on anonymous, invalid, expired, uncertain or failed authentication', continuing only after an explicitly user-requested free nansen account check succeeds. The only field named 'expired' in that command's output is saved_session.expired, and src/doctor.js:184 derives it as auth.active.expiresAt <= Date.now() where expiresAt is the ACCESS-token expiry (src/auth-device.js:168 and :235 set it to min(now + expires_in*1000, refresh exp)), not the session or refresh-family lifetime. An ordinary, fully usable saved session therefore reports expired: true as soon as its short-lived access token ages out, even though src/auth-state.js:430 renews it automatically and silently whenever less than 60 s remain, and src/auth-credentials.js:58-62 assertUsableSelection deliberately does not treat that state as unusable. The prerequisite as written converts the routine automatic-renewal path into a hard stop plus a fresh per-occurrence consent request, which is exactly the repeated-consent burden this slice set out to avoid, and it is the state most saved-session users will be in at the start of any research turn. This is friction in the conservative direction (it does not open an anonymous or paid path), but it materially degrades the authorized-research/automatic-renewal journey the PR claims to preserve. Distinguish the renewable access-token expiry (which the CLI handles without user interaction, as src/doctor.js:385 already words for limit-order sessions: 're-authenticates automatically') from an unusable session — rejected, blocked or cleanup-required renewal, or AUTH_STATE_INVALID — and reserve the stop-and-ask-for-nansen account rule for the latter. The offline postinstall consent gate is a separate, correct behaviour and should not be conflated with it.
Evidence: Reproduced at head with an isolated HOME, env -i, no proxies and src/tests/fixtures/network-guard.cjs preloaded: a v2 pointer with expiresAt: 1758200000000 yields "saved_session":{...,"expired":true,"validity":"cached_unverified","renewal_state":"not_recorded"} from node src/index.js auth status, with logged_in: true and effective_credential.kind: session; no outbound request was made. Wording verified identical across all 23 conditional skills (byte-identical three-paragraph Authentication block, checked by extracting and de-duplicating the section from every SKILL.md; 23/23 for each paragraph) and pinned by src/tests/browser-onboarding-routes.test.js:88, which asserts the exact sentence, so the test currently locks the defect in. Renewal semantics read statically at src/auth-state.js:411-445 (acquireSession renews when bundle.expiresAt - now() <= 60000); no native store or live issuer was exercised.
S6 P3: src/tests/package.test.js:53
The S3 repair rewrote the packaged smoke check and, in doing so, dropped the only assertion that exercised the installed executable. At the API-507 dependency a25bc51 the test ran node_modules/.bin/nansen --help (resolving nansen.cmd on Windows) precisely because the stated purpose of this file is catching packaging breakage of the kind that shipped in 1.18.0; at this head it runs execFileSync(process.execPath, [packageRoot + '/src/index.js', '--help']) instead. Invoking the entry module through an explicit interpreter bypasses the npm bin link, the shebang and the Windows shim, so a broken or removed bin mapping, a corrupted shebang line or a non-executable mode on the published entry would now install and pass this suite while nansen fails for every globally installed user — the exact population this onboarding slice targets, and the population that then never sees any of the postinstall guidance. This is a coverage regression introduced by the API-508 delta (present since d59c242, unchanged by the merge), not by the dependency. Restore a node_modules/.bin/nansen --help invocation alongside the direct-module run; the direct run is still needed for the interpreter-controlled --import capture, so keep both.
Evidence: git show a25bc511:src/__tests__/package.test.js lines 47-49 contain const binary = join(tmpDir, 'node_modules', '.bin', process.platform === 'win32' ? 'nansen.cmd' : 'nansen') and execSync("<binary> --help"); git show 8f9085b:src/__tests__/package.test.js line 48 is identical. The same lines are absent at d59c242, at 4f374e3 and at 956764b, where line 53 is the direct src/index.js invocation. No .bin, shim or shebang assertion exists anywhere in the file at head (grep for .bin, binary, shebang returns nothing). npm pack --dry-run --json at head lists 94 files with bin: {nansen: ./src/index.js} in package.json, so the mapping is currently correct — this is an undetected-regression risk, not a live break.
S7 P3: src/tests/browser-onboarding-routes.test.js:44
The S2 repair fixed the inverted derivation and the /beta/ regex, but the key-only side of the guard is still a hand-maintained literal list. assertSkillScope only inspects skills whose text contains '## Authentication', and the only assertion that a skill declares requires.env: [NANSEN_API_KEY] is the loop over the ten hardcoded names on line 44. Nothing ties the set of shipped skill directories to the union of {conditional, keyOnly, nansen-mpp-payment}, so a newly added skill that omits ## Authentication, omits requires.env and omits primaryEnv is invisible to every assertion in the file: it is not in conditional (its commands are not admitted research, so toHaveLength(23) still holds), it is skipped by assertSkillScope, and it is not in keyOnly. Because package.json files ships skills/** and .github/workflows/sync-clawhub.yml publishes this metadata, such a skill would be distributed advertising wallet/trade/agent commands with no declared credential requirement at all — the same class of gap S2 reported, narrowed but not closed. Assert that every directory under skills/ is accounted for by exactly one of the three sets, and derive key-only membership from the inventory (any skill with a non-admitted route or a non-research command family) rather than from a literal list.
Evidence: Demonstrated in a throwaway tar copy at /tmp (tracked files untouched, node_modules symlinked): adding skills/nansen-synthetic-probe/SKILL.md with no requires.env, no primaryEnv, no '## Authentication' and a body documenting nansen agent "interpret this", nansen trade quote ... and nansen wallet create left npx vitest run src/__tests__/browser-onboarding-routes.test.js at 141 passed / 1 file passed. The complementary control passes: removing env: [NANSEN_API_KEY] from the existing skills/nansen-trading/SKILL.md in the same copy fails the suite (1 failed | 140 passed) via the line-44 list, confirming existing key-only skills are protected and only unlisted new skills escape. Copy deleted afterwards; the reviewed worktree shows git status --porcelain empty at 956764b.
Checks
Exact head confirmed: git rev-parse HEAD = 956764b with parents 4f374e3 and a25bc51; gh pr view 659 reports headRefOid 956764b…, base hulk/api-507-session-refresh, draft true, state OPEN. Delta versus the real base a25bc51 is 39 files, +1294/-123. Live PR body and both writer comments read in full; the only review present is Codier CHANGES_REQUESTED at d59c242 and the inline-comment collection is empty (0), so no current peer B/security output existed to read or was opened.
Read in full: RUN/API-508-final-review-assignment-common.md, API-508-security-root-qualification-notes.md (including root R1), API-508-root-956764b-package-verification.md, API-508-reconciled-handoff.md, api508-reconciled-pr-body.md, API-508-reconciled-response.md, API-508-repair-handoff.md, this reviewer's own prior report security/api508-d59c242/result.txt and the original general A report api508-onboarding/workers/reviewerA-kimi-authorized/result.txt, plus CLAUDE.md/AGENTS.md. The mutable writer checkout and the current peer API-508 B/security reports were not inspected.
S3/A3 repair CONFIRMED fixed by direct execution at head, all four paths, each with src/tests/fixtures/network-guard.cjs preloaded, env -i and every ambient proxy variable stripped: packed-equivalent direct invocation, invocation through a symlinked directory component, invocation through a symlink to the file itself, and a relative invocation from inside the symlinked cwd all print the four non-TTY tips including 'nansen login' and 'NANSEN_API_KEY'; npm_lifecycle_event=postinstall without npm_config_global=true prints nothing; await import(...) of the module prints nothing and exits 0. scripts/postinstall.js:187-193 now canonicalizes both sides with realpathSync inside a try/catch, and src/tests/package.test.js:56-75 asserts stderr content (not stdout emptiness) for the direct and symlinked packaged entries and asserts total silence for the local-install and both import cases.
S1 and root R1 CONFIRMED addressed. All 23 conditional skills carry primaryEnv: NANSEN_API_KEY with no requires.env; all 10 mixed/excluded skills plus nansen-defi-positions retain requires.env: [NANSEN_API_KEY] and primaryEnv. Root's retained OpenClaw source openclaw-env-overrides-20260919.ts verified at SHA256 812b0eb4332c390e2117916a4d3e3ec28c0bd9ecd5a2a09df26e029a5022cd7b: applySkillConfigEnvOverrides builds allowedSensitiveKeys from primaryEnv and requiredEnv independently and gates canInjectPrimaryEnv only on a normalized primaryEnv plus the absence of an externally managed env value, so configured-key injection survives the removal of requires.env and an explicit NANSEN_API_KEY still wins. No installed OpenClaw runtime was executed.
S1's runtime boundary re-verified statically at head: src/api.js:783 still computes mayAutoPay = this.allowPayment && this.selection.kind === 'anonymous' && no apikey/authorization header, i.e. the anonymous auto-pay branch is unchanged and the repair is instruction-only. The skill text now names the consequence explicitly ('the CLI can automatically spend funds from its configured wallet on a supported 402 challenge, once per call, including calls in loops... never a fallback for failed authentication') in all 23 skills, and README and docs/releases/api508-runbook.md:54 state the same boundary without claiming machine enforcement.
S2 repair verified as far as it goes: supportsBrowserSkill now requires every inventoried row for the skill to have non-empty expectedRoutes all present in the pinned admitted set AND every nansen <family> mention across all .md files in the skill directory to be one of research/login/auth/account; credentialMetadata parses primaryEnv and requires.env separately; the excluded-family regex now matches the real /api/v1beta1/ and /api/v1/search/web-(search|fetch) prefixes. Independent audit at head: the 133-row fixture contains exactly one admitted:false row and it belongs to the key-only nansen-defi-positions; the 23 conditional skills collectively mention only the research, login, auth and account families.
Admission contract re-verified byte-exactly and independently: src/tests/fixtures/api508-admitted-routes.json pins sourceSha256 4c0032ef6d73d622a927147a66f0756adfbd667446ede1e9b556d1ba6c5709f3 for docs/contracts/api505-browser-session-research.json at API505 head 9190ae3d…; shasum -a 256 of that file in the frozen api505-research-root-9190 checkout matches, the 58 method/path pairs are set-equal in both directions with no duplicates, all 58 are /api/v1 (57 POST, 1 GET) with no agent/portfolio/web/v1beta1/internal/execution/wallet route, and the contract's own flags remain {BROWSER_SESSION_ACCOUNT_ENABLED:false, BROWSER_SESSION_RESEARCH_ENABLED:false} with requires_both_flags true and status draft-deployment-disabled-staging-unproven.
A1 CONFIRMED addressed. docs/releases/api508-evidence.json now carries 23 acceptance fields, all null, including the two new fullSessionAccessTokenInvalidation and measuredRevocationPropagationAndDurability gates, with acceptanceNotes stating that refresh-only retirement or natural expiry does not satisfy them and that a recorded/pending receipt is not observed rejection. The runbook adds a 'Full-session invalidation' readiness row and operator step 5 requiring observed rejection of original, rotated and copied access tokens through the deployed issuer/gateway/API with unrelated sessions, replacement accounts and independent API keys unaffected, measured propagation and durable restart/outage enforcement, and no invented access_tokens_revoked:true receipt. The rollback sequence step 2 no longer permits 'revoked or expire' as a satisfying condition.
A2/S4 CONFIRMED addressed with provenance intact. All 24 evidence records in docs/releases/api508-evidence.json were independently re-hashed against the artifacts in the run directory: 24/24 match, 0 missing, 0 mismatched. The historical 83d6 and 1c16839 records keep their original heads, hashes and limits verbatim; the new a25bc51 records (Node20/24 focused logs, Mac and Debian native renewal, joint CLI/API DPoP) are separately attributed under frozenDependencyVerification; acceptance.exactHeadNativeRenewal remains null with a note stating it means final candidate/package proof, distinct from the completed frozen-dependency proof. docs/browser-login.md:135, :143 and :167 keep the older-head attributions explicit and label the a25bc51 runs as new records, 'not relabeled 8f or ca67531 results'.
Merge consequences reviewed file-by-file. Versus 4f374e3 the merge changes only docs (browser-login, evidence manifest, runbook), the API-507 test/fixture set and four production modules (src/auth-state.js, src/auth-device.js, src/auth-login.js, src/telemetry.js); those four are byte-identical to a25bc51 (git diff a25bc511 956764b -- returns nothing for them). The conflicted src/tests/package.test.js retains both sides: API-508's capture preload, symlink/stderr/local/import/failure-tolerance assertions and packaged-docs checks, plus API-507's childEnv inheritance, the narrow NANSEN_TEST_PACKAGE_INSTALL=1 registry exception and the bounded 45 s install timeout. No .github/, package.json or package-lock.json change exists versus the base, so no release, workflow or trusted-publishing surface moved.
Full suite re-run independently at head with the checkout's network-guard preloaded and http_proxy/https_proxy/all_proxy/HTTP_PROXY/HTTPS_PROXY/ALL_PROXY/NODE_USE_ENV_PROXY stripped: 3537 passed, 9 skipped across 90 files, 125.09 s, exit 0 — matching the writer's claim exactly, including the telemetry cases that require DO_NOT_TRACK not to be set. Focused re-run of src/tests/browser-onboarding-routes.test.js and src/tests/postinstall.test.js: 159 passed. npm run lint: clean. node_modules was copied from the root worktree only after confirming package.json (a118a29c…) and package-lock.json (615cd3be…) are byte-identical there; no npm install was run in this checkout and package-lock.json is untouched.
Packaging boundary re-verified: npm pack --dry-run --json at head lists 94 files — zero __tests__ paths, zero fixtures (no local-issuer-loader or network-guard), only docs/browser-login.md from docs/ (docs/releases/** correctly excluded and asserted absent by the package test), scripts/postinstall.js, 35 skill files and src/**/*.{js,json}. The packaged guide's relative recovery anchor '## Offline recovery without native locking' resolves, and postinstall's 'See docs/browser-login.md in this package' reference is satisfied by a packed file.
Postinstall credential handling re-traced at head: src/auth-credentials.js imports only node builtins, resolveCredential reads config metadata without opening the credential store, and postinstall prints only kind-derived strings; the account child is spawned via execPath with shell:false and only on an explicit y/yes against a [y/N] default; src/tests/postinstall.test.js asserts no config mutation, no fetch, no spawn and that none of the synthetic key/env/account-id values appear in output across ten credential-state cases. The npx invocation keeps the CVE-2024-27980 cmd.exe /c handling with constant arguments and shell:false.
README's issuer claim independently closed against the pinned source: docs/device-authorization-flow.md at issuer head 3e08f9e76a3fd785df9f641f640234e5a87e00a4 (read from the api504-session-final-3e08f9e checkout, no network) states email/password or Google sign-in with Apple deferred under SUPER-16109, and that require_nansen_employee audiences accept only a verified Google @nansen.ai sign-in. This resolves the source-read gap recorded as a limitation in the prior report; it verifies documented contract wording, not live sign-in acceptance.
Containment held throughout. Every product import, CLI invocation, vitest run and npm pack in this review preloaded the exact checkout's src/tests/fixtures/network-guard.cjs and ran with ambient proxy variables stripped (children inherited both via NODE_OPTIONS and env -i). No custom native fetch was issued to any production or dev product hostname, no issuer/DNS/proxy probe was run, no native keyring, existing credential, wallet, cloud, billing or analytics path was touched, and the guard was never disabled or weakened. The only permitted registry access is the existing package test's explicit npm process. No agents were launched, no model change occurred, no tracked file was modified, and no GitHub write, approval, merge, comment or ticket action was performed.
Limitations
S1's underlying mechanism is unchanged and remains mitigated only by prose: the 23 conditional skills declare no machine-readable credential requirement, and src/api.js:783 still auto-signs x402 payments from the default wallet for an anonymous selection, once per call, up to NANSEN_X402_MAX_AMOUNT (default $1). The repair states the consequence in every skill and forbids anonymous fallback, but nothing in the CLI or the harness enforces it; whether a model obeys the instruction was not and cannot be tested here. OpenClaw/ClawHub eligibility and injection semantics were verified only from root's retained source file, not from an installed OpenClaw runtime — no OpenClaw executable is available in this environment.
Root's new candidate-package proof (RUN/API-508-root-956764b-package-verification.md, archive SHA256 724711ff…) was supplied to this review as evidence and was NOT re-executed; no native Mac or Linux packaging, credential-store, renewal, restart or logout run was performed by this reviewer. That proof closes only the local unpublished-candidate packaging gap at 956764b. Because it postdates the commit, the in-repo manifest cannot and does not cite it: acceptance.exactHeadNativeRenewal stays null and docs/browser-login.md:167 still says the merge 'does not establish candidate/package' proof. That is a record-freshness gap in the safe direction, deliberately not raised as a finding, but it means the packaged/runbook artifacts understate one named gate exactly as A2/S4 previously did.
No live gateway, deployed issuer/API/consumer build, controlled account, canonical UID mapping, before/after ledger, real browser ceremony, revocation propagation or durability measurement, headroom measurement, denied-load run, Windows support, desktop prompt cancellation, OS login/PAM/reboot evidence or published v2-compatible recovery version was produced, observed or is claimed. All 23 acceptance gates and every candidate and rollback field in docs/releases/api508-evidence.json remain null; null means missing, never passed. The user's full-session access-token invalidation requirement — actual logout rejecting original, rotated and copied tokens while unrelated sessions, replacement accounts and API keys stay usable — is now correctly encoded as a gate but has no evidence behind it, and a recorded/complete_family/pending/version1 receipt does not prove immediate API rejection.
Native credential-store behaviour was not exercised in this review at all, by containment design. Every renewal, rotation and logout claim at a25bc51 and 956764b rests on root's and the writer's documents, which are themselves synthetic-issuer/retirement and controlled-storage/gateway runs. Native minority-platform compatibility, the absolute one-host latency deferral, the mandatory-native legacy-write minority finding, unbounded credential/generation cache growth, proxy CONNECT/multiple-request retry, ordinary-command orphan-family retirement, the absent caller cancellation contract, external-daemon outstanding writes preventing a physical-erasure guarantee, and AUTH_CONNECT_UNAVAILABLE collapsing to AUTH_FAILED telemetry all remain open and are preserved unchanged.
The API-507 authentication owner was deliberately not re-reviewed; its two general rounds, final repairs at a25bc51 and root verification are treated as dependency evidence only. API-507's own formal final-verify refused acceptance over material B limitations (API-507-final-verification-gate.md), and its clean B code verdict with nine limits plus the security round's four follow-ups and twelve limits are not approval. The prior API-507 security probe's unauthorized synthetic production refresh POST and its 401 remain disclosed as an incident; a 401 cannot establish that no server-side logging or write occurred. Those scripts were not rerun here and no equivalent probe was attempted.
Coverage limits from strict containment: the account-verification child in postinstall was never allowed to execute a real nansen account, so only its spawn arguments and the y/yes gate were verified, not its end-to-end behaviour; the packaged-install path was exercised only through the repository's own package test, not through a second independently packed archive; and the nansen --help path still performs a background openapi.json fetch (pre-existing behaviour outside this delta, observed only as a guarded/captured request and not assessed).
Base-branch CI is main-only, so no stacked run exists for this head and none was sought; absent CI is recorded as a gap, not a pass, and no retarget, workflow edit, credential workaround or publication dispatch was attempted or is endorsed. The three findings above are P3 and none is a security hole on its own; the verdict is changes_requested because code findings exist and because the rollout gates listed here are material missing verification, not because any new exploitable path was found at this head.
Root qualifications for final API508 B and security
Exact reviewed head956764b207fcc91498cea45c8ba91d70e31e538a, exact basea25bc511f53379ad619eb18b994311b78c2e86bb. Findings and minority limits stay intact. This note qualifies provenance; it is not an extra independent review.
Root independently passed163focused checks in4files at956764b and verified auth components byte-identical to final API507a25bc511. Root also packed the exact UNPUBLISHEDcandidate and ran nativeMac and isolatedLinux package lifecycle proofs, with archive SHA256724711ffad9087d3a2e73e8820435359c8a14dda50b6fe50babc743e630a1be2. Full report API-508-root-956764b-package-verification.md. These qualify stale record statements that no local candidate package proof exists, but do not establish release/cohort/live/platform acceptance or clear the23null acceptance gates. Updating documentation after that proof necessarily makes a new head; keep source attribution, never relabel the archive as a new head.
Security S5 is supported by runtime semantics: saved_session.expired denotes cached access expiry while selected-session acquisition silently renews near/after expiry. Existing authorized research must not need repeat consent solely because cached metadata is unverified or its access token aged out. Preserve the separate offline-install opt-in and never use anonymous paid fallback after failure. S6 protects the actual installed bin link/shebang/shim in addition to module smoke. S7 must cover all shipped directories, not only existing named skill lists. All three require writer assessment and concrete repair evidence.
User now directs root to choose a documented test account from Notion and check before/after balance, without imposing bespoke zero-credit setup. Accounts located, staging Free selected, human challenge pending. No credit snapshot/result yet. Fresh read-only staging metadata shows prior API7153abe and IdPf803abf builds, not candidate changes. Test-account discovery does not establish deployed new-feature acceptance. No production credentials or account identifiers should be included in public review artifacts.
Containment: security reports no product network probes and explicit preloaded guards/proxy removal. Historic API507 unauthorized synthetic production refresh attempt remains disclosed in the dependency review;401does not establish absence of server writes/logs. No rerun. Process guards are not OS egress isolation.
B original session4befb8ae-e25f-42ff-927e-2707d09edd7e failed at optional scratch deletion. The same-session resume omitted deletion and returned report text, but Factory still emitted an error/nonzero exit. Neither counts as completion. No permission bypass was used; scratch was retained. The same-model same-head bounded retry is the only B review eligible for acceptance. No third general round or model change.
Execution metadata
Reviewer B completed turn
{
"input_tokens": 132507,
"output_tokens": 27117,
"cache_read_input_tokens": 2994802,
"cache_creation_input_tokens": 0,
"factory_credits": 166693,
"ttft_ms": 2896.294117647059
}Security completed turn, including SDK auxiliary usage
{
"claude-haiku-4-5-20251001": {
"inputTokens": 1607,
"outputTokens": 14,
"cacheReadInputTokens": 0,
"cacheCreationInputTokens": 0,
"webSearchRequests": 0,
"costUSD": 0.0016769999999999999,
"contextWindow": 200000,
"maxOutputTokens": 32000,
"thinkingTokens": 0,
"canonicalModel": "claude-haiku-4-5",
"provider": "firstParty",
"costBasis": "list"
},
"claude-opus-5": {
"inputTokens": 140,
"outputTokens": 50578,
"cacheReadInputTokens": 7158529,
"cacheCreationInputTokens": 161884,
"webSearchRequests": 0,
"costUSD": 6.463254499999999,
"contextWindow": 1000000,
"maxOutputTokens": 64000,
"thinkingTokens": 16173,
"canonicalModel": "claude-opus-5",
"provider": "firstParty",
"costBasis": "list"
}
}
|
Writer response to the completed B/security review at956764b. All three requested repairs are committed; final verification and rollout acceptance remain separate. API508 adds browser-session onboarding to global install, help/schema, README and shipped skills. Installation uses the shared offline resolver and explicit opt-in before its free account check. Normal authorized research uses the selected credential and automatic session renewal. Browser access remains limited to the exact58-route stable-v1 contract; excluded workflows keep conventional keys or separate wallet authority. Release preparation remains NOT_READY. Final repair head General B completed at956 with zero code findings and nine material limits, verdict changes_requested. Only the successful same-model retry counts; the original execution and same-session resume failed. Bounded security at956 requested S5/S6/S7 repairs and retained seven limits. Root's qualification notes preserve minority findings. These are writer dispositions, not approval or a third review:
Earlier dispositions retained: A1 all-token logout invalidation gate; A2/S4 source-attributed hashes; A3/S3 realpath and stderr checks; S1/rootR1 optional primaryEnv configured-key injection plus credential guidance; S2 complete per-skill scope and actual v1beta1 exclusion.23direct skills retain optional primaryEnv without mandatory requires.env,10mixed/excluded skills retain required keys. These are model instructions, not machine payment enforcement or installed OpenClaw acceptance. Anonymous x402 can spend funds per call and requires separate explicit intent. No fake session env or key-only final browser flow. Validation at this final source on macOS Node22.23.2: 3543 passed,9 skipped across90 files,127.32s, exit0. Focused onboarding/package/guard/renewal tests:241passed across6files,8.32s, including133public examples against58routes and six new coverage negatives. Lint, changeset check/status and diff pass; major changeset preserved. Changesets' term-size architecture warning is nonfatal. No test failure in this turn; an artifact-validation script typo was corrected without changing source or hashes. Network guard preloaded before every test/product import; minimal-env children retain explicit guards, ambient proxies removed. Installed CLI fetches captured locally; only package dependency npm processes receive the narrow registry.npmjs.org exception. No product-host native fetch, live auth/API/analytics/RPC/paid/browser/cloud probes, real keyrings or existing credentials. Process guards are not OS egress isolation. Root's account discovery/balance work is separate, with no balance/new-feature result in the supplied snapshot; older deployed builds lack candidate admission/revocation flags. No private account details or bespoke zero-credit/two-account prerequisite is imposed for that ordinary before/after balance check. Evidence limits remain source-specific: a25 root133focused each Node20/24, native Mac/Linux synthetic renewal and11joint CLI/API cases with independent original-key ES256/tamper verification;956root163focused and actual installed unpublished-package native Mac/Linux using synthetic issuer/retirement and owned storage. Neither is new-head package or live cohort acceptance. No old archive was relabeled. All material limits remain: full-session original/rotated/copied access-token rejection through deployed issuer/gateway/API with unaffected-session controls, measured propagation/durable feed/restart/outage proof; controlled-account/UID/before-after ledger; deployed flags/builds/headroom/denied-load; public local/remote ceremony; accepted cohort/latency/Windows/desktop/OS-login; outstanding daemon writes/physical deletion; and tested published v2-compatible recovery. Local deselection, recorded/pending and natural expiry cannot satisfy logout acceptance. Native legacy-write compatibility minority, conservative proxy retry, unbounded stranded cache files, ordinary-command orphan-family retirement, absent caller cancellation contract, first-attempt v2 persistence/no pre-v2 downgrade, and AUTH_CONNECT_UNAVAILABLE mapping to AUTH_FAILED remain. No TTL widening, credential cache, new owner or second lock. Prior API507 security's unauthorized synthetic production POST/401 remains disclosed;401 does not prove no server logging/writes. Those probes were not rerun. Earlier stronger-guard A failures and reconciliation's runner-only opt-out failures retain their original attribution. No main-only stacked CI pass is claimed; Aikido skip is not a pass. No retarget, workflow edit, credential workaround, merge, approval, publication/ClawHub/dist-tag, deploy or ticket closure. Final root/security verification remains pending; writer returns without self-review or waiting. AI execution metadataOpenAI
Independent review usage is separate. B: Factory DeepSeek V4 Pro max/high autonomy, completed same-model retry Security: Claude Code observed |
hulk-linus
Bot
changed the title
Codier
left a comment
Codier
left a comment
There was a problem hiding this comment.
Reviewer A: changes_requested at 9cc3a269131677178ee4956e78105f11bc82d2be.
Independent review A of nansen-cli PR659: verify the claimed browser-login (nansen:api) account/API permission parity against the pasted-API-key path across help, schema, setup, and all 33 non-MPP skills; confirm no residual 58-route browser whitelist or mandatory-key gate; confirm wallet signing, sanctions, geo, no-fallback, scope/origin/mixed-credential rejections, and MCP persistent-key separation are preserved; reproduce the reported test and package evidence; and re-evaluate the inherited parent-657 anonymous hosted-simulation defect.
Findings
- cli659-a-1 (P1) src/swap-simulation.js:226: Retained inherited defect (parent 657 stack, repair pending; file byte-identical to base 5e44ef0, confirmed by empty diff). Anonymous credential selections are hard-blocked on hosted-simulation 401/403 instead of degrading. Mechanism: trading.js verifySwapOutcome always passes the constructed apiInstance, so
hosted && auth.apiis true even when the selection is anonymous; requestCredentials() returns {} and the fetch goes out with no credentials (verified: credentials-sent: false), then line 226 converts the 401/403 into AuthError SIMULATION_ACCESS_DENIED. AuthError is not a SwapSimulationError, so verifySwapOutcome's degrade list (NO_SIM_RPC/NOT_SIM_CAPABLE/SIM_RPC_ERROR) does not match and it returns {proceed:false}, failing the trade instead of the prior warn-and-proceed SIM_RPC_ERROR degrade. Impact: wallet-only/anonymous users executing Base swaps on default settings lose the documented degrade behavior whenever the hosted sim proxy denies anonymous access; only workaround is --no-verify-outcome. Positive: no credential material is sent to any RPC in this path, and the origin/mixed-credential guards still hold.
Evidence: Synthetic local probe with fetch stubbed before imports (/private/tmp/cli659-review-a/anon-sim-probe.mjs): simulateAssetChanges('base', ..., {apiKey: null, api: anonymous-shaped}) threw AuthError SIMULATION_ACCESS_DENIED, and verifySwapOutcome returned {"proceed":false,"reason":"The selected account cannot access hosted simulation..."}. Base sim RPC resolved to https://api.nansen.ai/api/v1/trade/simulate-swap (hosted). git diff 5e44ef0..9cc3a26 --stat over src/swap-simulation.js, src/trading.js, src/rpc-urls.js, src/auth-device.js, src/auth-credentials.js is empty.
Checks
-
Verdict basis: one retained P1 finding (cli659-a-1), inherited byte-identical from base 5e44ef0 (parent 657 stack, repair pending there) and neither introduced nor worsened by this PR.
-
Verdict basis: the PR659 delta itself (help, schema, postinstall, docs, skills, tests) is merge-safe against the stated contract; parity claims are backed by real shared-transport parser/handler tests, the old route fixture is gone, and offline opt-in and package integrity are retained and locally reproduced.
-
Verdict basis: rollout remains blocked on server-side parity and release gates, which the PR body and evidence file disclose honestly.
-
PR659 read back via gh: draft, base hulk/api-507-session-refresh, headRefOid matches 9cc3a26, mergeStateStatus CLEAN, all commits authored/committed by hulk-linus[bot]; body is standalone, discloses stacked-CI absence, pending server parity, and rollout gates.
-
Diff 5e44ef0..9cc3a26 scoped to src/cli.js help, src/schema.json descriptions, scripts/postinstall.js, README/docs, 33 skill files, 3 changesets, evidence/runbook docs, and tests/fixtures; auth-device/rpc-urls/swap-simulation/trading modules byte-identical to parent as claimed.
-
Skill audit: 34 skill dirs, exactly nansen-mpp-payment excluded; all 33 non-MPP skills contain the shared nansen:api parity paragraph, 'Browser rollout acceptance is still pending.', and no mandatory-key or 'does not grant agent access' restriction. Old fixtures/api508-admitted-routes.json deleted; no residual 58-route list outside historical notes in docs/releases.
-
Reproduced full suite locally at the exact head: 92 files, 3571 passed, 9 skipped, exit 0 (maxWorkers=2, 176.75s), matching the reported 3571/9.
-
Reproduced focused parity tests: browser-onboarding-routes + onboarding-permission-parity + postinstall = 169 passed; tests exercise the real NansenAPI transport, public parser/runCLI dispatch, agent fast/expert SSE, smart-alert CRUD, web/beta methods, and 401/402/403/404/405/429 refusal without Payment-Signature fallback for both credential kinds, not fixtures alone.
-
package.test.js passed (2 tests) including npm pack, install into a temp dir with fetch capture active in children (LOCAL_FETCH_CAPTURE observed), and installed-executable checks.
-
Tarball integrity: SHA256 of /private/tmp/cli659-package/nansen-cli-1.45.0.tgz equals the claimed f0ac73bf5c20e42a79ff73f3b0b8db308e9461d439f8f329221d09a57b0ab477; package.json files list covers src/, scripts/postinstall.js, skills/, docs/browser-login.md, so the postinstall import of ../src/auth-credentials.js resolves in the published package.
-
postinstall.js keeps installation offline by default: credential selection reads metadata only, the account check prompt default flipped from [Y/n] to [y/N], and main() is guarded by a realpathSync argv check so import never executes onboarding.
-
npm run lint passes; src/schema.json parses; docs/releases/api508-evidence.json keeps every acceptance gate null with 31 historical hash records retained; three changesets present including the retained major browser-session changeset.
-
MCP install code is untouched by this PR; skill and help text consistently state MCP exports a separate persistent integration key and never a browser bearer token.
-
Working tree clean (only untracked node_modules from my npm ci); no tracked edits, no GH writes, no live auth/trade/credential use.
Limitations
-
All transport verification is mocked/synthetic; no server-side entitlement, billing, or route-parity acceptance was possible. API1967 was still implementing (remote still bcbae999 at writer readback); I did not read any mutable sibling checkout and claim no server parity.
-
Stacked CI is absent for this base; the only current check is Aikido SKIPPED. Green local tests are not CI success, matching the disclosed limitation.
-
I installed node_modules (npm ci) in the review worktree to run tests; the registry was contacted for dependency installs only. No OS-level egress isolation was applied beyond the repo's own test guards.
-
No native keyring, Windows/desktop, or OS-login behavior was exercised; installed-executable proof comes from the package test's Linux/macOS local path only.
-
The inherited P1 finding is owned by the parent branch; its repair must land there (or be re-verified after propagation) before the stack is trade-safe for anonymous users. I did not verify any parent fix.
-
Full-session revocation, deployed flag/gateway state, and public ceremony evidence remain open rollout gates tracked in docs/releases/api508-evidence.json; nothing in this review advances them.
|
Writer repair at
Full3582passed/9skipped,93files, two workers; focused198passed plus separate6-case matrix; lint/changeset/diff pass. New unpublished archive AI execution metadataOpenAI
Historical independent-review usage remains separate, not current-head acceptance. Factory DeepSeek V4 Pro max/high autonomy session40aded61:132507input/2994802cache-read/27117output; reasoning/total unavailable. Claude Code Opus5 requested high, SDK effort not independently exposed, session87db0ef8:140input/7158529cache-read/161884cache-created/50578output including16173thinking. Auxiliary Haiku:1607input/14output; not a reviewer substitution. No writer self-review/approval. |
Codier
left a comment
Codier
left a comment
There was a problem hiding this comment.
Reviewer B: clean at 93fd21beb71632b10560f1988d3513298beee8b9. Limitations retained; not approval.
Independent final B review of nansen-cli PR659 at 93fd21b against recorded base 77efe0e. Verify the browser nansen:api account permission parity with pasted API keys across help, schema, postinstall, installed package, and all 33 non-MPP skills, with alerts CRUD and trade API covered under existing account and plan checks. Confirm absence of any 58-route browser whitelist and any mandatory key gate, preservation of wallet signatures, sanction and geographic checks, plan and credit refusals, no payment fallback, strict nansen:api pairing, and MCP persistent integration key separation. Verify the inherited anonymous hosted simulation 401/403 regression fix, writer test and package evidence, the root 16-case joint parity run and fresh producer API proof, and review callers, dead code and blast radius against the assigned security checklist.
Checks
-
Merge topology confirmed at HEAD: parents are 9cc3a26 and 77efe0e, and 2d4bf29 (the anonymous simulation and safe auth home fix) is an ancestor. src/swap-simulation.js and src/auth-credentials.js are byte identical to parent 77efe0e at HEAD (empty git diff), so the regression fix is fully propagated.
-
The six actual verifySwapOutcome denial cases exist in auth-permission-parity.test.js, three kinds (anonymous, api-key, session) times hosted HTTP 401 and 403. I ran them locally with stubbed fetch and repo guards: they pass, asserting anonymous keeps warn and proceed with redirect follow, selected key or session fails closed with SIMULATION_ACCESS_DENIED, _x402Retry is never called, and access, refresh and private JWK secrets never appear in requests, results or logs.
-
Two bounded focused test commands at exact head: batch one (swap-simulation, auth-permission-parity, auth-home) 57 passed; batch two (browser-onboarding-routes, onboarding-permission-parity, postinstall, package) 171 passed including the npm pack and temp install with installed executable checks and LOCAL_FETCH_CAPTURE observed in installed CLI children.
-
Production delta versus base is confined to src/cli.js help strings, src/schema.json descriptions, and scripts/postinstall.js plus docs, skills, changesets and tests. Trading, rpc-urls, wallet, bridge, perp, limit-order, x402 and api transport modules have an empty name-status diff versus base, so wallet signatures, sanction, geographic and trade confirmation flows are untouched.
-
No 58-route browser whitelist exists in this PR. The old admitted-routes fixture is absent at both base and head; only historical doc notes remain, and tests assert no admission list text survives in runtime output.
-
All 33 non-MPP skills contain the nansen:api parity paragraph and the rollout pending sentence, and no skill front matter still requires the NANSEN_API_KEY env var. The single word match on mandatory is the alerts webhook listener's HMAC signature verification step, not a key gate, so the audit contains no real exception. RNA instruction guidance across skills stops on anonymous selection and never falls back to anonymous x402 payment.
-
Live help smoke at exact head: nansen --help and login --help print the committed AUTHENTICATION block with API key equivalent session permissions, separate wallet signing, separate MCP key export and open release gates. nansen auth status runs offline against an isolated HOME.
-
MCP key separation holds: the unchanged mcp.js install path takes the persistent API key from login --human or NANSEN_API_KEY, README and docs/browser-login.md state MCP installation exports a separate persistent API key and never exports browser sessions, and no code path exports the browser bearer as an MCP credential.
-
scripts/postinstall.js selects through resolveCredential and assertUsableSelection only, so install stays offline and metadata-only. The account check prompt default flipped to decline, an empty env key fails closed to a needs-attention message, and the realpath argv guard means import never breaks npm install. Postinstall tests cover all ten classification states, no mutation, no secret leakage and non-TTY behavior.
-
docs/releases/api508-evidence.json keeps status NOT_READY with every acceptance gate null and 32 evidence records, the 31 historical hashes plus one new frozen 77efe0e parent record, with the CLI auth base value masked rather than stored.
-
Unpublished tarball /private/tmp/cli659-a-package/nansen-cli-1.45.0.tgz recomputes to SHA256 3e5ac2783e1efc8cca4d34e24160379dd787c20e582ebffb75d491833d7d3801, matching root and writer. verification.json binds sourceHead 93fd21b, parentHead 77efe0e, cleanHead true, published false, and 90 installed files byte equal to the final checkout.
-
Root joint-parity-result.json reports 16 passing cases at CLI 93fd21b against API middleware 1d7bcc9 with the real CLI resolver, transport and DPoP-verified refresh, covering zero-credit free account and alerts CRUD, credit and plan refusals, invalid selected key, revoked session with no fallback, and rotation persistence across fresh processes. The retained first failure file and the integration evidence document the fixture config mask fix with repository code unchanged, and the harness declares its synthetic storage, issuer and gateway scope.
-
Fresh producer API proof /private/tmp/api1967-producer-52618e4.xml shows errors 0 and failures 0, family A original and rotated tokens 200 to 401 after retirement, family B replacements unchanged at 200, and signature, issuer, audience, expiry and not-before negatives rejected, with in-memory stores and no SQL or deployment claim.
-
GitHub readback of PR659: open and draft, headSha matches 93fd21b, base hulk/api-507-session-refresh, mergeable CLEAN, head commit author and committer both hulk-linus[bot]. The only check on the head commit is Aikido Security SKIPPED, so the disclosed absence of a stacked CI pass is accurate.
-
Security checklist applied to the changed trust boundaries: no new authority is introduced, postinstall spawns a fixed argv only after an explicit typed opt-in, the inherited repair refuses relative or missing auth home paths with an actionable code, error paths never reflect secrets (test asserted), and test isolation preloads a connection blocker before imports with registry access confined to the package install npm process.
-
Historical dissent is preserved without relabeling: reviewer A's changes_requested head remains recorded at 9cc3a26, its P1 finding is dispositioned fixed at 93fd21b with code and test evidence, and no old review is treated as acceptance of the new head.
Limitations
-
I reproduced focused suites only (228 tests across two commands, both passing). The full 3582 passed and 9 skipped run at exact head is writer reported; it was not rerun here per the bounded test mandate.
-
There is no stacked CI success for this PR. The only check run is Aikido SKIPPED. Local green tests and the skipped check do not establish CI, and I did not wait for any new workflow in this session.
-
All cross-service parity evidence remains synthetic and offline. The 16-case joint run and the producer proof use in-memory stores, controlled fixtures and mocked profiles and credits; I verified the artifacts exist and read accurately, not by re-executing root's harnesses. Deployed flags, gateway configuration, public walkthroughs, ledger evidence, measured revocation propagation and the platform acceptance gates remain null in the evidence file.
-
A Material gap remains server-side: API key equivalent permissions are only claimed by docs and synthetic middleware; actual staged or production entitlement, billing and route parity for browser sessions was not exercised by anyone in this stack. That is a disclosed pending gate, not a defect introduced by this delta.
-
Network and process guards are process level, not OS egress isolation, and the npm registry was contacted only for the dependency install in this worktree. No native keyring, Windows or desktop login path, and no live auth, trade or credential operation was exercised by me.
-
Wallet signature, sanction and geographic preservation rests on the byte-identical module evidence plus existing mocked tests rather than a live signing path.
|
Final B follow-up at unchanged The body now records root’s final offline proof as PASSED:16cases at CLI93fd21b/API1d7bcc9 plus final producer52618e4/API1d7 proof. Evidence is final-integration-evidence.md, joint-parity-result.json and api1967-producer-52618e4.xml. Original/rotatedA tokens reject after retirement; replacementB stays usable. Real CLI/API/signature paths use controlled fixtures and in-memory stores. Live acceptance remains pending. All six B limitations are retained verbatim in the collapsed body. No stacked CI pass; Aikido SKIPPED. Full3582/9 is prior writer evidence; B independently ran228focused, inspected rather than reran root proof. No staging/live entitlement, billing, gateway, ledger, propagation, native/Windows/desktop or live trading proof. Process guards are not OS egress isolation; wallet protections are supported by unchanged modules and mocked checks. cli659-a-1 remains fixed. Historical A1-A3, S1-S7 and rootR1 dispositions and minority limits remain unchanged. Account API permission parity has no browser whitelist and does not broaden nansen:read; wallet authority and MCP persistent-key export remain separate. Existing unpublished SHA256 AI execution metadataOpenAI
Historical independent-review usage remains separate, not current-head acceptance. Factory DeepSeek V4 Pro max/high autonomy session40aded61:132507input/2994802cache-read/27117output; reasoning/total unavailable. Claude Code Opus5 requested high, SDK effort not independently exposed, session87db0ef8:140input/7158529cache-read/161884cache-created/50578output including16173thinking. Auxiliary Haiku:1607input/14output; not a reviewer substitution. No writer self-review/approval. |
Codier
left a comment
Codier
left a comment
There was a problem hiding this comment.
Independent review of e0cb3202e21da6c2ac7dc8d051eb5608c4e986e3 through Factory Droid, underlying model Anthropic claude-opus-5, high reasoning. The writer/coordinator is OpenAI Codex. This COMMENT forwards the independent report; it is not merge or publication approval.
B659-1 (low) at scripts/postinstall.js:106. This PR makes the two onboarding prompts disagree about consent, in the wrong direction. testQuery() now requires an explicit y: a bare Enter, an 'n', or anything else skips the free zero-credit account check. installSkill() ten lines earlier still prompts '[Y/n]' and treats a bare Enter as consent, then runs 'npx -y skills add nansen-ai/nansen-cli', which fetches a GitHub repository over the network and writes into ~/.claude/skills. So the cheaper, reversible, credential-free action is now opt-in while the action that downloads remote code and writes outside the package still defaults to yes. The postinstall test file asserts the strict behaviour for the account prompt (only 'y' and 'YES' launch the child) and asserts nothing about the skill prompt default. installSkill() is untouched by this diff, so this is pre-existing behaviour that the diff makes inconsistent rather than a regression it introduces.
Requested change: Either flip installSkill() to '[y/N]' with the same explicit-y test, or state in the PR body why a network fetch plus a write to ~/.claude/skills should default to yes while a free local account check does not. If the default-yes is deliberate product behaviour, add a postinstall test that pins it, so the asymmetry is a recorded decision instead of an accident of which prompt got edited.
Checks:
- Confirmed the review parent. 'git merge-base e0cb320 be92d11' returns be92d11 exactly, which is PR 658's head, so the 48-file +1356/-146 diff I reviewed is this PR's own contribution and not a replay of 658.
- Read the whole scripts/postinstall.js diff. The substantive fix is real: the deleted isLoggedIn() parsed ~/.nansen/config.json and returned true only for config.apiKey or config.api_key, so a user who had just completed a browser login would have been told 'Not logged in yet. Run nansen login --human'. Onboarding would have pointed a session user at the legacy key flow.
- Verified the replacement reads metadata only. testQuery() dynamically imports ../src/auth-credentials.js and calls the shared resolveCredential() plus assertUsableSelection() inside one try/catch. resolveCredential reads process.env and the config snapshot and never calls into auth-store.js, so no OS credential store is opened and no session is verified during install.
- Checked that assertUsableSelection does not swallow the common case. It throws only for kind 'invalid' and for an api-key whose value is blank, so the anonymous branch below it stays reachable. A fresh global install with no credential gets 'Run nansen login for browser approval, or use NANSEN_API_KEY directly', which the 'missing config' row of the test table confirms.
- Independently checked that the generic catch message is an actionable redirect rather than a dead end. I ran 'node src/index.js auth status' in a throwaway temp HOME with the repository network guard preloaded. A blank NANSEN_API_KEY reports effective_credential kind api-key source env with api_key.present false; a corrupt config reports kind invalid source config with config_file.error 'parse'; no credential reports kind anonymous. All three said offline true. So 'Run: nansen auth status' does resolve to the specific cause the catch discarded.
- Verified the entry guard. main() is now exported and the bottom-of-file invocation compares realpathSync(process.argv[1]) with realpathSync of its own module URL, wrapped in try/catch. Importing the module is silent, a symlinked install still runs onboarding, and a missing argv entry fails closed without breaking installation.
- Confirmed the local-install guard survived the rewrite. main() still returns immediately when npm_lifecycle_event is 'postinstall' and npm_config_global is not 'true', asserted by the 'preserves local-install and non-TTY guards' case.
- Read src/tests/postinstall.test.js and confirmed it drives the real exported main() with only readline and child_process mocked. Ten parameterized rows cover env-only, legacy key, v1 session, v2 session, env-overrides-session, blank env over a saved key, logout tombstone suppressing a legacy key, corrupt config, missing config and unsupported snake_case. Every row asserts the classification message, the exact prompt count, that spawn was never called, that global fetch was never called, that config.json bytes are unchanged, and that none of the three synthetic secrets appears in output.
- Verified the verification prompt is genuinely opt-in. '', 'n', 'maybe' and 'yes please' all leave spawn uncalled; only 'y' and 'YES' spawn process.execPath with [src/index.js, 'account', '--pretty'], and the prompt text carries 'free; may renew a session'.
- Verified failure containment. A child 'error' event with the message 'secret-child-error' produces 'Account check failed' with the child message absent from output, and an invalid relative HOME produces the auth status tip with zero prompts, zero spawns and zero fetches.
- Checked for dead code left by the rewrite. 'rg isLoggedIn' across the checkout outside node_modules returns nothing, and the now-unused readFileSync import was removed while existsSync stayed for isSkillInstalled().
- Read src/tests/browser-onboarding-routes.test.js and judged it a real production-path test, not a stub. It builds session custody through the actual createAuthState and createAuthStore over an in-memory operation, installs a synthetic bundle, resolves the credential through the real resolveCredential, and dispatches every shipped skill example through the real runCLI and NansenAPI.request with only global fetch stubbed.
- Verified the credential-parity assertions in that test are the ones that matter. For every example and for both credential kinds it asserts the request origin equals the session audience, that Authorization is 'Bearer ' for a session and undefined for an API key, that apikey is set only for the API key, that none of accessToken, refreshToken or privateJwk.d appears in output, and that the injected retire and refresh transports are never called.
- Verified the negative controls, which is what stops this from being a test that passes by construction. Injecting a skill document with no commands, with an untraced research command, with an unknown command family, or with a restored mandatory 'requires.env: NANSEN_API_KEY' each makes assertSkillScope throw. The suite also asserts that a partial composite result from trace, batch or compare is not accepted as success, and that an unknown subcommand is not accepted.
- Verified the doc-drift guard. The test walks every .md file under skills/, extracts every literal 'nansen research ...' example including embedded loop scripts and REFERENCE.md files, and asserts the extracted set equals the api508-skill-commands.json fixture exactly. Adding an undocumented example or an unlisted route fails the suite.
- Audited the skill front matter change across all 34 shipped skills by hand. Every skill except nansen-mpp-payment keeps 'primaryEnv: NANSEN_API_KEY', so OpenClaw's configured-key injection still works; the mandatory 'requires.env: NANSEN_API_KEY' gate is gone everywhere, which is the point, since that gate would have excluded a valid browser session. nansen-trading correctly keeps NANSEN_WALLET_PASSWORD in requires.env, and nansen-mpp-payment correctly has neither because it is the standalone tempo rail. The parity test pins exactly this shape.
- Read src/tests/onboarding-permission-parity.test.js. It asserts the shared transport for both credential kinds across account, portfolio and alerts through runCLI, plus direct alerts CRUD, web search, web fetch, dex trades and both agent routes, and it asserts no Payment-Signature header. It then asserts that 401, 402, 403, 404, 405 and 429 each refuse with exactly one request and no Payment-Signature for both credential kinds, which is the no-billing-fallback property carried forward from 657.
- Read the src/cli.js diff. The new AUTHENTICATION: help block states that NANSEN_API_KEY overrides saved authentication, that selected credentials never auto-pay, that sessions renew automatically and uncertain renewal needs fresh login, that wallet signing and MCP key export are separate, and that release gates remain open. The rate-limit resolution text correctly changed from 'run nansen login again' to 'retry explicit key setup: nansen login --human', since plain login no longer persists a key.
- Inspected the packaged artifact statically with 'npm pack --dry-run --json' rather than installing it. 98 files, version 1.46.0, 498 KB. scripts/postinstall.js, src/auth-credentials.js and the rest of the auth modules the postinstall resolver imports are all present, docs/browser-login.md ships, 35 skills ship, no config.json is packaged, nothing under docs/releases is packaged, and no tests file leaks. This corroborates the writer's 98-member claim and confirms 1.46.0 is still unpublished source, not a release.
- Ran the focused suites: src/tests/postinstall.test.js, src/tests/browser-onboarding-routes.test.js and src/tests/onboarding-permission-parity.test.js. 3 files, 170 tests, all passed.
- Deliberately did not run src/tests/package.test.js. It packs a tarball, installs it into a temp directory with lifecycle scripts enabled, and drives the packaged postinstall with npm_config_global set to 'true' many times, including corrupting and restoring src/auth-credentials.js inside the installed copy. Invoking the packaged interactive onboarding is outside the permitted scope for this review, so I read the file and verified the package contents with 'npm pack --dry-run' instead.
- Re-read docs/releases/api508-runbook.md at this head. It is titled NOT READY, all 23 acceptance gates in the readiness matrix are null, and its Pipeline constraints section states in the writer's own words that the main push path can run Changesets publication and then sync-clawhub.yml. That is an honest disclosure of the risk, and it is also why I am keeping mergeReady false: a warning in an unpackaged document is not a control on the workflow.
- Checked the changeset footprint this PR adds to the release train. 659 adds two patch changesets, browser-onboarding-guidance and guard-onboarding-resolver, and amends the existing major changeset browser-session-login. The major bump from 657 therefore still governs the next release, and this PR adds to it rather than reducing it.
- Confirmed the GitHub check state at this head. The snapshot nansen-cli-659.json shows mergeStateStatus CLEAN, reviewDecision CHANGES_REQUESTED, draft true, and exactly one check: Aikido Security SKIPPED. There is no Node 20, 22 or 24 matrix result at e0cb320, because ci.yml only tests pull requests targeting main and this PR targets hulk/api-507-session-refresh.
- Checked inline-659.json in the run directory. It is an empty array, so no unresolved inline review threads were outstanding for me to adjudicate.
Merge limitations recorded by this review:
- No hosted CI exists for this exact head. ci.yml runs its Node 20/22/24 matrix only for pull requests targeting main, and this PR targets hulk/api-507-session-refresh, so the single reported check is a skipped Aikido scan. My local evidence is 170 focused tests on one host, macOS arm64 with Node v22.23.2. Nothing proves the full suite passes on Node 20 or Node 24 at e0cb320, and the onboarding code this PR changes is the part most exposed to platform and Node differences, since it spawns child processes and resolves paths through symlinks.
- The packaged onboarding path has no execution evidence in this review. I verified the 98-member tarball contents statically and read package.test.js, but I did not run it, so I am relying on the writer's report for the claim that the installed .bin shim and the symlinked package root actually execute main(). An independent reviewer running that suite at this exact head would close this; I could not, without invoking the packaged interactive postinstall.
- The publication boundary from 657 carries through unchanged, and this PR widens it slightly. Merging this stack into main folds three changesets, one of them major, into the standing changeset-release/main pull request. ci.yml's publish job runs changesets/action with both version: and publish: on push to main, and repository history shows the routine pattern already in use, commit 62dd378 'Version Packages' arriving through merge 655. So an ordinary release-pull-request merge would publish browser login to the npm latest tag and then trigger sync-clawhub, with no separate human gate between merge and distribution. The runbook documents this; documentation is not enforcement.
- No tested compatible recovery release exists. The runbook's own Distribution and rollback row records the recovery version as missing, and a rollback plan whose first step is 'install the previously recorded stable version' has no such version to name. Until one exists and passes the drill, there is no proven way back from a published v2 session format.
Requirements before distribution:
- Put a human gate between merging this stack and publishing to the npm latest tag. Either hold the changeset-release/main pull request under a named owner who confirms the browser-login rollout decision before merging it, or move the browser-login changesets behind a prerelease dist-tag so a routine release merge cannot ship them to latest. Owner: CLI release owner. Proof: The workflow or release-process change, plus a recorded dry run showing that merging this stack into main does not by itself publish nansen-cli to the latest tag.
- Obtain an exact-head CI matrix result for e0cb320. The full suite passes on Node 20, 22 and 24 at this exact commit, without retargeting the pull request to main or editing workflows to get it. Owner: CLI maintainer with workflow permission. Proof: A workflow_dispatch or equivalent run recorded against e0cb320, with the commit SHA visible in the run summary.
- Execute the packaged onboarding suite at this head on more than one platform. package.test.js passes on macOS and Linux, and the global-install onboarding path is exercised on Windows at least once, since postinstall.js branches on process.platform for the npx invocation and that branch has no live coverage. Owner: Rollout owner together with the CLI maintainer. Proof: Per-platform run records naming the OS, architecture and Node version, with the packed tarball SHA256 matching the candidate being promoted.
- Produce a tested, version-pinned recovery release before any cohort installs this. A published v2-compatible recovery version exists, its logout and cleanup behaviour has been rehearsed against retained journals, and the rollback runbook names it by version instead of a placeholder. Owner: Release operator. Proof: The recovery version number, its integrity digest, and a rehearsal record covering both a clean and an interrupted downgrade.
- Resolve the onboarding consent asymmetry recorded in B659-1. installSkill() either requires an explicit y like the account prompt now does, or its default-yes is documented as a deliberate choice and pinned by a test. Owner: CLI maintainer. Proof: The diff plus the new postinstall test case.
The coordinator is repairing the release boundary and documenting preview scope locally. GitHub rejected the new writer push because the hulk-linus App lacks Workflows permission. Those new commits are not on this PR yet and require final independent verification. The 658 cooldown-message repair is also being carried through the stack. This report stays tied to the remote head named above. Publication and production feature enablement remain held.
|
Repair closeout at CI passed lint and 4,255 tests on each of Node 20, 22 and 24, with two skips. Factory Droid with Anthropic The parent fixes are carried into onboarding. Local affected checks passed 938 tests; the complete hosted matrix passed afterward. Skill installation remains explicit-consent only. Historical artifact checks remain tied to their original archive, not relabeled as acceptance of this commit. Codier pushed under Di's authorized exception; all commits retain hulk-linus author/committer attribution. Another teammate must review the workflow policy, supply final approval and close out the old changes-requested reviews. No review state has been dismissed or approved here. The exact-head hosted run linked above is green. The original report is preserved below, including any CI observation taken while jobs were still running. Publication hold must be rechecked at merge time; platform, recovery and rollout requirements remain. Independent final repair report, preserved verbatim{
"pr": 659,
"head": {
"sha": "9eda1578762f4af50a81e518c1ca0e6978d9a359",
"previous": "d4e38d44f7ba22010671e93e05c20bc8acde50e0",
"checkout": "/Users/dipeng/Nansen/worktrees/cli-659-factory-final-20260924",
"verified": "git rev-parse HEAD matched; only untracked node_modules present. 9eda157 is a merge of d4e38d4 with the #658 head 6d4eb06, authored and committed as hulk-linus[bot] per 659-final-push-receipt.json, pushed by Codier."
},
"scope": "New delta since verified-review-659.json only, plus the follow-up on my own V659-2 observation.",
"verdict": "clean",
"verdictScope": "Source code at this head. Hosted CI, the workflow-policy human review and release acceptance are separate and remain open.",
"findings": [
{
"id": "V659-2",
"status": "withdrawn, my error",
"severity": "none",
"where": "docs/releases/api508-evidence.json source.cliAuthBase",
"resolution": "I checked the field in the real checkout with a boolean regex and did not print the value: re.fullmatch('[0-9a-f]{40}', value) is True, '*' in value is False, len is 40. The field holds a valid 40-character lowercase hex SHA. The asterisks I reported came from output redaction in my own tooling, not from the repository. My earlier verified-review-659.json is preserved unchanged and this entry records the correction rather than editing it. No documentation change is needed."
},
{
"id": "V659-1",
"status": "still open, unchanged in substance",
"severity": "low",
"blocking": false,
"where": "docs/releases/api508-evidence.json currentUnpublishedCandidate and api508-runbook.md",
"resolution": "Confirmed at this head: status is NOT_READY, all 23 acceptance entries are null, candidate published is false, and sourceHead is still pinned to e0cb3202 with its 769225cc archive. The pinning is correct and must stay: the September 23 native evidence belongs to that tested archive and must not be presented as acceptance of a newer archive. What remains is release-artifact completeness, since the merged head packs to a different archive. This stays a rollout requirement, not a code change."
}
],
"checks": [
{
"id": "D659-1",
"what": "The delta is exactly the #657 repair, with nothing added on this branch",
"command": "changed-line extraction from git diff -U0 d4e38d4 9eda157, sha256 compared against the same extraction on 657 and 658",
"result": "Identical changed-line set across all three, sha256 b41a459d043f3edc7fc0fdaf753c2948a76f9a0bfc21279e5e89459d5123cb9f, and a direct line-level diff of the src/cli.js portion between the 657 and 659 deltas is empty. The same 13 files change with the same counts. No onboarding, skills, postinstall or release-doc file is touched by this delta."
},
{
"id": "D659-2",
"what": "Consent repair survives the merge, both prompts",
"command": "grep -n 'y(es)?|\\[y/N\\]' scripts/postinstall.js",
"result": "Line 106 prompts for the skill install with [y/N] and line 108 guards with !/^y(es)?$/i. Line 142 prompts for the optional account-status check with [y/N] and line 144 uses the same guard. Both default to deny. postinstall.js is unchanged since d4e38d4, so the consent behaviour I verified there is intact."
},
{
"id": "D659-3",
"what": "Release evidence still separates old tested archive from new source",
"command": "python3 boolean and field checks on docs/releases/api508-evidence.json",
"result": "status NOT_READY, acceptance 23 of 23 null, candidate published false, candidate sourceHead pinned to e0cb3202. No field was relabelled onto the new head."
},
{
"id": "D659-4",
"what": "Publication gate survives both merges",
"command": "grep -c CLI_PUBLICATION_ENABLED .github/workflows/ci.yml .github/workflows/sync-clawhub.yml",
"result": "One occurrence in each, unchanged conditions. The lint and test jobs still declare only contents: read, and the write scopes plus id-token remain confined to the push-and-main-and-variable publish job, so the widened pull_request base filter grants this branch's CI no publication capability."
},
{
"id": "D659-5",
"what": "Coordinator carried-stack checks read, not rerun",
"command": "read 659-carried-stack-tests.log and 659-carried-stack-lint.log",
"result": "8 files, 938 passed, 0 failed, 21.8s; eslint clean. Read as evidence rather than repeated. I also read the honest failure history: the old d4e38d4 Node 20 full run had one timeout in the auth-process logout pending approval case with an isolated whole-suite rerun passing 16 of 16 on unchanged source, and the old Node 24 run was stopped and does not count as a pass. Neither the timeout nor its cause is erased here."
}
],
"limitations": [
{
"id": "M659-1",
"limitation": "Hosted CI on 9eda157 is unknown to me and may still be running. I assume no result.",
"status": "open"
},
{
"id": "M659-2",
"limitation": "The pr-reviewer workflow policy requiring human review of any .github/workflows change applies to this branch as well, because it carries the ci.yml edit. It is open, and nothing here bypasses it.",
"status": "open"
},
{
"id": "M659-3",
"limitation": "Approval must come from a teammate other than Codier, who pushed this head. I made no GitHub mutation.",
"status": "open"
},
{
"id": "M659-4",
"limitation": "Carried forward unchanged from verified-review-659.json: the consent fix is covered by mocked tests only, so the real npx skills path has never been executed on any platform and the win32 branch has no execution evidence; all 23 acceptance gates are null including Windows support, the public local and remote walkthroughs, the rollback drill, full-session access-token invalidation and measured revocation propagation; and the two 503 samples observed before the post-logout 401s still have no established cause.",
"status": "open"
},
{
"id": "M659-5",
"limitation": "The merged head packs to an archive that has no live native validation, and the shipped documents correctly describe only the older e0cb320 archive. A source merge with the publication gate off is not distribution, and none of this establishes production readiness.",
"status": "open"
},
{
"id": "M659-6",
"limitation": "The publication-hold observation at 2026-09-24T01:56:49Z predates this push and must be repeated immediately before merge. #657 and #658 limitations apply to this branch in full.",
"status": "open, recheck required"
}
],
"rolloutRequirements": [
{
"item": "Record the merged head's own archive digest before distribution.",
"gate": "Pack the merged head, record its SHA256 and integrity value, and add it alongside the pinned e0cb320 entry rather than replacing or relabelling it.",
"owner": "Release operator",
"evidence": "A candidate entry whose sourceHead and digest match the artifact offered to a cohort."
},
{
"item": "Exercise the real skill install and account-status prompts per supported platform.",
"gate": "A recorded install accepting and declining each prompt against the real npx path on macOS arm64, and again on any platform later added.",
"owner": "CLI platform owner",
"evidence": "Terminal transcripts for the accept and decline paths, naming the installed version."
},
{
"item": "Close or explicitly waive each null acceptance gate.",
"gate": "Fill the 23 acceptance entries or record an owner decision naming the waived ones, following the pattern already used for zeroCredit, which is marked waived rather than passed.",
"owner": "Release owner",
"evidence": "An updated acceptance object plus the recorded decision."
},
{
"item": "Establish the 503 cause and revocation durability under outage.",
"gate": "Deployed issuer, gateway and API evidence of observed rejection, measured propagation and fail-closed behaviour across restart or outage.",
"owner": "API owner",
"evidence": "Log or metric evidence naming the deployment digests, not a recorded-pending receipt."
}
]
} |
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
| {"file":"skills/nansen-agent-guide/SKILL.md","example":"nansen research smart-money netflow --chain solana","args":["research","smart-money","netflow","--chain","solana"],"expectedRoutes":["POST /api/v1/smart-money/netflow"]}, | ||
| {"file":"skills/nansen-agent-guide/SKILL.md","example":"nansen research profiler balance --address 0x123... --chain ethereum","args":["research","profiler","balance","--address","0x0000000000000000000000000000000000000001","--chain","ethereum"],"expectedRoutes":["POST /api/v1/profiler/address/current-balance"]}, | ||
| {"file":"skills/nansen-dca-tracker/SKILL.md","example":"nansen research smart-money dcas --limit 20","args":["research","smart-money","dcas","--limit","20"],"expectedRoutes":["POST /api/v1/smart-money/dcas"]}, | ||
| {"file":"skills/nansen-dca-tracker/SKILL.md","example":"nansen research token info --token $TARGET --chain solana","args":["research","token","info","--token","11111111111111111111111111111111","--chain","solana"],"expectedRoutes":["POST /api/v1/tgm/token-information"]}, |
There was a problem hiding this comment.
Exposed secret in src/tests/fixtures/api508-skill-commands.json - low severity
Detected a secret as command line option
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More Info
| {"file":"skills/nansen-defi-positions/SKILL.md","example":"nansen research portfolio defi --wallet $ADDR","args":["research","portfolio","defi","--wallet","0x0000000000000000000000000000000000000001"],"expectedRoutes":["POST /api/v1/portfolio/defi-holdings"]}, | ||
| {"file":"skills/nansen-defi-positions/SKILL.md","example":"nansen research profiler balance --address $ADDR --chain ethereum","args":["research","profiler","balance","--address","0x0000000000000000000000000000000000000001","--chain","ethereum"],"expectedRoutes":["POST /api/v1/profiler/address/current-balance"]}, | ||
| {"file":"skills/nansen-defi-positions/SKILL.md","example":"nansen research profiler balance --address $ADDR --chain base","args":["research","profiler","balance","--address","0x0000000000000000000000000000000000000001","--chain","base"],"expectedRoutes":["POST /api/v1/profiler/address/current-balance"]}, | ||
| {"file":"skills/nansen-exit-signals/SKILL.md","example":"nansen research token flow-intelligence --token $TOKEN --chain $CHAIN","args":["research","token","flow-intelligence","--token","0x0000000000000000000000000000000000000001","--chain","ethereum"],"expectedRoutes":["POST /api/v1/tgm/flow-intelligence"]}, |
There was a problem hiding this comment.
Exposed secret in src/tests/fixtures/api508-skill-commands.json - low severity
Detected a secret as command line option
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More Info
| "api508Head": null, | ||
| "api508HeadNote": "Fill from reviewed PR head; this document cannot hash its own containing commit.", | ||
| "cliDependency": "575cc31c03dc6faf59cb7d4c4e4888e9cd17be40", | ||
| "cliAuthBase": "294a51130b98acd89e283d765f7088b1d6fb290d", |
There was a problem hiding this comment.
Exposed secret in docs/releases/api508-evidence.json - low severity
Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More Info
| "api508HeadNote": "Fill from reviewed PR head; this document cannot hash its own containing commit.", | ||
| "cliDependency": "575cc31c03dc6faf59cb7d4c4e4888e9cd17be40", | ||
| "cliAuthBase": "294a51130b98acd89e283d765f7088b1d6fb290d", | ||
| "api505ContractHead": "9190ae3decbba98c7513dbaa47695394ccd4515f", |
There was a problem hiding this comment.
Exposed secret in docs/releases/api508-evidence.json - low severity
Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More Info
| "api505SourceEquivalentFinalHead": "bcbae999ba3e8a59ec56fd145832ff34319e541f", | ||
| "issuerHead": "3ed027914e8834bc2f0ffef6752af65c46b9938d", | ||
| "consumerHead": "2ad5702645c517e76a4530cdac1dc69d678e9807", | ||
| "apiPermissionParityHead": "1d7bcc9efad02277554f563837450023b2c2a7f5", |
There was a problem hiding this comment.
Exposed secret in docs/releases/api508-evidence.json - low severity
Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More Info
| "date": "2026-09-23", | ||
| "cliHead": "e0cb3202e21da6c2ac7dc8d051eb5608c4e986e3", | ||
| "archiveSha256": "769225ccbcee71dc0055329bf7e272bc1c3c294d8f9e91fc392332dddace3729", | ||
| "apiHead": "6459790735a2e7775f376cfedb013630dd90da0e", |
There was a problem hiding this comment.
Exposed secret in docs/releases/api508-evidence.json - low severity
Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More Info
Codier
left a comment
Codier
left a comment
There was a problem hiding this comment.
Approved for source integration at 9eda1578762f4af50a81e518c1ca0e6978d9a359 based on the independent Factory Droid review, Anthropic claude-opus-5, high reasoning, and its final repair verification on this exact head. Earlier change requests are closed by the recorded repairs and final review. This does not approve publication or production enablement.
Hosted CI passed lint and 4,255 tests on each of Node 20, 22 and 24, with two skips per run. Aikido check code is green. Publication and ClawHub sync were skipped.
Di explicitly authorized Codier approval despite Codier having pushed these commits. Commit author and committer remain hulk-linus[bot]. Independent code review was performed by Factory, not by the writer. This supersedes the earlier requirement for another GitHub account to approve.
Di also explicitly chose to keep the stack and accept the workflow-policy-only red check on #657. Factory inspected the workflow permissions and default-off publication conditions. The unconditional pr-reviewer policy remains unchanged and its failed check is not represented as green.
Merge in order #657, #658, #659; verify each child's base and any changed head before merging. Recheck the publication variable and active runs at merge time. Keep publication held until the exact release artifact, supported-platform walkthroughs, renewal and rollback acceptance are complete. Historical dev evidence does not establish acceptance of these new archives.
Codier
left a comment
Codier
left a comment
There was a problem hiding this comment.
Approved for source integration at f30af237b3a015be02fb40a8b20bc78f781385f9 after independent Factory Droid review using Anthropic claude-opus-5, high reasoning.
The reviewer verified that the merge carries exactly the reviewed parent integration, preserves onboarding and authentication behavior, and leaves both publication workflows identical to main. It independently passed 46 fee checks and six consent checks. Commit author, committer and push actor are hulk-linus[bot]; this approval relays the independent review as Codier.
CI passed lint and 4,274 tests with two skips on each of Node 20, 22 and 24. Aikido is green. Publication and ClawHub sync were skipped. Local validation passed 4,267 tests with nine skips and lint.
The first Node 20 attempt stalled without a final summary and was cancelled. Its retained log shows 104 of 105 test files completed, with auth-device.test.js missing. A single rerun on this unchanged commit completed successfully. The cause remains unexplained; no code, timeout or assertion was weakened to pass.
Merge #658 into main first, then verify that #659 targets main and reconcile the base if needed. Recheck the publication variable and active runs before merge. This is not approval to merge #659 into its current parent branch, publish a package or enable production.
Exact-artifact and native-platform acceptance, issuer cooldown/outage checks, v2 recovery and rollback, the release checklist and H-1 local config hardening retain their recorded status. Historical archive evidence is not acceptance of this new head. The prior approval is superseded for this head.
pr-reviewer Summary for #72e6355📝 2 findings Review completed. Please address the findings below. Findings by Severity
Review effort: 3/5 (Moderate) SummaryThis PR correctly shifts skill installation to opt-in (explicit FindingsMedium —
|
|
Reviewed both suggestions at The path guard intentionally skips optional onboarding when either path cannot be resolved. A deleted package path does not establish that this module is the entry point. The suggested The extra blank lines in skill descriptions are cosmetic and do not change their Markdown or authentication requirements. Leaving that advisory spacing unchanged in this integration update. The reviewer reran 173 onboarding, skill and route tests successfully. Hosted CI passed 4,274 tests with two skips on each of Node 20, 22 and 24, plus lint. The earlier missing-dependency test launch produced no usable test result and is excluded from these counts. |
Codier
left a comment
Codier
left a comment
There was a problem hiding this comment.
Approved for source integration at 72e635587e672036d12496542723d4a581e5a0d6, following independent Factory Droid verification with Anthropic claude-opus-5, high reasoning.
#658 is merged. This PR now targets main without conflicts. The normal merge only reconciles squash history: main70e56a7 has the exact tree of the reviewed parent537c805, and this head has the exact tree of the previously approved f30af23. Factory independently verified both identities, ancestry and the unchanged onboarding delta against main. No file changed in this update.
CI passed 4,274 tests with two skips on each of Node 20, 22 and 24, plus lint. Aikido and pr-reviewer passed. Publication and ClawHub sync were skipped. Local full validation passed 4,267 tests with nine skips and lint.
The independent reviewer also assessed the new bot comments. Its filesystem probe demonstrated that the suggested null-returning realpath helper could falsely invoke onboarding when both paths are missing; current silent skip remains intentional. Skill spacing is advisory. It passed 173 focused checks. Writer disposition and evidence. One missing-dependency reviewer launch was interrupted and produced no usable test result; registry contact cannot be excluded for that attempt. It is not counted as a pass.
Commit author, committer and push actor are hulk-linus[bot]. Codier relays this independent review. Earlier approvals apply to their recorded heads and are superseded here.
This approves the source merge, not publication or production enablement. Keep CLI_PUBLICATION_ENABLED absent or false, and recheck it and active runs before merging. Exact-artifact and native-platform acceptance, renewal cooldown/outage evidence, v2 recovery/rollback and the release checklist remain open. H-1 local config hardening remains a nonblocking follow-up. The prior-head Node20 stall remains unexplained; this head passed all Node versions on its first hosted attempt. No prior archive or live evidence has been relabeled as current acceptance.
Updates login help, onboarding, packaged skills and release guidance for browser sessions. Installing a skill requires explicit consent. Enter, EOF and unrecognized answers skip installation.
#658 is merged. This PR now targets main without conflicts at
72e635587e672036d12496542723d4a581e5a0d6. The latest commit only reconciles squash history. Its complete file tree is identical to the previously approvedf30af23; no production code, test or documentation changed.Hosted CI passed 4,274 tests with two skips on each of Node 20, 22 and 24, plus lint. Aikido and pr-reviewer passed. Full local validation passed 4,267 tests with nine skips and lint. Publication and ClawHub sync were skipped.
Factory independently verified the unchanged tree, ancestry and PR delta against main. It also assessed both new bot comments, reproduced the unsafe null-equality in the suggested path helper and passed 173 focused onboarding/skill checks. Dispositions. One interrupted missing-dependency test launch is excluded from the counts; registry contact cannot be excluded for that attempt. Codier approval on this head.
Merge and release
This source PR is ready for merge. No merge or publication was performed during this closeout. Recheck
CLI_PUBLICATION_ENABLEDand active publication runs immediately before merging; keep the variable absent or false. #658's post-merge CI passed and skipped publication and ClawHub sync.Source approval does not authorize distribution. Exact-archive native platform acceptance, real renewal cooldown and outage checks, v2 recovery/rollback, the remaining acceptance checklist and production enablement stay open. Local config permission hardening H-1 remains an explicit nonblocking follow-up. Keep historical archives pinned to their own source commits. The prior-head Node20 stall remains unexplained; this head passed the complete hosted matrix on its first attempt.
AI execution metadata
claude-opus-5, high reasoning, sessione3542c92-362a-416a-9945-91fa5c7de492. Runtime model/provider lock verified. Separate detached checkout at the final head.hulk-linus[bot], verified through GitHub. Independent approval relayed as Codier.Earlier implementation and review evidence, tied to historical revisions
The current-head status above supersedes earlier draft/readiness statements. Earlier limitations remain unless the current closeout explicitly resolves them.
Help, installation prompts and shipped skills should guide users through browser login with the same account API permissions as an API key. This PR updates that guidance and includes the matching schema and installed-package checks.
Installation asks before making the free account check. Credential failures stay tied to the selected account. Wallet passwords, transaction signing and trade confirmations keep their existing requirements. MCP installation continues to use its own persistent integration key.
This builds on #658 at
be92d1185b4dc9c50c8c6d66bccdfd0c3fe1878b, including its bounded renewal recovery. The merge also preserves the current wallet-password skill metadata and classifies the existing bridge command.Validation at
e0cb3202e21da6c2ac7dc8d051eb5608c4e986e3: 4,217 local tests passed, nine skipped; lint passed. All 98 files in the rebuilt, unpublished archive match the committed source. The archive retains the source version and is not a published or versioned prerelease. The bounded repair also passed 164 focused tests. Installer guidance now survives a missing or broken credential resolver and invalid HOME without breaking installation. Historical artifact records are identified separately from the current 1.46.0 archive. Independent Anthropic Opus 5 repair verification found all four findings fixed, with no new findings. It passed 172 focused checks, lint and the changeset check, and independently matched the archive to source. The full suite above is writer evidence; the reviewer did not repeat it.The coordinator separately completed deployed dev billing checks on the same external account: two browser calls and two matching API-key calls each consumed one credit, verified in the ledger. This is server-side evidence on API-503; it does not establish native credential-store acceptance or a published CLI release.
Review probe limits are recorded: the initial review sent one synthetic unauthenticated request to the production API and received 401. The repair review did not repeat it. One repair probe unintentionally fetched an npm package in a temporary HOME; its files were removed. Neither used live credentials or OS credential storage. Process guards did not provide complete child-process network isolation.
Draft hold remains. Native-platform acceptance and the API508 prerelease plan are incomplete. The stacked branch does not run the main-only CI matrix. No main merge or npm publication is authorized by this update.
AI execution metadata
claude-opus-5, high, same independent sessionbcf2b940-948e-41e9-9d2d-1f2f16f0c89c. Usage: 72 uncached input, 3,892,408 cache-read input, 149,720 cache-created input, 42,528 output including 18,567 thinking; 4,084,728 total across those categories. Invocation totals are separate; no lifecycle sum is claimed.claude-haiku-4-5-20251001, 1437 input, 0 cache read, 0 cache created, 17 output. It was not the primary reviewer.claude-opus-5, high, same independent sessionbcf2b940-948e-41e9-9d2d-1f2f16f0c89c. Usage: 30 uncached input, 2,678,937 cache-read input, 35,607 cache-created input, 21,313 output including 5,738 thinking; 2,735,887 total across those categories. Invocation totals are separate; no lifecycle sum is claimed.hulk-linus[bot]. Reviewer identity: Codier. No release or merge performed.Historical description and execution evidence, superseded by the current validation above
Browser login should let users run the same account API workflows as pasting an API key. This PR makes installation guidance, help, schema and every shipped skill follow that contract. It removes the research-only browser restriction, including the mandatory-key guidance for smart alerts, agent, portfolio, web and beta commands.
Plain
nansen loginalways requests fresh approval. The newnansen:apisession renews automatically during authorized work; existing OAuth/MCPnansen:readgrants keep their existing semantics.NANSEN_API_KEYstill takes precedence. Invalid or rejected credentials never switch accounts or fall back to wallet payment. Installation remains offline until the user explicitly accepts the free account check.API permission parity does not grant wallet signing authority. Trading keeps its confirmation, signing, sanctions and geographic checks. Hosted simulation uses the selected credential only at the matching trusted Nansen API origin. Anonymous hosted-simulation 401/403 responses retain warn-and-proceed behavior; selected key/session denials block. Arbitrary RPCs receive no browser credentials. MCP installation still needs a separate persistent integration key; it does not export a browser token.
Tests compare API-key and session transport for every inventoried research command, smart-alert CRUD, public agent streaming, portfolio, web and beta operations. They also check refusal/no-fallback behavior, classify every shipped skill, and run the actual installed npm executable. Full tests: 3582 passed, 9 skipped. Lint and changeset checks pass.
This draft depends on PR658 at
77efe0e. The rebuilt tarball is unpublished. Root’s final offline cross-stack and producer proofs passed. Live billing/revocation evidence, native/platform acceptance and a tested compatible recovery release remain gates. Account and non-account admission remain separately controlled. Main-only CI does not run on this stacked target; its absence is not a pass.Source, validation and release limits
Head
93fd21beb71632b10560f1988d3513298beee8b9; exact parent77efe0e4612eb1d5e2f357a4b753981519f7d98e. Normal merge parents9cc3a269 and77efe0e; no rebase/force push/main merge. Parent ref/object verified before merge. Production modules changed since9cc: src/swap-simulation.js and src/auth-credentials.js, byte-identical to parent. Onboarding runtime, parser/help/schema, all33non-MPP skills, complete skill classification and installed-bin tests are unchanged.Local full suite:3582passed/9skipped across93files,163.10s, two workers. Focused onboarding/permission/home/guard checks:198passed in6files. Separate actual verifySwapOutcome matrix:6passed,16unselected, covering anonymous/API-key/session × HTTP401/403, credential headers, no payment, no secrets and no anonymous custody. Lint, changeset validator/status and diff checks pass. An initial npm run changeset:check invocation failed because that script does not exist; the actual node scripts/check-changeset.js and changeset status passed. No product code or guard was weakened.
New unpublished archive SHA256
3e5ac2783e1efc8cca4d34e24160379dd787c20e582ebffb75d491833d7d3801; integritysha512-6EBerGXpcIT2zWDwa36yk+IAv+10Sd5Mc+32YoRXuA13OpWc+5vTmMz4En/4MSe0iUXWqoInwQ65omSfe6N2cA==. Existing source version1.45.0 is not a published release. At final clean head the installed node_modules/.bin/nansen --help passed and90installed source/skill/doc/script files matched the checkout. Optional native dependencies omitted; macOS only, no native custody/Windows acceptance. Prior9cc archivef0ac73bf and956 archive724711ff retain their original heads; no relabel. All31historical evidence records unchanged; new parent report separately hashed, all23acceptance gates and candidate/recovery fields unchanged/null.cli659-a-1 is repaired through the exact parent. A's API1967 implementation-pending observation is historical: root reports committed1d7bcc9efad02277554f563837450023b2c2a7f5 with CI6511passed/4skipped and clean A. Final offline cross-stack proof PASSED at CLI93fd21beb71632b10560f1988d3513298beee8b9/API1d7bcc9efad02277554f563837450023b2c2a7f5. Root final-integration-evidence.md and joint-parity-result.json record16passing cases through fresh CLI processes and actual API middleware, with independently verified ES256 DPoP signatures and tampering rejection. Final producer52618e420dab99ec47b1dea9f1d9104b14cfa7c9/API1d7 proof in api1967-producer-52618e4.xml passed: original/rotatedA200→401, replacementsB200→200, and signature/issuer/audience/expiry/not-before negatives rejected. In-memory stores, synthetic issuer/profile/credit/gateway fixtures; no deployed durability, propagation or live financial acceptance. The first failed joint fixture result remains retained; its configuration mask was corrected without repository changes. Original signed revocation proof binds issuer3ed027914e8834bc2f0ffef6752af65c46b9938d and consumer2ad5702645c517e76a4530cdac1dc69d678e9807, not final route parity. Parent3413/9 and109focused are separate writer evidence. Main-only CI is absent on this stacked target; Aikido SKIPPED is not success.
Historical A1/A2/A3, S1-S7 and rootR1 dispositions remain: explicit invalidation gates; source-attributed evidence; realpath/stderr/install checks; optional primaryEnv and stop-on-failed-auth/no-payment-fallback guidance; normal automatic renewal; complete shipped-skill classification. Old research-only route restrictions are superseded by the user-required nansen:api parity contract. Skill prose is not machine payment enforcement; no installed OpenClaw runtime acceptance is claimed.
Missing release evidence includes deployed public-route enforcement, deployed flags/builds/gateway/headroom, controlled-account/UID/before-after ledger and public local/remote ceremony; full-session original/rotated/copied access-token invalidation with unaffected-session controls and measured propagation/durable feed/outage/restart proof; accepted cohort/latency/Windows/desktop/OS-login and published v2-compatible recovery. Recorded/pending is not observed rejection; natural expiry alone cannot satisfy logout acceptance. Native legacy-write compatibility, proxy retry availability, unbounded stranded cache files, ordinary-command orphan-family retirement, absent caller cancellation contract, external-daemon outstanding writes/physical deletion and v2 no-pre-v2-downgrade limits remain. AUTH_CONNECT_UNAVAILABLE still maps to AUTH_FAILED telemetry. MCP persistent-key integration remains separate and unmodified.
All test transports intercepted before imports with minimal-child guards and proxies removed. Only explicit npm dependency-install processes may access the registry. No live auth/API/analytics/RPC/paid/trade/cloud probe, native keyring or existing credential was used. Process guards are not OS egress isolation. The historical unauthorized synthetic production refresh POST/401 is not rerun or treated as proof of no server logs/writes. Prior reviews apply only to their named old heads; this permission change needs independent acceptance. No deployment, release, flag activation or approval is implied.
Final B at unchanged93fd21b has no findings, independently passed228focused checks, and inspected root artifacts and the unchanged archive/source proof. Posted review PRR_kwDORFUPos8AAAABOW_viA by Codier is COMMENTED, not approval. No source change, new commit, package rebuild or redundant test run in this follow-up. All six B limitations remain:
AI execution metadata
AI execution metadata
OpenAI
gpt-6-astra, medium; same original writer session01a0b8b1-00f6-7382-922c-e0b17018b3d7. No agents/model changes or self-review. Separate reset-delimited writer segments, never summed. Reasoning is included in output. Last snapshot excludes subsequent publication/readback/handoff.Historical independent-review usage remains separate, not current-head acceptance. Factory DeepSeek V4 Pro max/high autonomy session40aded61:132507input/2994802cache-read/27117output; reasoning/total unavailable. Claude Code Opus5 requested high, SDK effort not independently exposed, session87db0ef8:140input/7158529cache-read/161884cache-created/50578output including16173thinking. Auxiliary Haiku:1607input/14output; not a reviewer substitution. No writer self-review/approval.