FIPS 140-3 validated password hashing — ADR, packaging, and build pipeline - #8469
Draft
aaronlippold wants to merge 197 commits into
Draft
FIPS 140-3 validated password hashing — ADR, packaging, and build pipeline#8469aaronlippold wants to merge 197 commits into
aaronlippold wants to merge 197 commits into
Conversation
Draft ADR for replacing bcryptjs with PBKDF2 via node:crypto. Committed before adversarial review revisions so the research and industry survey are preserved. Known issues addressed in the following commit: - bcrypt fallback not gated on FIPS mode (STIG V-222571 finding) - CMVP certificate numbers incorrect - STIG mapping table defects - Admin seeder omitted as a hash call site Authored by: Aaron Lippold<lippold@gmail.com>
Five-agent adversarial review found ~30 defects. One reviewer fabricated
its citations and retracted; independent sub-agents verified the substance
against primary sources and refuted one central argument.
Renamed: "FIPS 140-3 Compliant" -> "via a FIPS 140-3 Validated Module".
FedRAMP FRR8 prohibits the term "FIPS compliant".
Design changes:
- bcrypt fallback now GATED on FIPS mode, with a defined cutover and
forced-reset endgame. The prior unconditional fallback was weaker than
both Keycloak (refuses) and GitLab (gates on FIPS mode).
- Eight call sites, not seven. The admin bootstrap seeder was omitted;
it runs on every container start, so fresh installs would provision
the administrator with a bcrypt hash.
- Compare-and-swap rehash writes. A bare save() races the existing
un-awaited updateLoginMetadata and can silently revert a password change.
- Strict algorithm allowlist. The prior "$pbkdf2-sha*$" wildcard accepted
md5 and sha1 (verified against node:crypto).
- Iterations parsed by regex only; parseInt('6e5') is 6.
- 128-character password cap (Django CVE-2013-1443; policy 140sp4857).
- PASSWORD_HASH_WRITE_ENABLED gate + durable format marker for rolling
deploys and downgrade protection.
Corrections:
- CMVP certs were both wrong. #4985 is the OpenSSL Project's module, not
Red Hat's; #4754 is libgcrypt and Historical. Correct: #4746 / #4857.
- IG 2.4.A argument withdrawn — it scopes to functions within the module
boundary and does not reach bcryptjs. The ASD STIG basis (V-222571
check text) stands on its own.
- SP 800-132 approval is storage-scoped per IG D.N; the claim now rests
on the HMAC-SHA-512 primitive executing inside the validated module.
- V-222542 never mentions FIPS validation — bcrypt already satisfies it.
Real exposure is V-222571/V-222572, both CAT II, not CAT I.
- V-222543 was claimed satisfied via Helmet; main.ts explicitly removes
upgrade-insecure-requests.
- etag uses SHA-1, not MD5 — verified from source against two reviewers.
- Measured performance inverts the risk rating: bcryptjs cost 14 is
1120ms, so this is a 7.7x improvement, not a regression. Documents the
libuv threadpool ceiling the prior draft missed.
- ENVIRONMENT_VARIABLES.md does not exist here; that filename came from
Vulcan. Docs land in docs/fips-deployment.md so air-gapped customers
can read them.
Every normative claim is marked [V] verified or [U] unverified.
Authored by: Aaron Lippold<lippold@gmail.com>
Move the heimdall-server RPM packaging into the application repository. The spec's %files section enumerates the app's build output and Version: tracks the app version, so the two must change together. Excluded from the import: - rpmbuild/ working tree — generated; Makefile:69 copies the canonical heimdall-server.spec into rpmbuild/SPECS/ at build time - man/man1/heimdall-cli-*.1 — those document heimdall-cli, which now lives at github.com/mitre/heimdall-cli Server man pages (heimdall-server.8, backend.env.5, sysconfig.5) come with the packaging; CLI man pages stay with the CLI. Authored by: Aaron Lippold<lippold@gmail.com>
The NOTICE was never lost. daac353 (#7276) consolidated it into LICENSE.md, which is the standard MITRE SAF layout — LICENSE.md carries the Apache-2.0 terms followed by the NOTICE section. vulcan does the same, which is why its NOTICE.md is an empty placeholder. Restoring a separate NOTICE.md duplicated content already present in LICENSE.md. Authored by: Aaron Lippold<lippold@gmail.com>
Authored by: Aaron Lippold<lippold@gmail.com>
libs/hdf-converters/LICENSE.md was a fragment — no copyright line, no case number, no Apache-2.0 grant text, and it referenced 'the following Notice' when the file contained no Notice. Replaced with the repository's MITRE SAF LICENSE.md, which is a strict superset: it retains the non-endorsement clause and adds the copyright, Case Number 18-3678, the Apache-2.0 grant, and the NOTICE section. Authored by: Aaron Lippold<lippold@gmail.com>
Every workspace package now declares its license, and every package that publishes to npm ships the license text. Added LICENSE.md (MITRE SAF, Apache-2.0, Case Number 18-3678) to the two published packages that shipped without one: - @mitre/heimdall-lite (apps/frontend) - inspecjs (libs/inspecjs) Both declared "license": "Apache-2.0" in package.json while carrying no license text at all — a metadata claim with nothing behind it, on packages users actually install. Added the missing "license" field to: - root (carried LICENSE.md but never declared it in package.json) - @heimdall/common - @heimdall/cypress-tests Private workspaces (heimdall-server, @heimdall/common, @heimdall/password-complexity, @heimdall/cypress-tests) intentionally do not carry their own LICENSE.md — the repository root covers them. Duplicating the file into every workspace creates copies that drift, which is exactly how libs/hdf-converters ended up with a fragment missing the copyright line, the case number, and the Apache-2.0 grant. Authored by: Aaron Lippold<lippold@gmail.com>
Published packages (@mitre/heimdall-lite, @mitre/hdf-converters, inspecjs)
now carry the metadata npm surfaces and warns about:
- keywords, homepage, bugs, author ("MITRE Corporation", matching the
convention already used by hdf-libs, mcp-config-scaffold, and mitre-slidev)
- repository.url corrected to the git+https://...git form npm normalizes to.
Writing it explicitly avoids a mismatch against the value npm stores, which
is also what provenance verification compares against (npm/cli#7978).
- publishConfig.access "public" on the two scoped packages. This was passed
as a CLI flag in push-to-npm.yml; declaring it in the manifest means a
scoped package cannot accidentally publish restricted.
Descriptions added to the two workspaces that had none (root and
@heimdall/cypress-tests).
Private workspaces deliberately do NOT get keywords/homepage/bugs/author.
Babel, Jest, and Vue all keep private workspace manifests minimal — those
fields exist for npm discovery and only drift on packages that never
publish. Descriptions are kept everywhere since they help navigation.
LICENSE.md files added earlier are not listed in "files": npm always
includes LICENSE*, README, and package.json in the tarball regardless.
Authored by: Aaron Lippold<lippold@gmail.com>
The three published packages rewrite "main" from src/index.ts to lib/index.js in prepack and restore it in postpack. This is not a stylistic choice — npm has never supported manifest field overrides in publishConfig (it accepts only config keys: registry, access, tag, provenance), npm 13 will hard-error on unknown keys, and Yarn 1 reads only access and registry. Publishing a pre-built tarball rules out any publish-time fix as well, since `npm publish <file>.tgz` takes the tarball byte-for-byte and never runs prepack. The rewrite must happen at pack time. The defect is that Yarn 1's pack has no try/finally around the lifecycle scripts, so a failed pack skips postpack entirely and strands a mutated package.json alongside an untracked package.json.orig. pack:all runs --parallel across three packages, so one failure can corrupt the tree. - gitignore package.json.orig so a failure cannot leave an untracked artifact - add a CI guard after pack:all that fails the workflow if any package.json was left modified or a .orig file remains, rather than publishing a tarball built from a corrupted tree Authored by: Aaron Lippold<lippold@gmail.com>
heimdall2 carried only CODE_OF_CONDUCT.md, LICENSE.md, README.md, and CHANGELOG. OpenSSF Scorecard's Security-Policy check looks for a SECURITY.md with reporting contacts and a disclosure timeline — for a security compliance product shipping into DoD and FedRAMP environments, publishing no vulnerability-reporting policy was a conspicuous gap. Adapted from the SAF templates and rewritten for this stack rather than find-replaced. Both documents describe the actual toolchain: Yarn workspaces with lerna, NestJS + Sequelize, Vue 2, vitest (with the note that the test runners use swc and do not typecheck, so `yarn backend build` is a separate gate), and the workspace-scoped lint scripts. SECURITY.md records deployment guidance drawn from how Heimdall actually behaves: TLS must terminate in front of it because Helmet emits headers but cannot enforce transport, API key support is disabled when API_KEY_SECRET is unset, a lost API key must be regenerated rather than recovered, and evaluation data may contain scanned-host detail. CONTRIBUTING.md documents the repository layout and the converter contribution path, which is the most common kind of outside contribution. Closes heimdall2-30c.3 Authored by: Aaron Lippold<lippold@gmail.com>
… recovery Second revision, folding in the second review round, the empirical UBI9 findings, and the now-real repository topology. Corrections to my own prior draft: - --force-fips REVERSED. RHEL's Node prints "Using options related to FIPS is not recommended, configure FIPS in openssl instead." The RHEL model is host FIPS -> OpenSSL -> Node inherits. This deletes the launcher preflight, the start:fips script, and the systemd crash-loop concern entirely, and makes the startup assertion the only real check. - The IG block quote is 2.4.C, not 2.4.A, and the ellipsis had dropped "and/or AES XTS". 2.4.C also turns out to SUPPORT the design: it says the module service "may still be considered approved" when a caller uses PBKDF outside storage applications, making this a documentation obligation rather than a design defect. - V-222571 is now quoted in full. The prior draft quoted 172 of 1,238 characters and called it verbatim, omitting the risk-acceptance path and the NSS clause -- which makes PASSWORD_HASH_ALGORITHM=sha256 itself a finding on NSS. - silent:true does not exist on sequelize.query(); it is a Model.update option. Corrected to the single Model.update form. - #4746 sunsets 2026-07-30; only #4857 is current. - The §6/§9 iteration-floor contradiction is resolved: floors and caps apply to hashing only, never verification. - The §7 regression AC was unsatisfiable — updateLoginMetadata changes lastLogin/loginCount/updatedAt on every login by design. Narrowed to passwordChangedAt and forcePasswordChange. New scope: - Credential recovery. Heimdall has no self-service reset and admins cannot edit their own account without their password, so enabling FIPS before cutover would lock out a single-admin deployment. heimdall-cli reset-password is the remedy — but it writes bcrypt, so adding PBKDF2 to it is a BLOCKING cross-repo dependency, not a follow-up. - Phase 2 now invalidates credentials rather than only setting forcePasswordChange, which converted nothing. - Configurable password complexity, using the env var names heimdall-cli already reads. The CLI and the app silently disagree today about what a valid password is. - Repository boundary documented now that it is real: packaging/rpm in-tree, heimdall-cli at github.com/mitre/heimdall-cli, bound by libs/password-hash-vectors with a formatVersion stamp. Findings folded in: the eighth call site (the admin seeder, which runs on every container start), the getFips seam, the 256-character external-auth placeholder that the length cap would have broken, compare-and-swap writes, the write gate, requiresReset as an enumeration oracle, and the app-side version check replacing an RPM %pre guard that cannot fire on downgrade. Unverified citations resolved: IA-7 dropped (it governs authenticating to a module, not password verification), V-16793 dropped (retired in 2016), zeroization dropped (IG 9.6.A exempts hashed passwords), SI-6 and IR 8547 qualified, CMVP MM §7.9 promoted and it supports us. Authored by: Aaron Lippold<lippold@gmail.com>
The import in 35d47de left four references pointing at saf-packaging's directory layout, where heimdall-server/, heimdall-cli/, and scripts/ were siblings. None of them resolve from packaging/rpm/ in this repo, so the RPM could not be built from its new home. sources: no longer downloads a tarball from GitHub. The packaging now lives inside heimdall2, so the source is two directories up — scripts/fetch-source.sh is not ported. Two thirds of that script was a dispatcher for vulcan and saf-cli, which is dead weight here, and dropping the download keeps airgapped builds working without a mirror. RPMs are built from tagged points in time, so `sources` archives the tag matching the spec Version (v$(VERSION)) and fails with an actionable message when that tag is not present locally. DEV=1 archives HEAD instead, clearly marked as not a release build. heimdall-cli is a separate repository now, not a sibling directory, so it is cloned at a pinned ref rather than referenced by path. HEIMDALL_CLI_REF is the contract carrier — it records exactly which CLI a given RPM shipped, and release builds must override it with a tag. Man pages are generated from the same pinned checkout as the binary, so they cannot drift from the commands they document. Fixes a real bug while here: CLI_COMMIT was `git rev-parse HEAD` evaluated in the packaging repo, so the binary's --version output reported a heimdall2 commit as the heimdall-cli commit. It now resolves from the CLI checkout. scripts/setup-build-deps.sh is ported (7 heimdall references, 0 references to any other package — it was mis-filed in saf-packaging's shared scripts/). copr-submit.sh and sign-rpms.sh stay there; both are genuinely cross-SAF distribution tooling. Adds a guard so a developer on macOS gets "rpmspec is required" rather than "tag v not found". NOT yet verified by an actual build — that requires a RHEL-family host and is the gate before saf-packaging's copies are removed (heimdall2-30c.5). Authored by: Aaron Lippold<lippold@gmail.com>
NAME and VERSION came from `rpmspec`, which only exists on RHEL-family hosts.
Every version-dependent target was therefore unusable on a developer laptop,
and failed with a misleading message ("tag v not found") rather than an honest
one, because VERSION silently resolved to an empty string.
Version now resolves with POSIX sed, which works everywhere:
- The repository VERSION file is canonical. It is what the release process and
release-drafter already work from.
- The spec's own Version: field is read separately and compared against it by
a new check-version target that every version-dependent target depends on.
The comparison is the point. A spec that drifts from the repository version is
not hypothetical — the copy on feat/rpm-build sat at 2.12.6 while the shipped
one tracked to 2.13.1, and nothing caught it. Now a mismatch fails the build
with both values printed.
check-upstream no longer shells out to rpmspec either; it reuses the resolved
VERSION. rpmspec is still required to actually build an RPM, which is correct —
it is just no longer required to read a version number.
Authored by: Aaron Lippold<lippold@gmail.com>
heimdall2 has never built an RPM in CI, and release v2.13.1 ships zero downloadable assets. This adds both. Structure adapted from redis/memtier_benchmark's release-rpm.yml, which is the closest production analogue: distro containers matrixed over EL version and architecture, an install smoke test, and release upload gated on the event. One deliberate divergence. memtier builds an SRPM once and installs it into each arch job so both come from identical sources. That does not work here: Source15 is a pre-built architecture-specific heimdall-cli binary, so an x86_64 SRPM cannot build an aarch64 RPM. Each arch does a full native build instead. Sources stay identical by construction — every leg archives the same tag and clones heimdall-cli at the same HEIMDALL_CLI_REF. Choices worth recording: - Native ubuntu-24.04-arm runners rather than QEMU. Free for public repos, and roughly 5-10x faster for a Node + Go build; QEMU routinely times out on compiles this size. The Go CLI is built natively, not cross-compiled. - One workflow, not two. PRs touching packaging/ prove the RPM still builds and installs; a published release additionally uploads. The thing that ships is the thing CI exercised. - CI builds with DEV=1 (archives HEAD) because on a feature branch the tag for an in-progress version does not exist yet. Releases build from the tag. The flag makes that explicit rather than silently producing an RPM that claims to be a release. - The smoke test installs into a clean container with no build dependencies present, so a missing Requires: fails in CI rather than on a customer host. It also asserts heimdall-cli is installed and reports its provenance, which is what proves HEIMDALL_CLI_REF threaded through correctly. - actions/attest-build-provenance rather than GPG signing. smallstep/cli ships .sigstore.json alongside each RPM; this is the same idea and keeps a release key out of Actions secrets. Whether MITRE also wants detached GPG signatures is a decision for the security team, not a default. - make deps is the same script a human runs, so CI and local builds cannot drift. Not yet exercised — the workflow has never run. First PR touching packaging/ will be the real test. Authored by: Aaron Lippold<lippold@gmail.com>
The build pipeline is only half the story — where RPMs are built, signed, and hosted is the other half, and it was undocumented. COPR project mitresaf/saf (ID 249476) created with epel-8/epel-9 chroots on x86_64 and aarch64. As an open-source project this gives real mock chroots on native multi-architecture builders at no cost, which is an authentic EL build environment rather than the approximation a container-on-Ubuntu CI job provides. Caddy — which this RPM already Recommends — ships the same way. Three verified facts shape the design: - enable_net has defaulted to false since June 2022 and %build runs yarn install, so every COPR build fails without it. Confirmed set. - COPR keeps one build per package indefinitely and deletes the rest after 14 days, with all content removed 180 days after a chroot goes EOL. It is a build farm, not archival storage — GitHub Releases is the durable home. - COPR signs with its own per-project key, generated on first successful build. That last point resolves an inconsistency worth recording: saf.repo and heimdall-server.repo disagree about which GPG key users verify against, and both reference a project namespace (@mitre/saf) that never existed. Correcting them is blocked until the first build publishes a key. Also documented: EPEL proper is not viable — not because of bundled node_modules, which Fedora made the default in F34, but because Koji builds are network-isolated and %build runs yarn install. And the air-gapped bundle deserves more investment than the online repo, since DoD sites mirror internally regardless. RKE2 is the closest analogue and worth following. Authored by: Aaron Lippold<lippold@gmail.com>
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 35573710 | Triggered | Generic Database Assignment | 35d47de | packaging/rpm/INSTALL.md | View secret |
| 36202370 | Triggered | Generic Password | 3dd3db9 | libs/password-hash-vectors/scripts/generate-vectors.ts | View secret |
| 36202372 | Triggered | Generic Password | 3dd3db9 | libs/password-hash-vectors/scripts/generate-vectors.ts | View secret |
| 36202371 | Triggered | Generic Password | c7532b7 | apps/backend/test/demo-users-seeder.spec.ts | View secret |
| 36202370 | Triggered | Generic Password | 3dd3db9 | libs/password-hash-vectors/src/vectors.ts | View secret |
| 22339997 | Triggered | PostgreSQL Credentials | 54dd1df | apps/backend/test/constants/environment_test.constant.ts | View secret |
| 36202374 | Triggered | Generic Password | 3dd3db9 | libs/password-hash-vectors/scripts/generate-vectors.ts | View secret |
| 36202367 | Triggered | Generic Password | 3dd3db9 | libs/password-hash-vectors/src/vectors.ts | View secret |
| 36202369 | Triggered | Generic Password | 3a09a58 | apps/backend/seeders/demo-seed-helpers.js | View secret |
| 36202368 | Triggered | Generic Password | 3dd3db9 | libs/password-hash-vectors/src/vectors.ts | View secret |
| 36202373 | Triggered | Generic Password | ebc5a78 | apps/backend/src/authn/rehash-lifecycle.spec.ts | View secret |
| 36202374 | Triggered | Generic Password | b6d6465 | apps/backend/src/crypto/password.spec.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| # may not have Node.js 22 on all EL8/EL9 minor versions and distro variants. | ||
| if ! rpm -q nodesource-release >/dev/null 2>&1; then | ||
| echo " Installing NodeSource repo for Node.js 22..." | ||
| curl -fsSL https://rpm.nodesource.com/setup_22.x | ${SUDO} bash - |
| arch_suffix="$(uname -m | sed 's/x86_64/amd64/;s/aarch64/arm64/')" | ||
| go_tarball="go${GO_VERSION}.linux-${arch_suffix}.tar.gz" | ||
| echo " Installing Go ${GO_VERSION}..." | ||
| curl -fsSL "https://go.dev/dl/${go_tarball}" | ${SUDO} tar -C /usr/local -xzf - |
| fi | ||
|
|
||
| if [[ "${ENABLE_NODESOURCE}" -eq 1 ]]; then | ||
| curl -fsSL https://rpm.nodesource.com/setup_22.x | ${SUDO} bash - |
| --repo mitre/heimdall2 \ | ||
| --archive tar.gz \ | ||
| --output "${dest}" || \ | ||
| curl -fsSL -o "${dest}" "${url}" |
| --output "${dest}" || \ | ||
| curl -fsSL -o "${dest}" "${url}" | ||
| else | ||
| curl -fsSL -o "${dest}" "${url}" |
| return | ||
| fi | ||
| # GCP | ||
| if curl -sf -m 2 -H "Metadata-Flavor: Google" http://metadata.google.internal/ >/dev/null 2>&1; then |
| echo " Open: ${EXTERNAL_URL}" | ||
| echo "" | ||
| echo " TLS: handled externally (load balancer / reverse proxy)" | ||
| echo " App listening on: http://0.0.0.0:${PORT}" |
The bare rockylinux container has neither git nor make. actions/checkout needs git, and every subsequent step goes through the Makefile — including `make deps`, which is what installs the real build dependencies. That was circular: make deps installs make, but make deps needs make to run. All four matrix legs failed with 'make: command not found' (exit 127) about a minute in. Authored by: Aaron Lippold<lippold@gmail.com>
Contributor
|
Review: ADR-006 accuracy check — approving, with corrections to fold in I did an independent verification pass on the ADR's 1. Wrong module version string, and the SSP guidance built on it (§15 / Certificates) — the one that matters. The ADR states the UBI9 provider self-identifies as 2. bcryptjs does call 3. heimdall-cli does not read the 4. Keycloak row is mis-attributed (Context table). The "will not be able to login / ask users to reset" quotes are real but describe argon2 (Keycloak 25+ default), not bcrypt. The refuse-and-reset pattern the ADR leans on is genuine; the algorithm is wrong. 5. Mattermost row is stale. As of Mattermost v11 it defaults to PBKDF2-HMAC-SHA256 @ 600k with a Minor: OWASP's current PBKDF2-SHA512 figure is 220,000 (not 210,000), and the sheet now explicitly recommends "600,000 or more" for FIPS-140 contexts — strengthens the parameter choice. ASD V6R4's revision date is 2025-09-09 (not 2025-10-01); V-222542 currently lists only CCI-004062. Cert #4746 went Historical on 2026-07-30, consistent with the ADR's warning. rpm#1877 documents rpm 4.17/openSUSE, not RHEL 9/rpm 4.16 (behavior claim fine, attribution loose). The COPR project also has s390x chroots and uses the Pulp backend, where retention is "5 most recent successful builds," not the 14-day rule — GitHub-Releases-as-archive still follows. The 140sp4857 policy requires "a portion of the salt" ≥128 bits from the DRBG — slightly weaker than the ADR's phrasing; the design clears it either way. Structural, non-blocking: the recurring "the prior draft claimed X" backreferences are confusing for anyone who never read the prior draft — the corrections history should move to an appendix so the document states its current position directly. And the RPM/COPR/distribution material (§14, parts of §12) is separable from the FIPS hashing argument; nothing in the STIG calculus depends on packaging, and splitting it would make both halves easier to review. Finally, "carded separately" for the JWT/V-222570 finding should be verified as real — the referenced epic isn't resolvable from this repo, and this ADR is currently the only written record of that finding. |
…cedents Independent verification pass by @wdower on PR #8469. The core argument chain held; these are the corrections. The one that matters — the module version was wrong, and it is not fixable by pinning. The ADR cited 3.0.7-cda111b5812c30d4 from inspecting the running UBI9 container. Certificate #4857 validates 3.0.7-395c1a240fbfffd8; the observed string appears in no CMVP record. This is structural, not an inspection error: Red Hat validated one openssl build and has shipped security errata since, so any current UBI image carries a newer, non-validated maintenance build of the same module, essentially always. Pinning the literally-validated build would forgo every CVE fix since validation — which contradicts this ADR's own security-over-compliance tiebreaker. §15 now prescribes documentation instead: cite the certificate and its validated version, disclose the deployed build as a maintenance build, self-affirm the operational environment under CMVP MM §7.9, and cite the FedRAMP crypto policy already referenced for FRR8, which directs prioritizing patching over a frozen binary. Other corrections: - bcryptjs DOES call node:crypto (randomBytes for salt). Narrowed to "its hash computation is pure JavaScript" — the conclusion is unchanged, but the earlier phrasing failed this document's own precision standard. - heimdall-cli does NOT read the PASSWORD_* variables. Only the retired Python CLI did; the Go rewrite hardcodes its rules, as does libs/password-complexity. The §9 "contract" was aspirational and is now stated as such, with teaching the CLI those vars moved to §14's blocking cross-repo list. - Keycloak's refuse-and-reset quotes describe argon2, not bcrypt. Pattern genuine, algorithm corrected. - Mattermost is no longer a cautionary example. As of v11 it defaults to PBKDF2-HMAC-SHA256 @600k behind a requirefips build tag, making it a supporting precedent. - OWASP's PBKDF2-SHA512 floor is 220,000, and its guidance now says "600,000 or more" for FIPS-140 contexts — which supports the chosen parameter rather than merely permitting it. - COPR mitresaf/saf uses the Pulp backend: retention is the 5 most recent successful builds per package, not the 14-day rule. - ASD V6R4 revision date, V-222542's CCI list, cert #4746 now Historical, rpm#1877 attribution, and the "portion of the salt" phrasing in 140sp4857. Two blocking cross-repo items found while verifying: - cmd/gen-manpages did not survive heimdall-cli's extraction, while the spec's %files claims %{_mandir}/man1/heimdall-cli*.1* and the CLI gitignores man/man1/ as generated. The pages are neither committed nor generatable, so the RPM cannot build. The man: target now fails with an explicit message. - Teaching the Go CLI the PASSWORD_* variables. Also: the ADR twice said the JWT findings were "carded separately" when no card existed. Review caught it; now tracked as heimdall2-0bi. SonarCloud findings on the new workflow: third-party action pinned to a commit SHA (it holds contents: write, and a tag can be repointed), and read permissions moved from workflow level to job level. Authored by: Aaron Lippold<lippold@gmail.com>
… admin surface Amendments matching the carded epic (heimdall2-e25, 28 children): - §9: PASSWORD_KDF_CONCURRENCY env var (default 2) - §11: limiter design settled — zero-dep semaphore in password.ts, bounded queue, UV_THREADPOOL_SIZE=8 in Dockerfile/cmd.sh/systemd - §12: marker plants on first PBKDF2 write; write gate derived — forced on for fresh installs, default off on upgrade - §13: runtime dependency audit carded (e25.3), FIPS-host trip shared with the §15 spike - §17: admin surface concretized against existing code — derived passwordHashScheme (admin list only), bulk force-change endpoint, bulk legacy-key deletion, Migration tab, /health + /health/details shapes Authored by: Aaron Lippold<lippold@gmail.com>
The test tier containers cannot provide: kernel FIPS mode, SELinux enforcing, fapolicyd, real systemd. Serves ADR-006's empirical cards (provider activation spike, runtime dependency audit) and iterative RPM build/install testing now that local VMs are unavailable. - FIPS-on before first reachable session: cloud-init runs fips-mode-setup --enable + reboot (supported method on the pinned RHEL 9.4 AMI; 9.5+ deprecates it for switching, noted inline) - Access via SSM Session Manager over 443 — the MITRE network filters the SSH protocol at the edge (TCP connects, banner exchange killed; verified against EC2 and github.com). Minimal IAM role, SSM core policy only; SSM agent installed pre-reboot via user-data - Two lifecycle profiles: ephemeral (apply/destroy) and persistent dev box (stop between sessions) - t3.medium default; resize on evidence Verified live: instance reaches fips_enabled=1 + provision marker via SSM polling through install -> FIPS enable -> reboot -> re-register. Authored by: Aaron Lippold<lippold@gmail.com>
| key_name = var.key_name | ||
| subnet_id = var.subnet_id | ||
| vpc_security_group_ids = [var.security_group_id] | ||
| associate_public_ip_address = true |
…cked elevation Authored by: Aaron Lippold<lippold@gmail.com>
Authored by: Aaron Lippold<lippold@gmail.com>
…timeouts Authored by: Aaron Lippold<lippold@gmail.com>
…low 3% CPU Nine 5-minute periods of basic (free) monitoring; builds and test runs burn CPU so a job can never be stopped mid-run. treat_missing_data notBreaching so a stopped instance does not flap the alarm. 0 disables. Authored by: Aaron Lippold<lippold@gmail.com>
Resolves the instance by Name tag (rebuild-proof, same pattern as the ssh alias). wake waits for SSM registration — the actual ssh-ready signal — not just instance-running. Full cycle verified live: the idle alarm auto-stopped the box on its own at the 45-min mark, wake brought it back FIPS-on with a new IP the alias resolved untouched, pause stopped it again. Authored by: Aaron Lippold<lippold@gmail.com>
…he ssh alias Default renamed heimdall-fips-dev -> heimdall-fips (tag now matches the alias). Per-user boxes: export TF_VAR_name + TF_VAR_key_name, separate state dir. README warns that renaming an existing box REPLACES it (IAM profile name derives from name; instance-profile change forces replacement) — plan before approving against a box carrying work. Verified on the replacement box: fresh build FIPS-on, alias resolves the new tag, tmux present from user-data, alarm re-bound under the new name. Authored by: Aaron Lippold<lippold@gmail.com>
The update-frequency pattern wrote (?:.\d{0,10})? with the dot unescaped, so any character could stand in for the decimal point and a value like 1x5d parsed its number as 1x5. It now matches a literal point; advanceDate's existing cases still pass, and the ambiguity that made the pattern look unsafe goes with it.
The checklist comment matcher anchors to the start of a section. The separator splits immediately before every label, so a label is always first — searching further in only rescanned, position by position, the sections that have none.
libs/common's declared entry point re-exports its interfaces rather than being an empty file. The frontend interceptor declares the AxiosError it is handed, which surfaced that config.url is optional: a request without one resolves to the page's own origin, exactly as passing undefined did before. apps/frontend declares the webpack its vue.config requires, pinned to the version already resolved so the lockfile and the installed tree are unchanged.
cleanObjectValues says arrays are never dropped instead of testing a length below zero, which never held. Whether an empty array SHOULD be dropped changes emitted ASFF and its fixtures, so that is heimdall2-puf.
Authored by: Aaron Lippold<lippold@gmail.com>
security/detect-unsafe-regex is safe-regex's star-height heuristic, which cannot tell an ambiguous pattern from a merely nested one. regexp/no-super-linear-backtracking and no-super-linear-move are enabled, model actual backtracking, and are what found and fixed the real cases on this branch — the DATABASE_URL tail and the attestation frequency. They report the patterns still flagged by the heuristic clean, so keeping both means acting on the cruder signal. import-x/no-named-as-default is off for the two application entry points: helmet and vue-cookies are CommonJS packages whose default export is the middleware or plugin itself, and whose interop also exposes it under its own name. Authored by: Aaron Lippold<lippold@gmail.com>
Three specs placed their assertions inside the if that narrowed the value being asserted on. If a conversion ever stopped producing the expected shape, the branch simply would not run and the test would pass having asserted nothing at all — the same vacuous-pass problem, reached by a different route. Each now throws when the shape is wrong, so a broken fixture fails as a fixture problem and the assertion below it always runs. The guards sit above the fixture reads they do not need. Authored by: Aaron Lippold<lippold@gmail.com>
…promise Creating an evaluation resolved its group work through .then()/.catch() and never awaited it. In the group path the attach promise was discarded outright; in the user path forEach discarded one promise per group. Either way the response could report success before the evaluation had joined anything, and a failure could only surface as an unhandled rejection. Both paths now await, and the group path's try/catch keeps turning those failures into the same BadRequestException it always did. findAndCountAll stops seeding two variables it immediately overwrote in both branches and picks its service call with a conditional, so the shared assignment and the shared return each appear once. The spec's array guards throw TypeError, matching what they check. This commit also carries the parked nmq changes that live in these same two files — they are the same edit surface and were not separable without index surgery. The full backend suite passes with them (368/368). Authored by: Aaron Lippold<lippold@gmail.com>
yarn lint:ci exits 0 across all 709 files, so the step no longer needs to report without failing. Removing the line — not the step — is what its own comment asked for once the backlog reached zero. The journey was 58,124 problems to zero: the configuration repair landed earlier, and this branch closed the remaining backlog. Authored by: Aaron Lippold<lippold@gmail.com>
The AC review found both of these fixed but unpinned, so neither had a failing test to precede it. Each test here is mutation-proven: reverting the fix makes it fail, and the failure reproduces the original defect. attestations: while the separator inside the number was written unescaped it matched any character, so '1,5d' parsed its number as '1,5' — which moment ignores — and the attestation date silently never advanced. Reverting the escape fails the test with 'expected 1662758942000 to be greater than 1662758942000', which is exactly that: the date not moving. A second case keeps a genuine decimal working. ExportJson: the old check compared the last SIX characters against the five-character '.json', so it never matched and every export gained a second extension. Reverting it fails with "expected 'results.json.json' to be 'results.json'". My first attempt at the attestations test asserted isValid(), which the old regex also satisfied — it passed against the very bug it claimed to pin. The mutation check is what caught that. Authored by: Aaron Lippold<lippold@gmail.com>
Round 2 of the AC review applied round 1's standard to the rest of the disclosure list and found four more fixes with nothing pinning them. Each test below is mutation-proven — reverting the fix fails it, and the failure reproduces the original defect.
reverse-asff: the mapper walked controls in reverse via .reverse(), reordering the caller's own HDF. The fixtures could not see it because they compare only the mapper's output; the new assertion compares the INPUT before and after. Reverting fails with the caller's 243 control ids in reversed order.
parseCsv: threw the parser's raw error array, so callers got a value with no message or stack. Reverting fails with 'expected [ { type: FieldMismatch, ... } ] to be an instance of Error'.
google.strategy: callbackURL was built inside a template literal followed by || 'disabled', which can never fire, so an unset EXTERNAL_URL produced a bare path instead of the placeholder its sibling options use. Reverting fails with "expected '/authn/google/callback' to be 'disabled'".
async_util: pins that the read yields the file's text. This one is characterization rather than mutation-proven — the branch the review flagged no longer exists, having been superseded when the FileReader was replaced by Blob#text.
Not tested, and why: jwt.strategy's secretOrKeyProvider was de-asynced with the promise voided at the boundary. done() is still called identically on both paths, so no observable behavior changed; the only available assertion would read passport's private _secretOrKeyProvider, which tests implementation rather than requirement.
Authored by: Aaron Lippold<lippold@gmail.com>
…onfig The document was written 2026-06-25 against a 211-line config and had gone substantially wrong against the 814-line one it now describes. It is cited by eslint.config.mjs itself, which references its auto-fix hazard numbers. Corrected claims: the ~405 remaining hdf-converters errors are now zero; the lint:ci escape is already removed and CI is blocking; engines fields are set on every workspace package; consistent-boolean-name is off, not an enforced error; filename-case is kebabCase with scoped unions, not a four-case union; restrict-template-expressions is not configured in the file at all; and the perfectionist rows described disabled overrides when the preset is deliberately never extended. Auto-fix hazard #6 was missing entirely even though the config cites it by number - no-useless-else destroys continue statements, which caused a stack overflow in inspecjs. Added, with three further hazards found since. New material: the preset-curation principle, the eight rule-pair collisions and how each was resolved, the six inline disables with their impossibility rationale, Prettier adoption, and a closing section directing readers to eslint --print-config rather than grepping the config. Authored by: Aaron Lippold<lippold@gmail.com>
The phase table marked only yvx.1 as done while yvx.7, yvx.8 and yvx.9 are also closed on the board. Section 5.3 states that the Depends-on column mirrors the board's own dependencies. That became false when the content cards were wired to the repo-wide Prettier reformat: Prettier formats Markdown, so a page written before that commit is simply rewritten by it, costing a second review of the same lines. The column lists intra-epic order only, so the cross-epic edge is recorded beneath the table instead. Authored by: Aaron Lippold<lippold@gmail.com>
Section 5 listed three files. Sections 10 and 12 later added four more to the same module, and their filenames appeared nowhere in the document: fips.ts, hash-write-decision.ts, hash-write-gate.service.ts and hash-migration-marker.model.ts, plus the migration that creates the marker table. Records why SUPPORTED_HASH_MARKER_VERSION is a write-epoch integer rather than a package version - package versions are unreliable here (root is 0.0.0, backend and frontend skew) and semver strings compare wrongly as text. That reasoning existed only in a stored memory. The interface declarations in the code block are now type declarations, matching the code and the consistent-type-definitions rule that forces it, with a comment so it is not changed back. Three exported functions are now documented: hashPasswordWithSalt, configureKdfLimiter and kdfLimiterState. No decision content changed. Verified unchanged and correct: the injectable getFips seam via a namespace import, and all three rollout mechanisms in section 12 including the refusal to start on a newer credential write epoch. Authored by: Aaron Lippold<lippold@gmail.com>
One branch carries four workstreams by deliberate decision: FIPS password hashing, RPM packaging, the VitePress documentation site, and the repo-wide ESLint cleanup. The lint work is here because FIPS was around 70 percent done and the docs platform largely built, so the foundation was cleaned in the same PR - recorded in the lint-first-foundation memory as a 2026-06-17 decision and reaffirmed 2026-08-13. The plan states that scope, the verified gate state, the dependency-wired execution order, the remaining effort by stream, and the items that are known and deliberately not blockers. It also carries the rule that prompted writing it down: this PR's purpose is recorded here and must not be re-derived from commit statistics. On 2026-08-14 that inference was made from commit-message keyword counts, written into the recovery files as fact, and used to propose splitting the branch. It was wrong, and the decision had been recorded in two places the whole time. Authored by: Aaron Lippold<lippold@gmail.com>
The Cypress to Playwright port was largely done in 2025-10 and is not in this repository's history, so it was rediscovered from scratch. Records the clone path, branch and commit, what is there, the one real gap (cy.register was never ported), and that it rides a pnpm branch. Authored by: Aaron Lippold<lippold@gmail.com>
The ESLint autofix in 3bdd1f1 alphabetized GroupsController's methods, which moved @get(':id') above @get('/my'). Nest registers routes in declaration order, so every GET /groups/my resolved to findById('my') and Postgres rejected the literal string as a bigint: invalid input syntax for type bigint: "my" That broke GUI login outright. Authentication itself never failed — the token was issued and committed to the store — but the login handler awaits a bootstrap chain that ends in /groups/my, and it has no catch, so the rejection propagated and the router push to / on the following line never ran. Users were left on the login page with an internal server error. Fixed by moving findForUser above findById, with a comment recording that declaration order is semantic here and must not be sorted by member name. The perfectionist plugin is already fully disabled, so the sorter cannot re-break it. Pinned by a new route-resolution suite that boots the real Nest application and issues real HTTP requests — calling the handler directly succeeds regardless of declaration order and so cannot detect route shadowing. A second test asserts GET /groups/:id still resolves to findById for a real numeric id, guarding against over-correction. RED: expected 500 to be 200. GREEN: 21/21. Backend: 30 files / 374 tests pass, tsc --noEmit clean, repo-wide lint:ci exit 0. Repo-wide scan: 0 shadowed routes across 11 controllers (was 1). Live: GET /groups/my returns 200 on the running dev server, and the GUI login flow now enters the app. Card: heimdall2-8han.3 — AC-VERIFY PASS 10/10, gate heimdall2-6b4p resolved. Authored by: Aaron Lippold<lippold@gmail.com>
AC-verify round 3 on the lint epic found three behavior changes that had no
discriminating test, so reverting any of them would have been silent. Each is
now pinned by a test proven to fail against the reverted code.
groups/Users.vue had no test at all, so the component harness is new here.
1. onUpdateGroupUserRole: an index assignment became a guarded splice. Vue 2
cannot observe arr[i] = x, so the promotion was silently non-reactive.
Asserting on vm.currentUsers would have passed against the bug — the old
write mutates the array exactly as splice does — so the test asserts on
the RENDERED row text instead. The same fix removed a stray '-1' own
property created when indexOf missed.
2. deleteUserConfirm: a blind index read became a guarded .at(), fixing a
crash on undefined.groupRole when no user was selected.
3. cyclonedx credits: a template-literal wrap rendered a missing individual
as the literal string "undefined". No fixture covered it — verified, not
assumed: vex.json's only credited vulnerability has individuals, and the
golden output contains no "undefined". The test builds the missing case
from the real document.
evaluations.controller create() now awaits the group attach on both paths.
Existing tests pass identically with the awaits removed, because a discarded
promise is indistinguishable from an awaited one until it rejects — so the new
tests induce the rejection with mockRejectedValue.
Also fixed here, as a disclosed you-find-it-you-fix-it fold-in with the card's
Files contract widened first: the displayedHeaders computed pushed into its own
dependency, appending another Actions column on every toggle of editable. The
trigger was measured rather than reasoned about — repeated reads do not grow the
list ([5,5,5,5,5]) because the computed caches, but toggling the prop does
([5,5,6,6,7]). The first RED test passed and would have dismissed a real defect.
Frontend 13 files / 56 tests, backend 30 / 374, hdf 164 passed / 168 (4
environmental, named and re-run individually to confirm 4 named = 4 reported).
Live: Playwright confirmed the promoted role renders and the table has exactly
five columns; the create path returned 201 with the attach confirmed from both
sides.
Card: heimdall2-8han.1 — AC-VERIFY PASS 11/11, gate heimdall2-sszi resolved.
Authored by: Aaron Lippold<lippold@gmail.com>
Generated with mitre/repo-minder's templates/disa.j2 and its own default values
- the tool MITRE uses to standardize 240+ SAF repositories - rather than by
hand-editing legal text. Fixes carried by the current template:
- dead link: iase.disa.mil -> cyber.mil/stigs/downloads. DISA moved STIG
downloads years ago and the old host no longer resolves.
- "copyright copyright/ digital rights legend" -> "copyright/digital rights
legend"
- "U. S. Government" -> "U.S. Government"
- adds the markdown structure: License, Redistribution Terms, Notice,
Third-Party Content
- adds the AI-assisted development disclosure
The Notice is unchanged in substance: the MITRE grant, the U.S. Government
contract rights paragraph and the contact block were already present, in an
older plain-text rendering.
Authored by: Aaron Lippold<lippold@gmail.com>
The ESLint sweep on this branch shipped two user-visible regressions with the same root cause. 3bdd1f1 (route shadowing) was fixed as 8han.3. 14c13a0 turned three fire-and-forget calls into an awaited chain, so a failing GET /groups/my blocks login for every entry path. Verified branch-local: git merge-base --is-ancestor 14c13a0 origin/master returns not-on-master. Path A ruled: repair it in this PR, the same standard applied to 8han.3. The fix goes at the choke point GetUserInfo:261-262 rather than the call sites — local, LDAP, all five OAuth providers and page reload funnel through it, so per-component catch blocks would cover one path of four. Five claims from the six-agent review are corrected on measurement at 3090a5f. Each was structurally plausible; none had been checked against the code, which is the same failure the review itself caught in the first draft. - CheckForServer does not need memoizing. server.ts:163 early-returns on !this.loading; loading initializes true (:66) and is only ever committed false (the finally at :189), so the body already runs once per page load. - The dead 401 gate is not merely an empty-string default. origin is derived from error.config?.url, which is relative for Heimdall's own API calls and resolves to location.origin, while externalUrl is a backend-configured absolute URL. The two sides describe different things. - The proposed groups.error field has no precedent and no consumer. Its cited model, store/evaluations.ts:79-92, is the SET_LOADING/finally loading pattern and carries no error field; no data store in the repo has one. The only error state is snackbar.ts:25, the interceptor channel Decision 5 already routes through. Phase 1 keeps the precedented half (SET_LOADING(true) on entry, so a refetch stops rendering "loaded, empty"); Phase 2 owns the error channel, where vue-query supplies it natively. - requiresAdmin does not read the JWT claim — router.ts:99 reads the fetched profile. Switching it is dropped: Phase 1 keeps the profile commit awaited, so the ordering race it would dissolve cannot occur, and the claim is stale until expiry while the profile is refetched per login and reload. - Post-login redirect preservation is a new feature, not a repair. No redirect mechanism exists (router.ts:90-97 discards the destination; router.ts:77 is an unrelated catch-all), so the specified open-redirect validation guards a parameter nothing reads. Flagged pending a scope ruling rather than carded. Phase 1 is carded under epic heimdall2-86f6 as .1 (choke point, P0), .2 (401 session termination, P1) and .3 (dead LoginGithub, P2), .2 and .3 blocked on .1. Authored by: Aaron Lippold<lippold@gmail.com>
14c13a0 turned three fire-and-forget calls into an awaited chain, so a rejecting GET /groups/my propagated out of GetUserInfo and blocked login. Branch-local: git merge-base --is-ancestor 14c13a0 origin/master returns not-on-master, and on master both calls are unawaited. The fix goes at the choke point rather than the call sites. Local, LDAP, all five OAuth providers and page reload all funnel through GetUserInfo, so a per-component catch would have covered one path of four. - store/server.ts: the two secondary fetches are no longer awaited. They carry .catch(absorbAlreadyReportedFailure) rather than a bare void, preserving what 14c13a0 was legitimately cleaning up. The handler is named and documented because main.ts:48 already snackbars every failed request, so reporting again here would double-report. - store/groups.ts: Promise.allSettled instead of Promise.all, so one list failing no longer discards the other, plus SET_LOADING(true) on entry so a refetch stops rendering "loaded, empty". The flag still flips false only after both settle, which is the other defect 14c13a0 fixed. Measured, not assumed: only two of the four entry paths were ever broken. CheckForServer already tolerated the failure because its own catch swallows the rejection, which is why F5 recovered. Its test is a pinning test, and so is the requiresAdmin one — router.ts is deliberately unmodified, since the profile commit stays awaited and the JWT claim would be staler than the fetched value. New spec LoginDataLoading.spec.ts, 9 tests, describe.sequential because vitest.config.mts sets sequence.concurrent and the suite shares one Vuex store. Mutation-proved 6/6 CAUGHT. Neither half of the fix alone reproduces the regression, so the combined mutation is what pins the user-visible navigation test; a mutation surviving is also what exposed that nothing covered the un-awaiting until a test failing /users/user-find-all was added. Live tested: /groups/my forced to 500 via Playwright route interception, login completed, app reached. Authored by: Aaron Lippold<lippold@gmail.com>
heimdall2 had exactly one seeder, which creates admin@heimdall.local with a random password printed once. There was no dev/test user seed, so the only working accounts in any developer's database were whatever someone had registered by hand. That cost a live debugging session on 2026-08-15: the credentials for testing the app existed only because they had been created through the signup form hours earlier. Mirrors mitre/vulcan's convention — db/seeds.rb, db/seeds/data/00_users.rb and lib/seed_helpers.rb: a two-concern split, email-as-role naming, API users kept separate from human logins because only one session per account is allowed, one shared password from an env var, and idempotent find-or-create. The production guard is the point, not hygiene. packaging/rpm/cmd.sh runs db:seed:all on every container start, so an unguarded demo seeder would create known-credential accounts in production. The guard therefore lives in the seeder, and its test asserts that bulkInsert does not happen rather than that the guard code is present. The default password is 1qaz2wsx!QAZ@WSX rather than Vulcan's 12qwaszx!@qwaszx. Both are shift/unshift keyboard walks — the same physical keys typed once unshifted and once with shift held — but Vulcan's groups all six letters of each walk together, tripping a validator heimdall has and Vulcan does not: no four consecutive characters of the same class. Walking by column instead puts a digit at the head of each group, so no class runs past three, and the typeability the pattern exists for is preserved. The spec asserts this against the real policy module, because a seeded password the application would reject is a broken seed. Hashing is PBKDF2 unconditionally, through the same compiled pure function the administrator seeder uses. It deliberately does not replicate that seeder's ADR-006 §12 write-gate branch and does not plant the §12 marker: verifyPassword dispatches on the hash prefix so these rows are readable whatever the gate is set to, the gate's bcrypt fallback exists for a rolling-deploy window that cannot arise in dev or test, bcrypt generation is itself a finding under FIPS mode, and marker planting belongs to the administrator seeder alone. Mutation-proved 5/5, including one gap the mutations found: flipping forcePasswordChange to true was detected by nothing, and would have put every demo login on a change-password screen. Authored by: Aaron Lippold<lippold@gmail.com>
heimdall has two different columns named role: Users.role is app-wide (admin|user) and GroupUsers.role is scoped to one group (owner|member). Nothing seeded an account that could exercise the second, which is where the interesting authorization lives — GET /groups/my returning 500 is what started this whole line of work, and no seeded account could reproduce it. Mirrors mitre/vulcan's db/seeds/data/05_memberships.rb: a separate ordered file that assigns memberships to the EXISTING demo users rather than minting group-specific accounts. Groups.id is autoincrement and Groups.name is unique, so the seeder inserts and then re-reads by name rather than assuming an id. Every NOT NULL column is supplied explicitly instead of leaning on a default — including GroupUsers.role, whose default is 'member': relying on it would silently make every member a member and never write 'owner'. down() removes memberships before the group, because the FK is ON DELETE SET NULL and dropping the group first would orphan the rows rather than remove them. Also moves demo-seed-helpers.js out of seeders/ and adds a guard that makes the reason permanent. sequelize-cli loads EVERY file in the seeders directory and calls up() on it, so a support module parked there is not inert: db:seed:all died with "Could not find migration method: up" and exit 1, and cmd.sh runs that command under set -e before starting the app. A container would not have booted. The new seeders-directory-guard spec reads the real directory with sequelize-cli's own pattern and asserts every file it would load exports up and down, plus asserts the scan is non-empty so it cannot pass vacuously. seed-support/ is added to eslint.config.mjs's sequelize-scripts-untyped block, the same treatment migrations/ and seeders/ already get: without it the type-aware project service fatals and the file is silently not linted at all. Mutation-proved 6/6, with sked.1's 5 re-run because this touches files those proofs depend on. Verified live: db:seed:all exits 0, and GET /groups/my returns the seeded group with both role concepts distinct. Authored by: Aaron Lippold<lippold@gmail.com>
TenableController carried no guard, and no global guard exists. Any unauthenticated caller could POST an arbitrary host_url with credentials to /api/tenable/login, and the handler returned the upstream response body — an outbound-request primitive with a readable answer, reachable without any Heimdall account. The catch-all proxy was equally exposed. Adds the class-level @UseGuards(JwtAuthGuard) that every other authenticated controller already uses. Class level rather than per route: @ALL('*splat') means any future route is reachable the moment it is declared, and a per-route list would silently miss it. A mutation narrowing the guard to @post('login') is caught by the proxy test, so that requirement is enforced rather than merely documented. The spec registers the real JwtAuthGuard and the real JwtStrategy with stubbed config and user lookups, so the no-token path exercises the actual passport chain instead of a stub, and signs a genuine token to prove the guard still admits authenticated callers. It installs express-session because without it the proxy handler throws before answering, and the assertion that separates a guard rejection from a handler rejection would pass for the wrong reason. Verified live at both layers: unauthenticated requests to both routes answer 401 with the guard's body rather than the handler's, and an authenticated browser request carries its Authorization header through to the handler. Also ignores .playwright-mcp/, which the browser tooling writes into the repository unprompted. Authored by: Aaron Lippold<lippold@gmail.com>
POST /api/tenable/login took host_url from the request body and fetched it, returning the upstream response to the caller. heimdall2-86f6.5 closed that to anonymous callers; any authenticated user could still aim the server at any host. The destination name must now appear on a configured allowlist before any outbound request is attempted. The allowlist is TENABLE_HOST_URL plus an optional TENABLE_ADDITIONAL_HOST_URLS, so single-host deployments need no new configuration. Comparison is on WHATWG URL origins, never on substrings: origin drops a default port, lowercases the host and discards the path, so the https://host and https://host:443 forms the frontend produces compare equal, while tenable.example.com.attacker.test does not match tenable.example.com. Only http and https may become allowlist entries. An empty allowlist refuses everything rather than allowing anything, and no rejection echoes the requested host back. The frontend reported these refusals as a browser Content Security Policy violation. Its server-mode 400 branch recognised one backend code and sent every other coded rejection to the CSP explanation, so an operator was told to fix a policy that was never involved. A 400 carrying a backend body now shows that body's message; CSP reporting on the no-response path is unchanged. THIS IS ONE OF THREE CONTROLS AND DOES NOT FIX SSRF. A permitted name still resolves to whatever DNS returns at request time, and a permitted host can still answer 302 and move the request elsewhere. Those are heimdall2-86f6.13 and heimdall2-86f6.12. Authored by: Aaron Lippold<lippold@gmail.com>
The host allowlist decides where a request may be SENT. It cannot decide where the RESPONSE sends it next: axios follows up to 21 redirects by default, so an allowlisted host answering `302 Location: http://169.254.169.254/...` moved the request somewhere the allowlist had never approved. Both outbound paths now set maxRedirects: 0 — the login probe in the controller and the proxy's own axios instance in the service, which are two separate configurations. axios settles a 3xx as an error once redirects are disabled, because its default validateStatus accepts 2xx only. That arrives as an ordinary AxiosError carrying the 3xx response, and the controller's existing switch would have sent it to the default branch, which reports `status: error.response?.status` — re-emitting the upstream's 302 as this API's own status code. A refused redirect is therefore classified explicitly, before every other case, as 502 UPSTREAM_REDIRECT_REFUSED. The message does not name the redirect target, which the upstream controls. Tested against two real local servers rather than by inspecting the axios config: a redirector that answers 302 and a target that counts hits. Asserting that maxRedirects appears in a config object would pass whether or not the behaviour held. The target records zero hits on both paths, and each call site has its own assertion and its own mutation. THIS IS THE SECOND OF THREE CONTROLS AND DOES NOT FIX SSRF. A permitted name can still resolve into blocked address space; that is heimdall2-86f6.13. Authored by: Aaron Lippold<lippold@gmail.com>
The PR carried four declared workstreams. The security work that grew out of this branch's own lint regressions was a fifth, documented only in card notes, so a reviewer had no way to know it was in scope or how to judge it. The plan doc and the charter card now split that stream into the two halves a reviewer needs to treat differently: - Self-inflicted here and fixed here: 3bdd1f1 alphabetized GroupsController so @get(':id') swallowed @get('/my') and GUI login broke, and 14c13a0 coupled login to unrelated data. Neither is an ancestor of origin/master. - Pre-existing on master and found by the audit those breaks triggered: the Tenable proxy accepted a caller-supplied host, fetched it, and returned the response. Shipped in a23b7db (#7032) and live today; the commands to confirm that are in both documents. ADR-009 records the design that until now existed only in card notes: allowlist over host-pinning, origin comparison rather than string matching, refusal on an empty allowlist, no echo of the rejected host, redirects disabled at both call sites with a refused redirect classified rather than re-emitted as our own status code, and why the resolved-address check applies to request-supplied hosts only. It also records the rejected alternatives, including the boolean decision flag that could not narrow under this repo's compiler settings. The ADR states plainly that no single control closes SSRF and that the work is not complete until heimdall2-86f6.13 lands. Authored by: Aaron Lippold<lippold@gmail.com>
The host allowlist validates a NAME. The socket connects to whatever that name resolves to, so an attacker able to point a permitted name at an internal address defeats the allowlist without ever violating it — and the metadata service at 169.254.169.254 is one resolution away. Both outbound paths now use an agent that validates inside the connection's own DNS lookup. Node lets a connection supply its own resolver (`lookup` on socket.connect), so the address that is checked is the address the socket uses: there is no second resolution that could disagree with the first. This is the pattern request-filtering-agent implements by overriding Agent#createConnection. Implemented here on node:net BlockList rather than by adding that dependency — the pattern is small, and the classification is standard library that parses addresses and understands CIDR, including the IPv4-mapped IPv6 form. Blocked: 0.0.0.0/8, 10/8, 127/8, 169.254/16, 172.16/12, 192.168/16, ::1, fc00::/7, fe80::/10. Hosts given as literal addresses take a separate branch, because Node performs no lookup for them at all. Both httpAgent and httpsAgent are supplied at both call sites, since axios selects between them by protocol. Default-deny, with TENABLE_ALLOW_PRIVATE_ADDRESSES for deployments whose Security Center genuinely runs on private space. That setting is the one operator-visible behaviour change here. Tested against a real hit-counting server rather than by inspecting config: a NAME resolving to loopback is refused with the target recording zero hits, and the same request succeeds with the opt-out enabled — which is what proves the filter is the thing refusing. This completes the three controls in ADR-009. None of them closes SSRF alone, and the ADR says so. Authored by: Aaron Lippold<lippold@gmail.com>
heimdall2-86f6.6 added TENABLE_ADDITIONAL_HOST_URLS and heimdall2-86f6.13 added TENABLE_ALLOW_PRIVATE_ADDRESSES, and both stopped at .env-example. The canonical environment-variables reference — the page yvx.8 established as the single source of truth — did not mention either, while documenting every other setting this branch introduces. That matters most for TENABLE_ALLOW_PRIVATE_ADDRESSES, which defaults to false and refuses private address space. An operator whose Tenable.SC is reachable only on an internal address will see UPSTREAM_ADDRESS_REFUSED after upgrading, and had no documented way to find out why. The page now carries that upgrade note explicitly, alongside which refusal code each of the three controls emits. Audited the rest rather than assuming: of the 22 environment variables this branch adds to .env-example, these two were the only ones missing from the reference. Authored by: Aaron Lippold<lippold@gmail.com>
The eslint-disable at ldap.strategy.ts:83 justified itself with "Tracked separately" while nothing tracked it. Searched every card title, description and note for prefer-at / validateOrCreateUser / "no email" / "email address": the only near matches were izw.27 (same bug class, GitLab strategy, closed, on a different branch) and izw.109 (test coverage, not the behavior decision). The comment now cites heimdall2-86f6.14, which carries the real defect: an LDAP directory returning no mail attribute sends undefined into validateOrCreateUser(email: string), which reaches CreateUserDto.email on the create branch. That card also carries an audit AC for gitlab.strategy.ts:35, which does profile.emails[0].value unguarded on this branch. Comment only. git blame shows the guarded line is Amndeep's from 2026-03-13 and byte-identical on origin/master, so no runtime behavior changes here. Authored by: Aaron Lippold<lippold@gmail.com>
|
There was a problem hiding this comment.
SonarCloud found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
| @@ -76,21 +77,19 @@ | |||
| /** Sets the local storage value to the given value, stringified */ | |||
| set(val: T): void { | |||
| const nv = JSON.stringify(val); | |||
| window.localStorage.setItem(this.storageKey, nv); | |||
| localStorage.setItem(this.storageKey, nv); | |||
| } | ||
| } catch { | ||
| const splitLines = guessOptions.data.trim().split('\n'); | ||
| // If we don't have valid json, look for known strings inside the file text | ||
| if (guessOptions.filename.toLowerCase().endsWith('.nessus')) { | ||
| return INPUT_TYPES.NESSUS; | ||
| } else if ( | ||
| guessOptions.data.match(/xmlns.*http.*\/xccdf/) || // Keys matching (hopefully) all xccdf formats | ||
| guessOptions.filename.toLowerCase().indexOf('xccdf') !== -1 | ||
| XCCDF_XMLNS_PATTERN.test(guessOptions.data) || |
| ) { | ||
| return INPUT_TYPES.XCCDF; | ||
| } else if ( | ||
| guessOptions.data.match(/<netsparker-.*generated.*>/) || | ||
| guessOptions.data.match(/<invicti-.*generated.*>/) | ||
| NETSPARKER_GENERATED_PATTERN.test(guessOptions.data) || |
| guessOptions.data.match(/<netsparker-.*generated.*>/) || | ||
| guessOptions.data.match(/<invicti-.*generated.*>/) | ||
| NETSPARKER_GENERATED_PATTERN.test(guessOptions.data) || | ||
| INVICTI_GENERATED_PATTERN.test(guessOptions.data) |
Comment on lines
+234
to
+238
| session({ | ||
| resave: false, | ||
| saveUninitialized: false, | ||
| secret: 'tenable-filter-spec-session-secret', | ||
| }), |
Comment on lines
+129
to
+133
| session({ | ||
| resave: false, | ||
| saveUninitialized: false, | ||
| secret: 'tenable-redirect-spec-session-secret', | ||
| }), |
Comment on lines
+81
to
+85
| session({ | ||
| resave: false, | ||
| saveUninitialized: false, | ||
| secret: 'tenable-spec-session-secret', | ||
| }), |
Contributor
|
This pull request has a conflict. Could you fix it @aaronlippold? |
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
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Design and groundwork for replacing
bcryptjswith PBKDF2 executed inside a FIPS 140-3 validated module, so Heimdall can be deployed on STIG-hardened and FedRAMP-scoped systems.This PR contains no application code changes. It is the ADR, the packaging move, and the build/distribution pipeline that the implementation depends on. Implementation is tracked as epic
heimdall2-e25.Why
bcryptjsis pure JavaScript. It never callsnode:cryptoor OpenSSL, so on a FIPS-enabled host it runs outside the validated module, undetected and unblocked. The OS cannot see it.The exposure is narrower than it first appears, and the ADR is careful about this:
So: two CAT II findings, not a CAT I failure. Worth fixing; not worth overstating to an assessor.
What's here
1. ADR-006 —
docs/adr-006-fips-validated-password-hashing.mdThe substance of this PR. Read it first; everything else follows from it.
Every normative claim is marked [V] verified against a primary source or [U] unverified. That discipline exists for a reason documented in the ADR header: an earlier review pass produced confident, well-formatted citations — STIG check text, CMVP guidance, FedRAMP rule IDs — that had never been read. The reviewer retracted them, and on retraction found its own central argument ran backwards.
Corrections that survived independent verification:
--force-fipsis wrong on RHEL. Red Hat's own Node prints "Using options related to FIPS is not recommended, configure FIPS in openssl instead." Verified by running the UBI9 image. This deleted three cards' worth of design.PASSWORD_HASH_ALGORITHM=sha256itself a finding on NSS.bcryptjscost 14 is 1120 ms, so this is a 7.7× improvement, not a regression. The real cost — a libuv threadpool ceiling — was undocumented.2. Repository hygiene
libs/hdf-converters/LICENSE.mdwas a fragment — no copyright line, no case number, no Apache-2.0 grant, and it referenced "the following Notice" while containing none.licensefield with no license text:@mitre/heimdall-liteandinspecjs.SECURITY.mdandCONTRIBUTING.mdadded (OpenSSF Scorecard Security-Policy gap), written for this stack rather than templated.3. RPM packaging →
packaging/rpm/Moved in from
mitre/saf-packaging. The spec's%filestracks build output andVersion:tracks the app version, so the two must change together.The import left four references pointing at saf-packaging's sibling layout; none resolved here. Repaired:
fetch-source.shnot ported — the source is two directories up now.git archiveof the tag matchingVERSION, which also keeps airgapped builds working without a mirror.heimdall-cliis a separate repo now, cloned at a pinnedHEIMDALL_CLI_REF— the contract carrier recording exactly which CLI a given RPM shipped.rpmspec(RHEL-only) to POSIXsed, with drift detection between theVERSIONfile and the spec. That drift is not hypothetical: the copy onfeat/rpm-buildsat at 2.12.6 while the shipped one tracked 2.13.1, and nothing caught it.CLI_COMMITwasgit rev-parse HEADin the packaging repo, soheimdall-cli --versionreported a heimdall2 commit as the CLI commit.4. CI —
.github/workflows/build-rpm.ymlel8/el9 × x86_64/aarch64 on native runners (no QEMU), install smoke test in a clean container, release artifacts with build provenance. Structure adapted from
redis/memtier_benchmark.heimdall2 has never built an RPM in CI, and
v2.13.1ships zero downloadable assets.Related
go.modalready declaredmodule github.com/mitre/heimdall-cli, sogo installwas broken while it lived inside saf-packaging.mitresaf/saf(ID 249476) — build farm,epel-8/9×x86_64/aarch64.Known gaps
saf.repo/heimdall-server.repoare wrongheimdall-cliwrites bcrypt[U]itemsfipsmodule.cnfneeds a real FIPS host.Decisions that aren't mine
heimdall2-30c.4— Berry vs pnpm vs npm workspaces, not a foregone conclusion.Review notes
Start with the ADR. The commits are grouped: ADR (4), repo hygiene (6), RPM packaging (3), CI (1), plus a
NOTICE.mdadd-and-revert whose revert message documents that the notice was consolidated intoLICENSE.md— worth keeping in history.Authored by: Aaron Lippoldlippold@gmail.com