fix(windows,tui): DPAPI over stdin, browser opener without cmd, tag capability check #12

Merged

james.maes merged 3 commits from fix/windows-dpapi-browser-tags-20261106 into main 2026-11-06 15:32:20 +00:00

Conversation 0 Commits 3 Files Changed 8

+122 -7

james.maes commented 2026-11-06 14:46:08 +00:00

Owner

Copy link

Copy source

What and why

Upstreams the three Windows and TUI fixes that GG Forge has carried as local patches against this repository since cli-v1.0.0, so the next release contains them and Forge can stop running a patched development copy. Three commits, one per fix; each has a CHANGELOG line under Unreleased / Fixed.

1. fix(core): DPAPI keyring passes secrets to PowerShell over stdin (packages/core/src/session/keyring.ts, new packages/core/src/test/dpapi.test.ts). The secret was interpolated into the -Command string, so it sat in the process list and in process-creation auditing, and PowerShell parsing could mangle quotes, backticks and $(). The command is now fixed text: UTF-8 on both console streams, read stdin to the end, protect or unprotect, write compact JSON. Unprotect zeroes and frees the BSTR and disposes the SecureString. Any failure or non-string result is reported as "Windows credential protection failed; check PowerShell and your Windows profile", never with the arguments.
2. fix(cli): Windows browser opener (packages/cli/src/browser-login.ts, new packages/cli/src/test/browser-opener.test.ts). cmd /c start "" <url> splits at &, so qqq login --browser opened the authorization endpoint without client_id, state or redirect_uri and the loopback callback never came. The URL now travels in QQQ_BROWSER_URL to a fixed PowerShell -EncodedCommand (Start-Process -FilePath $env:QQQ_BROWSER_URL), windowsHide: true, shell from QQQ_POWERSHELL else powershell.exe.
3. fix(tui): tag feature and capability check (packages/tui/src/screens/record.tsx, packages/tui/test/enterprise.test.tsx, packages/tui/test/fake-collab.ts). Tags (t) calls getRecordFeatures first and opens the editor only with the TAGS feature and the tag capability; otherwise it flashes "Tags are not enabled for this table." or "Your role cannot tag this record.". A stale answer (screen moved to another record) is dropped.

Forge's patches 2 and 3 were reported as drifted; in fact the patch files had CRLF line endings. With them normalised, all three applied as-is (the TUI hunk at a 2-line offset), so nothing was rewritten by hand.

Type of change

• Bug fix
• New feature or command
• Breaking change (CLI grammar, output shape, exit code, or a package's exported API)
• Documentation
• Refactoring, tests or build

Packages touched

• @qrun-io/qqq-cli
• @qrun-io/qqq-core
• @qrun-io/qqq-ui-api-client (regenerated with npm run generate?)
• @qrun-io/qqq-ui-api-conformance
• packages/tui (qqq ui)

Checks

• npm ci && npm run build && npm test && npm run lint pass
• npm run test:tui passes (when packages/tui or the kernel changed)
• Tried against a server: not in this pass; the TUI test uses the fake server's /table/person/<id>/features route, and the two Windows tests run real PowerShell (DPAPI round trip, Start-Process shadowed to echo its -FilePath)
• CHANGELOG.md has an entry under the next version for user-visible changes
• README, TRY-IT.md or the package README updated for new or changed commands (no command changed)
• Commits are signed and use conventional-commit subjects (SSH-signed, DCO sign-off)

How it was tested

Windows 11, Node 24.22.0, npm 11.19.1, Bun 1.4.4, PowerShell 7.6.9 (pwsh) and Windows PowerShell 5.1.26100.

npm ci --ignore-scripts && npm run build                       # clean, all four workspaces
QQQ_POWERSHELL=pwsh npm test                                   # 449 pass, 0 fail, 1 pre-existing platform skip
QQQ_POWERSHELL=pwsh node --test packages/core/dist/test/dpapi.test.js            # 2 pass
QQQ_POWERSHELL=pwsh node --test packages/cli/dist/test/browser-opener.test.js    # 1 pass
node --test packages/cli/dist/test/browser-opener.test.js      # 1 pass (Windows PowerShell 5.1)
npm run lint                                                   # lint: ok
cd packages/tui && bun install --frozen-lockfile && bun test   # 469 pass, 0 fail (37 files); the 2 new tag tests included
cd packages/tui && npx tsc -p tsconfig.json                    # clean

Note on QQQ_POWERSHELL=pwsh: on the test PC, Windows PowerShell 5.1 cannot load Microsoft.PowerShell.Security at all (type-data clash with the PowerShell 7 module on PSModulePath), so the DPAPI round trip fails there with the unpatched main code exactly as with this branch. That is a host defect, not a regression; the redacted-failure test still passes under 5.1. The Windows CI lane runs the same tests.

Carried locally by GG Forge since cli-v1.0.0.

🤖 Generated with Claude Code

## What and why Upstreams the three Windows and TUI fixes that GG Forge has carried as local patches against this repository since `cli-v1.0.0`, so the next release contains them and Forge can stop running a patched development copy. Three commits, one per fix; each has a CHANGELOG line under Unreleased / Fixed. 1. **fix(core): DPAPI keyring passes secrets to PowerShell over stdin** (`packages/core/src/session/keyring.ts`, new `packages/core/src/test/dpapi.test.ts`). The secret was interpolated into the `-Command` string, so it sat in the process list and in process-creation auditing, and PowerShell parsing could mangle quotes, backticks and `$()`. The command is now fixed text: UTF-8 on both console streams, read stdin to the end, protect or unprotect, write compact JSON. Unprotect zeroes and frees the BSTR and disposes the SecureString. Any failure or non-string result is reported as "Windows credential protection failed; check PowerShell and your Windows profile", never with the arguments. 2. **fix(cli): Windows browser opener** (`packages/cli/src/browser-login.ts`, new `packages/cli/src/test/browser-opener.test.ts`). `cmd /c start "" <url>` splits at `&`, so `qqq login --browser` opened the authorization endpoint without `client_id`, `state` or `redirect_uri` and the loopback callback never came. The URL now travels in `QQQ_BROWSER_URL` to a fixed PowerShell `-EncodedCommand` (`Start-Process -FilePath $env:QQQ_BROWSER_URL`), `windowsHide: true`, shell from `QQQ_POWERSHELL` else `powershell.exe`. 3. **fix(tui): tag feature and capability check** (`packages/tui/src/screens/record.tsx`, `packages/tui/test/enterprise.test.tsx`, `packages/tui/test/fake-collab.ts`). Tags (`t`) calls `getRecordFeatures` first and opens the editor only with the `TAGS` feature and the `tag` capability; otherwise it flashes "Tags are not enabled for this table." or "Your role cannot tag this record.". A stale answer (screen moved to another record) is dropped. Forge's patches 2 and 3 were reported as drifted; in fact the patch files had CRLF line endings. With them normalised, all three applied as-is (the TUI hunk at a 2-line offset), so nothing was rewritten by hand. ## Type of change - [x] Bug fix - [ ] New feature or command - [ ] Breaking change (CLI grammar, output shape, exit code, or a package's exported API) - [ ] Documentation - [ ] Refactoring, tests or build ## Packages touched - [x] `@qrun-io/qqq-cli` - [x] `@qrun-io/qqq-core` - [ ] `@qrun-io/qqq-ui-api-client` (regenerated with `npm run generate`?) - [ ] `@qrun-io/qqq-ui-api-conformance` - [x] `packages/tui` (`qqq ui`) ## Checks - [x] `npm ci && npm run build && npm test && npm run lint` pass - [x] `npm run test:tui` passes (when `packages/tui` or the kernel changed) - [ ] Tried against a server: not in this pass; the TUI test uses the fake server's `/table/person/<id>/features` route, and the two Windows tests run real PowerShell (DPAPI round trip, `Start-Process` shadowed to echo its `-FilePath`) - [x] `CHANGELOG.md` has an entry under the next version for user-visible changes - [x] README, `TRY-IT.md` or the package README updated for new or changed commands (no command changed) - [x] Commits are signed and use conventional-commit subjects (SSH-signed, DCO sign-off) ## How it was tested Windows 11, Node 24.22.0, npm 11.19.1, Bun 1.4.4, PowerShell 7.6.9 (`pwsh`) and Windows PowerShell 5.1.26100. ```sh npm ci --ignore-scripts && npm run build # clean, all four workspaces QQQ_POWERSHELL=pwsh npm test # 449 pass, 0 fail, 1 pre-existing platform skip QQQ_POWERSHELL=pwsh node --test packages/core/dist/test/dpapi.test.js # 2 pass QQQ_POWERSHELL=pwsh node --test packages/cli/dist/test/browser-opener.test.js # 1 pass node --test packages/cli/dist/test/browser-opener.test.js # 1 pass (Windows PowerShell 5.1) npm run lint # lint: ok cd packages/tui && bun install --frozen-lockfile && bun test # 469 pass, 0 fail (37 files); the 2 new tag tests included cd packages/tui && npx tsc -p tsconfig.json # clean ``` Note on `QQQ_POWERSHELL=pwsh`: on the test PC, Windows PowerShell 5.1 cannot load `Microsoft.PowerShell.Security` at all (type-data clash with the PowerShell 7 module on `PSModulePath`), so the DPAPI round trip fails there with the unpatched `main` code exactly as with this branch. That is a host defect, not a regression; the redacted-failure test still passes under 5.1. The Windows CI lane runs the same tests. Carried locally by GG Forge since cli-v1.0.0. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

james.maes added 3 commits 2026-11-06 14:46:08 +00:00

fix(core): pass DPAPI secrets to PowerShell over stdin, not the command line ... a7767ba76c

The Windows keyring (DpapiFileStore) interpolated the secret into the
PowerShell -Command string with JSON.stringify. A token was therefore
visible in the process list and in process-creation auditing for the
lifetime of the call, and PowerShell's own parsing could still mangle
quotes, backticks and $() inside the quoted value.

The secret now goes over stdin: the fixed command sets UTF-8 on both
console streams, reads stdin to the end, runs the protect or unprotect
step on that value and writes the result as compact JSON, which the
caller parses. Unprotect zeroes and frees the BSTR and disposes the
SecureString after reading. Any subprocess failure, non-zero exit or
non-string result is reported as "Windows credential protection failed;
check PowerShell and your Windows profile", never with the arguments.

Tests (win32 only): a round trip of a value with fake JSON, shell
syntax, Unicode and surrounding whitespace that must not appear in the
secrets file in clear, and a corrupt blob that must fail with the
redacted message. Run with QQQ_POWERSHELL=pwsh where Windows PowerShell
5.1 cannot load Microsoft.PowerShell.Security.

Carried locally by GG Forge since cli-v1.0.0.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

fix(cli): open the browser on Windows without cmd /c start splitting the URL ... b126064144

openInBrowser used `cmd /c start "" <url>` on Windows. cmd treats `&`
as a command separator even inside an argument that was passed as one
string, so an OAuth authorization URL was cut at its first query
parameter: the browser opened the authorization endpoint without
client_id, state or redirect_uri and the loopback callback never
arrived. Only `qqq login --browser` was affected; the printed URL
worked when pasted.

The URL is now never shell text. The opener runs PowerShell
(QQQ_POWERSHELL, else powershell.exe) with a fixed -EncodedCommand,
`Start-Process -FilePath $env:QQQ_BROWSER_URL`, and the URL in that
environment variable; the child is detached, stdio ignored and
windowsHide set so no console window flashes.

Test (win32 only): stubs child_process.spawn to capture the call,
checks the command is not cmd, the URL is in the environment and
windowsHide is set, then runs the captured encoded command in a real
PowerShell with Start-Process shadowed to print its -FilePath, and
expects the full URL back, `&` and all.

Carried locally by GG Forge since cli-v1.0.0.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

fix(tui): check the record's tag feature and capability before the tag editor ... 2c3114e06b

The record screen opened the TagOverlay for any record whenever the
collaboration extension was present. On a table without the TAGS
feature, or for a role without the `tag` capability, the editor opened
and the save failed only when the server refused it.

openTags now calls getRecordFeatures for the record and opens the
overlay only when `features` contains TAGS and `capabilities.tag` is
true; otherwise it flashes "Tags are not enabled for this table." or
"Your role cannot tag this record.". The answer is ignored when the
screen has moved to another record meanwhile, and a request error is
flashed like any other.

Tests: the fake server answers /table/person/<id>/features from a
`recordTagPolicy` (enabled, allowed; reset with the rest of the
collaboration state), and enterprise.test.tsx presses t with the
feature off and with the capability denied and expects the message and
no overlay. The existing tag editor test keeps its defaults.

Carried locally by GG Forge since cli-v1.0.0.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>

james.maes merged commit 991950505d into main 2026-11-06 15:32:20 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

bug

Something isn't working

dependencies

documentation

Improvements or additions to documentation

duplicate

This issue or pull request already exists

enhancement

New feature or request

good first issue

Good for newcomers

help wanted

Extra attention is needed

invalid

This doesn't seem right

question

Further information is requested

secuurity

wontfix

This will not be worked on

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

forge-recovery (Forge Recovery) james.maes (James Maes)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: QRun-IO/qqq-cli#12