Repository navigation
Rebuild security baseline: secret hygiene and registry deletion guardrail - #33
Merged
Merged
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06f400ae93
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+53
to
+54
| $placeholderPwBytes = [byte[]](0x0D, 0x0A) | ||
| $placeholderPwSha256 = "7eb70257593da06f682a3ddda54a9d260d4fc514f645237f5ca74b08f8da61a6" |
There was a problem hiding this comment.
modules/PW.txt in this commit contains the one-byte LF placeholder (0A, SHA-256 01ba…546b), but this validator accepts only CRLF (0D 0A) or the corresponding different hash. Consequently every validate-release.ps1 invocation throws at the following check, blocking CI and all release builds even with the repository's intended empty password placeholder.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
… exact content - .gitignore: never commit config.json / logs/ (runtime user data)
…ardrail - add context-menu/RegistryDeleteSafety.ps1: allowlist, critical-root refusal, post-delete verification
The uninstaller now dot-sources RegistryDeleteSafety.ps1, so the fixture that copies the
- ci.yml: run scripts/test-registry-delete-safety.ps1 alongside the other checks
cenglin123
force-pushed
the
rebuild/security-v1.3.11-20260915
branch
from
September 19, 2026 11:24
06f400a to
8cf3f8e
Compare
… uninstall - [Files]: exclude config.json / logs\\* / modules\\PW.txt from the ignoreversion wildcard
Installer: never overwrite runtime user data, and clean up on uninstall
AzureXuanVerse
pushed a commit
to AzureXuanVerse/SteganographierGUI
that referenced
this pull request
Sep 21, 2026
The branch was cut from 47c188e, before PR cenglin123#33 (security baseline: runtime secret hygiene, PW.txt placeholder enforced by exact content, registry deletion routed through an explicit allowlist) and PR cenglin123#34 (installer never overwrites runtime user data) landed on main. It was 15 ahead and 7 behind, so pushing it as-is would have produced a release branch that silently drops the registry-deletion guardrail. Four files conflicted. Resolution principle: my layout work and main's security work are both kept, neither replaces the other. - .github/workflows/ci.yml: main adds test-registry-delete-safety.ps1, my side adds the registry-write, CLI temp-archive and PowerShell 5.1 suites. Kept all of them. - installer/SteganographierGUI.iss: both sides independently added the same Excludes/onlyifdoesntexist protection. Kept one copy of the Source line and the English comments (the rest of the file is English). My [UninstallRun] and [Code] pre-clean are kept - main has no [Code] - and main's note that deletion goes through RegistryDeleteSafety.ps1 is folded into the [UninstallRun] comment. - scripts/smoke-test.ps1 and scripts/test-context-menu-path.ps1: my _internal\ and context-menu\ layout against main's flat paths. Kept my layout and added main's RegistryDeleteSafety.ps1 to both the required-file list and the fixture. The merge auto-resolved context-menu/Uninstall-ContextMenu.ps1 and scripts/validate-release.ps1 correctly: the uninstaller now dot-sources RegistryDeleteSafety.ps1 and calls Remove-RegistryKeyTolerant, and the validator carries main's exact-bytes/SHA-256 placeholder check. One merge-created break, found by main's own suite rather than by inspection: test-registry-delete-safety.ps1 asserts that every key the uninstaller deletes is allowlisted, and three keys my side restored are not - the v1.3.9-era HKCR\Directory\Background\shell\SteganographierAndHashModifier, plus the CommandStore reveal and openSteganographier entries. Without them Remove-RegistryKeyTolerant would have refused and left exactly the pre-1.3.10 orphans that restoring those names was meant to clean up. Added the three to the allowlist; all are Steganographier-owned leaf keys shaped like their neighbours. Also restored the UTF-8 BOM on smoke-test.ps1 and test-context-menu-path.ps1, which the conflict edits stripped; scripts/test-script-encoding.ps1 caught both. Verified: test-registry-delete-safety, test-context-menu-path (pwsh 7 and Windows PowerShell 5.1), test-context-menu-registry-write (5.1, 39 values over 13 hostile roots), test-cli-temp-archive, test-script-encoding (24 scripts) and validate-release all pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
First batch of the security baseline, rebuilt from the clean baseline
5c3be5fd.1. Runtime secrets are never committed, and the password placeholder is enforced by exact content
.gitignore: ignoreconfig.jsonandlogs/— runtime user data written next to the app.scripts/validate-release.ps1:modules/PW.txtmust match the known placeholder by exact bytes (0D 0A) or SHA-256; anything else fails release validation. The check is deliberately not size-based — a real password book can be exactly as short as the placeholder, so size is not an identity.2. Registry deletion goes through an explicit allowlist guardrail
context-menu/RegistryDeleteSafety.ps1:/vs\,Registry::prefix, trailing separators);Contents may have changedwarning instead of being treated as "nothing happened".context-menu/Uninstall-ContextMenu.ps1no longer callsreg.exe deletedirectly.scripts/test-registry-delete-safety.ps1: a pure/parse-only suite that never touches the registry. Every branch (already absent / deleted / failed / still present / invoker threw) is driven through injected probes, so the tests are safe to run anywhere.Verification
Parser::ParseFileon every touched scriptscripts/validate-release.ps1scripts/test-context-menu-path.ps1scripts/test-registry-delete-safety.ps1Uninstall-ContextMenu.ps1 -InstallRoot <path> -WhatIfThe placeholder check was exercised against a same-size forged file (rejected), a longer unknown file (rejected) and a zero-byte file (rejected).
The guardrail suite was mutation-tested. Emptying the critical-root list, adding a nested allowlist entry, adding an over-broad entry, and reverting the uninstaller to a bare
reg.exe deleteare each caught by a specific assertion.Notes