chore(security): complete npm security hardening (follow-up to #360)#361
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Completes the remaining non-N/A items from the npm-security-best-practices audit (follow-up to #360) by adding maintainer/contributor policy documentation and tightening npm/package handling for the Obsidian plugin, plus a hardened devcontainer setup.
Changes:
- Add maintainer npm security policies and dependency vetting guidance in
SECURITY.mdandCONTRIBUTING.md. - Add a
filesallowlist and lockfile-lint wiring (plus committed lockfile) forplugin/obsidian. - Introduce a security-hardened
.devcontainer/devcontainer.jsonto standardize a safer dev environment.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
SECURITY.md |
Adds maintainer policies for npm account security and dependency vetting workflow. |
CONTRIBUTING.md |
Documents npm dependency hygiene practices for contributors. |
plugin/obsidian/package.json |
Adds files allowlist and a preinstall lockfile-lint check; adds lockfile-lint devDependency. |
plugin/obsidian/package-lock.json |
Commits the lockfile to support reproducible installs and enable npm ci. |
.devcontainer/devcontainer.json |
Adds a devcontainer definition with tightened container privileges and Go feature. |
Files not reviewed (1)
- plugin/obsidian/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "README.md" | ||
| ], | ||
| "scripts": { | ||
| "preinstall": "npx lockfile-lint --path package-lock.json --type npm --allowed-hosts npm --validate-https --validate-integrity", |
Comment on lines
+9
to
+10
| "manifest.json", | ||
| "README.md" |
Comment on lines
+169
to
+173
| - `ignore-scripts=true` — third-party lifecycle scripts do NOT run on install | ||
| - `allow-git=none` — git URLs as deps are rejected | ||
| - `min-release-age=3` — packages newer than 3 days old are rejected | ||
|
|
||
| If you have a legitimate reason to override these for local dev (e.g. `esbuild` postinstall), use a flag for that specific command — DO NOT edit `.npmrc`: |
Comment on lines
+7
to
+9
| "--cap-add=CHOWN", | ||
| "--cap-add=SETUID", | ||
| "--cap-add=SETGID" |
4 tasks
Alan-TheGentleman
changed the base branch from
chore/npm-security-hardening
to
main
Follow-up to #360 covering remaining best practices: - Add files allowlist to engram-brain (defense in depth) - Add lockfile + lockfile-lint preinstall for plugin/obsidian - Add .devcontainer/ with hardened container runtime flags - Document 2FA and OIDC publishing policy in SECURITY.md - Document npq + Snyk vetting workflow for contributors Audit reference: https://github.com/lirantal/npm-security-best-practices/
Alan-TheGentleman
force-pushed
the
chore/npm-security-hardening-followup
branch
from
4b0135d to
a880745
Compare
npm 10.x does not support OIDC trusted publishers — that was introduced in 11.5.1. Node 24 ships with npm 11.x but the specific bundled version can lag behind, so verify and upgrade defensively before publishing. Avoids npm@latest (known to self-corrupt with promise-retry errors in CI) by pinning to a specific version. Refs #362
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- plugin/obsidian/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
plugin/obsidian/package.json:14
- Even when lifecycle scripts are enabled, running
npx lockfile-lintinpreinstallwill fetch and executelockfile-lintfrom the registry before any dependencies are installed/pinned by the lockfile. That undermines the goal of lockfile-based supply-chain hardening. Prefer a check that uses a pinned tool version (e.g. a dedicated CI step or an npm script that runs afternpm ciusing the locally installedlockfile-lint).
"scripts": {
"preinstall": "npx lockfile-lint --path package-lock.json --type npm --allowed-hosts npm --validate-https --validate-integrity",
"build": "node esbuild.config.mjs",
| "README.md" | ||
| ], | ||
| "scripts": { | ||
| "preinstall": "npx lockfile-lint --path package-lock.json --type npm --allowed-hosts npm --validate-https --validate-integrity", |
Comment on lines
+9
to
+10
| "manifest.json", | ||
| "README.md" |
| "version": "1.25" | ||
| } | ||
| }, | ||
| "postCreateCommand": "echo 'Run: cd plugin/obsidian && npm ci --ignore-scripts'" |
| If you have a legitimate reason to override these for local dev (e.g. `esbuild` postinstall), use a flag for that specific command — DO NOT edit `.npmrc`: | ||
|
|
||
| ```bash | ||
| npm install --ignore-scripts=false esbuild |
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
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.
Closes #362
Summary
Follow-up to #360. Completes coverage of all non-N/A items in the Liran Tal npm-security-best-practices audit.
Stacked on: #360. Merge that one first, then GitHub will retarget this against main.
What's in this PR
SECURITY.md+CONTRIBUTING.mdpolicyplugin/obsidian/package.jsonpreinstall + lockfilenpm ci.devcontainer/devcontainer.jsonSECURITY.mdmaintainer policySECURITY.md+CONTRIBUTING.mdpolicyfilesallowlist (defense in depth)plugin/obsidian/package.jsonFull audit coverage after this PR
Required follow-up (manual, by maintainer)
auth-and-writes2FA on the npm account that ownsgentle-engramcd plugin/obsidian && npm ci --ignore-scriptsTest plan
cd plugin/obsidian && npx lockfile-lint --path package-lock.json --type npm --allowed-hosts npm --validate-https --validate-integritysucceeds locally.devcontainer/devcontainer.jsonopens cleanly in VS Code (if anyone tests it)Audit reference: https://github.com/lirantal/npm-security-best-practices/