From 37ae493fa73db6118d707c9c7e7d1a411249026d Mon Sep 17 00:00:00 2001 From: shuaiplus <2327005759@qq.com> Date: Thu, 7 May 2026 20:29:39 +0800 Subject: [PATCH] feat: add contributing guidelines and pull request template; update schema comments and documentation --- .github/PULL_REQUEST_TEMPLATE.md | 31 +++++ .github/scripts/security.cjs | 4 +- CONTRIBUTING.md | 133 ++++++++++++++++++++++ README.md | 3 + README_EN.md | 3 + migrations/0001_init.sql | 8 +- scripts/i18n-utils.cjs | 3 + scripts/i18n-validate.cjs | 4 + shared/backup-schema.ts | 8 ++ src/handlers/accounts.ts | 5 + src/handlers/backup.ts | 4 + src/handlers/ciphers.ts | 5 + src/handlers/domains.ts | 5 + src/handlers/sync.ts | 5 + src/services/backup-archive.ts | 11 ++ src/services/backup-import.ts | 10 ++ src/services/backup-settings-crypto.ts | 10 ++ src/services/domain-rules.ts | 8 ++ src/services/storage-domain-rules-repo.ts | 6 + src/services/storage-schema.ts | 12 +- src/services/storage.ts | 4 + webapp/src/lib/i18n.ts | 7 ++ 22 files changed, 284 insertions(+), 5 deletions(-) create mode 100644 .github/PULL_REQUEST_TEMPLATE.md create mode 100644 CONTRIBUTING.md diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 0000000..e91f21c --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,31 @@ +## Summary + + + +## Change Type + +- [ ] Bug fix +- [ ] Feature +- [ ] Compatibility update +- [ ] Documentation +- [ ] Refactor + +## Cross-File Checklist + +- [ ] I read `CONTRIBUTING.md`. +- [ ] Schema changes, if any, updated both runtime schema and `migrations/0001_init.sql`. +- [ ] Persistent data changes, if any, updated backup export/import or documented why backup is not needed. +- [ ] User-facing text changes, if any, updated all locale files. +- [ ] Bitwarden client compatibility was considered for sync/API shape changes. +- [ ] No secrets, tokens, private deployment values, or real vault data are included. + +## Checks + +- [ ] `npx tsc -p tsconfig.json --noEmit` +- [ ] `npx tsc -p webapp/tsconfig.json --noEmit` +- [ ] `npm run i18n:validate` +- [ ] `npm run build` + +## Notes + + diff --git a/.github/scripts/security.cjs b/.github/scripts/security.cjs index 3c71c3b..bead320 100644 --- a/.github/scripts/security.cjs +++ b/.github/scripts/security.cjs @@ -67,7 +67,7 @@ class SecurityReport { guideStep1: '1. **开发人员**：使用上方表格中的 **位置** 列找到确切的文件和行号。', guideStep2: '2. **纠正**：遵循为每个规则提供的文档链接以提交修复。', guideStep3: '3. **可追溯性**：完整的原始 `.sarif` 数据已附加到此分支。下载并将其导入您的 IDE（例如 VS Code SARIF 查看器）进行本地分析。', - footer: '💡 *由 Antigravity AI 安全引擎生成。透明度是我们的承诺。*', + footer: '💡 *由 NodeWarden 安全工作流生成。透明度是我们的承诺。*', auditedIcon: '✅ **已审计**', noFiles: '未检索到文件。', trivyTitle: '🛡️ 容器配置安全 (Trivy)', @@ -119,7 +119,7 @@ class SecurityReport { guideStep1: '1. **Developers**: Use the **Location** column in the tables above to find the exact file and line number.', guideStep2: '2. **Remediate**: Follow the documentation links provided for each rule to submit a fix.', guideStep3: '3. **Traceability**: Full raw `.sarif` data is attached to this branch. Download and import it into your IDE (e.g., VS Code SARIF Viewer) for local analysis.', - footer: '💡 *Generated by Antigravity AI Security Engine. Transparency is our commitment.*', + footer: '💡 *Generated by the NodeWarden security workflow. Transparency is our commitment.*', auditedIcon: '✅ **Audited**', noFiles: 'No files found.', trivyTitle: '🛡️ Container Config Security (Trivy)', diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..a85c5a1 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,133 @@ +# Contributing to NodeWarden + +Thanks for taking the time to improve NodeWarden. + +NodeWarden is a Bitwarden-compatible server with a custom web vault, Cloudflare +Workers/D1 storage, attachment storage, imports/exports, and scheduled backups. +Small changes can affect official clients, backups, migrations, or locale files, +so please keep changes focused and check the related parts of the project. + +## Before Opening an Issue + +For bug reports, include enough detail for someone else to reproduce the problem: + +- The client or browser you used. +- The page, API route, or action that failed. +- Screenshots, logs, or the exact error message. +- Whether the problem happened after sync, import, export, restore, upgrade, or + a fresh deployment. + +Please do not report NodeWarden-specific problems to the official Bitwarden +team. This project is independent from Bitwarden. + +## Pull Request Guidelines + +Keep pull requests small enough to review. A good PR should explain: + +- What changed and why. +- What user-facing behavior changed. +- Which related areas were checked. +- Which commands were run before submitting. + +Avoid mixing unrelated refactors with feature or bug-fix work. If a cleanup is +needed before the real fix, mention that clearly in the PR. + +## Areas That Need Extra Care + +Some parts of the codebase are deliberately connected. When changing one of +these areas, check the related files before calling the work complete. + +### Database Changes + +Runtime schema lives in `src/services/storage-schema.ts`. The initial D1 schema +lives in `migrations/0001_init.sql`. + +If you add or change a table, column, or index: + +- Update both schema files. +- Bump `STORAGE_SCHEMA_VERSION` in `src/services/storage.ts`. +- Decide whether the data should be included in instance backup. + +### Backup And Restore + +Backup export and restore are whitelist-based. This protects old backups from +breaking when fields are removed and prevents transient or secret runtime data +from being exported by accident. + +When adding persistent data, check: + +- `src/services/backup-archive.ts` +- `src/services/backup-import.ts` +- `webapp/src/lib/api/backup.ts` + +Do not export runtime lock rows such as `backup.runner.lock.v1`. Do not import +retired sensitive fields such as `users.api_key`. + +### Secrets And Provider Settings + +Provider credentials must not be stored or exported as plain config JSON. Follow +the encrypted settings pattern in `src/services/backup-settings-crypto.ts`, or +document a replacement design before changing it. + +### Bitwarden Client Compatibility + +Official Bitwarden clients may send or expect fields that are not used directly +by the web vault. Cipher and sync changes should preserve unknown client fields +unless they are known-invalid or server-owned. + +Check these files when changing vault item shape or sync behavior: + +- `src/handlers/ciphers.ts` +- `src/handlers/sync.ts` +- `src/services/storage-cipher-repo.ts` + +### Domain Rules + +Equivalent-domain settings store both client/UI rule state and derived active +groups. Do not remove `equivalent_domains`, `custom_equivalent_domains`, or +`excluded_global_equivalent_domains` as duplicates without a migration and +compatibility plan. + +### Accounts And Passwords + +`users.master_password_hash` is for server-side login verification. It is not the +vault decryption key. Password changes, key material, `securityStamp`, and +refresh-token revocation must stay aligned. + +Password hints are reminders, not recovery secrets. They must never contain the +master password, recovery codes, API keys, or anything that directly unlocks the +vault. + +### i18n + +Locale files are complete standalone bundles. When adding or changing user-facing +text, keep every locale in sync and run the validation script. + +For new locales, update: + +- `webapp/src/lib/i18n.ts` +- `webapp/src/lib/i18n/locales/*` +- `scripts/i18n-utils.cjs` + +## Recommended Checks + +For most backend or shared changes: + +```sh +npx tsc -p tsconfig.json --noEmit +npm run build +``` + +For webapp text or locale changes: + +```sh +npm run i18n:validate +npx tsc -p webapp/tsconfig.json --noEmit +npm run build +``` + +For documentation-only changes: + +```sh +git diff --check +``` diff --git a/README.md b/README.md index f99a5f1..1015897 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,9 @@ English: README_EN.md +贡献指南：CONTRIBUTING.md | +贡献与开发说明：CONTRIBUTING.md + > **免责声明** > 本项目仅供学习与交流使用，请定期备份你的密码库。 > 本项目与 Bitwarden 官方无关，请不要向 Bitwarden 官方反馈 NodeWarden 的问题。 diff --git a/README_EN.md b/README_EN.md index 49b515c..73eb01d 100644 --- a/README_EN.md +++ b/README_EN.md @@ -25,6 +25,9 @@ 中文说明：README.md +Contributing guide: CONTRIBUTING.md | +Contributing and development notes: CONTRIBUTING.md + > **Disclaimer** > > This project is for learning and discussion purposes only. Please back up your vault regularly. diff --git a/migrations/0001_init.sql b/migrations/0001_init.sql index 0197694..1e25231 100644 --- a/migrations/0001_init.sql +++ b/migrations/0001_init.sql @@ -1,8 +1,14 @@ PRAGMA foreign_keys = ON; -- IMPORTANT: --- Keep this file in sync with src/services/storage-schema.ts (SCHEMA_STATEMENTS). +-- This is the initial D1 schema. Keep it in sync with +-- src/services/storage-schema.ts (SCHEMA_STATEMENTS). -- Any new table/column/index must be added to both places together. +-- +-- WHEN CHANGING THIS: +-- - Also bump STORAGE_SCHEMA_VERSION in src/services/storage.ts. +-- - If the new table stores persistent data, update backup export/import. +-- - Keep src/services/storage-schema.ts idempotent for existing installs. CREATE TABLE IF NOT EXISTS config ( key TEXT PRIMARY KEY, diff --git a/scripts/i18n-utils.cjs b/scripts/i18n-utils.cjs index 93817df..028b377 100644 --- a/scripts/i18n-utils.cjs +++ b/scripts/i18n-utils.cjs @@ -2,6 +2,9 @@ const fs = require('fs'); const path = require('path'); const vm = require('vm'); +// CONTRACT: +// This list is the script-side locale source of truth. Keep it in sync with +// webapp/src/lib/i18n.ts whenever adding/removing a locale. const localeDir = path.join(__dirname, '..', 'webapp', 'src', 'lib', 'i18n', 'locales'); const localeFiles = [ diff --git a/scripts/i18n-validate.cjs b/scripts/i18n-validate.cjs index 22e8866..914ed7e 100644 --- a/scripts/i18n-validate.cjs +++ b/scripts/i18n-validate.cjs @@ -1,5 +1,9 @@ const { localeFiles, readLocale } = require('./i18n-utils.cjs'); +// CONTRACT: +// This is the authoritative locale consistency gate. It checks key parity, +// placeholder parity, and accidental mostly-English locale files. Run after any +// user-facing text or locale-file change. const locales = Object.fromEntries( localeFiles.map(([locale, fileName, variableName]) => [locale, readLocale(fileName, variableName)]) ); diff --git a/shared/backup-schema.ts b/shared/backup-schema.ts index 06d1dfc..3ccb519 100644 --- a/shared/backup-schema.ts +++ b/shared/backup-schema.ts @@ -1,3 +1,11 @@ +// Shared backup settings types used by both Worker and webapp code. +// +// CONTRACT: +// Keep this file serializable and provider-neutral. Runtime state is operational +// metadata; destination fields can contain provider credentials and must be +// encrypted by src/services/backup-settings-crypto.ts before storage/export. +// User-facing provider names should use canonical values here. Legacy aliases +// belong in backend normalization, not in this shared type. export const BACKUP_DEFAULT_TIMEZONE = 'UTC'; export const BACKUP_DEFAULT_RETENTION_COUNT = 30; export const BACKUP_DEFAULT_S3_REGION = 'auto'; diff --git a/src/handlers/accounts.ts b/src/handlers/accounts.ts index 03aee60..5d9f8d9 100644 --- a/src/handlers/accounts.ts +++ b/src/handlers/accounts.ts @@ -9,6 +9,11 @@ import { isTotpEnabled, verifyTotpToken } from '../utils/totp'; import { createRecoveryCode, recoveryCodeEquals } from '../utils/recovery-code'; import { buildAccountKeys } from '../utils/user-decryption'; +// CONTRACT: +// users.master_password_hash is server-side login verification only. It does +// not decrypt vault data. Password changes must keep encrypted user key material, +// securityStamp, refresh-token invalidation, and client compatibility together. +// Password hints are non-secret reminders; never treat them as recovery secrets. function looksLikeEncString(value: string): boolean { if (!value) return false; const firstDot = value.indexOf('.'); diff --git a/src/handlers/backup.ts b/src/handlers/backup.ts index bb75a61..a221120 100644 --- a/src/handlers/backup.ts +++ b/src/handlers/backup.ts @@ -85,6 +85,10 @@ const BACKUP_RUNNER_LOCK_KEY = 'backup.runner.lock.v1'; const BACKUP_RUNNER_LEASE_MS = 10 * 60 * 1000; const BACKUP_RUNNER_HEARTBEAT_MS = 30 * 1000; +// CONTRACT: +// The runner lock is a config-row lease, not a queue. It only prevents two +// backup/restore jobs from overlapping. Manual runs return conflict when the +// lease is held; scheduled runs skip quietly. Never export this row in backups. interface BackupRunnerLease { token: string; touch: () => Promise; diff --git a/src/handlers/ciphers.ts b/src/handlers/ciphers.ts index 2802634..8c2afa3 100644 --- a/src/handlers/ciphers.ts +++ b/src/handlers/ciphers.ts @@ -18,6 +18,11 @@ import { deleteAllAttachmentsForCipher, deleteAllAttachmentsForCiphers } from '. import { parsePagination, encodeContinuationToken } from '../utils/pagination'; import { readActingDeviceIdentifier } from '../utils/device'; +// CONTRACT: +// Cipher JSON is the highest-risk Bitwarden compatibility surface. Preserve +// unknown/future client fields by default, then override only server-owned +// fields. Any change to cipher response shape must be checked against /api/sync, +// attachments, import/export, and current official clients. function normalizeOptionalId(value: unknown): string | null { if (value == null) return null; const normalized = String(value).trim(); diff --git a/src/handlers/domains.ts b/src/handlers/domains.ts index 78a92d1..0e7c173 100644 --- a/src/handlers/domains.ts +++ b/src/handlers/domains.ts @@ -9,6 +9,11 @@ import { } from '../services/domain-rules'; import { errorResponse, jsonResponse } from '../utils/response'; +// CONTRACT: +// This route accepts both camelCase and PascalCase Bitwarden-compatible payloads. +// It stores custom rules, then derives equivalentDomains from the non-excluded +// custom rules. Keep this behavior aligned with backup import/export and +// src/services/storage-domain-rules-repo.ts. function firstPresent(payload: Record, keys: string[]): unknown { for (const key of keys) { if (Object.prototype.hasOwnProperty.call(payload, key)) return payload[key]; diff --git a/src/handlers/sync.ts b/src/handlers/sync.ts index 5dc6f9a..b5b98c8 100644 --- a/src/handlers/sync.ts +++ b/src/handlers/sync.ts @@ -11,6 +11,11 @@ import { } from '../utils/user-decryption'; import { buildDomainsResponse } from '../services/domain-rules'; +// CONTRACT: +// /api/sync reuses cipherToResponse() as the single cipher response shaper. +// Filtering invalid cipher responses here protects clients from stored rows that +// would otherwise make official apps fail after an HTTP 200 sync. +// Keep this aligned with src/handlers/ciphers.ts when adding new vault fields. function buildSyncCacheRequest(request: Request, userId: string, revisionDate: string, excludeDomains: boolean, excludeSends: boolean): Request { const url = new URL(request.url); const cacheUrl = new URL( diff --git a/src/services/backup-archive.ts b/src/services/backup-archive.ts index 0091584..dc48959 100644 --- a/src/services/backup-archive.ts +++ b/src/services/backup-archive.ts @@ -8,6 +8,17 @@ import { getBlobStorageKind, } from './blob-store'; +// CONTRACT: +// This file defines the exported instance-backup archive shape. Keep it in lock +// step with src/services/backup-import.ts and webapp/src/lib/api/backup.ts. +// +// WHEN CHANGING THIS: +// - Add persistent tables to BackupPayload, export SQL, manifest tableCounts, +// and validateBackupPayloadContents(). +// - Keep secrets and transient runtime rows sanitized before writing db.json. +// - users.api_key is intentionally not exported. +// - backup.settings.v1 is exported as portable-only; the current server runtime +// envelope must not leave the instance. type SqlRow = Record; const BACKUP_FORMAT_VERSION = 1; diff --git a/src/services/backup-import.ts b/src/services/backup-import.ts index b00e696..22bd09a 100644 --- a/src/services/backup-import.ts +++ b/src/services/backup-import.ts @@ -8,6 +8,16 @@ import { validateBackupPayloadContents, } from './backup-archive'; +// CONTRACT: +// Restore is intentionally whitelist-based. Old backups may contain retired +// fields, but only the columns listed here are imported. Keep this file in sync +// with src/services/backup-archive.ts whenever backup contents change. +// +// WHEN CHANGING THIS: +// - Update BackupTableName, BACKUP_TABLES, reset statements, prepared payloads, +// shadow-table count validation, insert column lists, and frontend import +// count types together. +// - Do not import users.api_key, even if an older backup contains it. type SqlRow = Record; type BackupTableName = | 'config' diff --git a/src/services/backup-settings-crypto.ts b/src/services/backup-settings-crypto.ts index 02cef35..2d11e08 100644 --- a/src/services/backup-settings-crypto.ts +++ b/src/services/backup-settings-crypto.ts @@ -1,5 +1,15 @@ import type { Env, User } from '../types'; +// CONTRACT: +// Backup settings contain provider credentials. They are stored as a v2 envelope: +// - runtime: AES-GCM encrypted with a key derived from JWT_SECRET for the current +// server's scheduled backup runner. +// - portable: AES-GCM encrypted with a random DEK; that DEK is RSA-wrapped for +// active admin public keys so settings can be repaired after restore/migration. +// +// New admin-entered provider secrets, such as mail API keys, should use this +// pattern or a deliberately documented replacement. Do not store provider +// secrets as plain config JSON. const RUNTIME_SALT = 'nodewarden.backup-settings.runtime.v2'; const RUNTIME_INFO = 'runtime'; const PORTABLE_ALGORITHM = 'RSA-OAEP'; diff --git a/src/services/domain-rules.ts b/src/services/domain-rules.ts index 45934a6..a2e634f 100644 --- a/src/services/domain-rules.ts +++ b/src/services/domain-rules.ts @@ -3,6 +3,14 @@ import customGlobalDomainsRaw from '../static/global_domains.custom.json'; import type { CustomEquivalentDomain, DomainRulesResponse, GlobalEquivalentDomain } from '../types'; import { normalizeEquivalentDomain } from '../../shared/domain-normalize'; +// CONTRACT: +// Equivalent domains are a Bitwarden compatibility surface. The DB stores both +// the full custom rule list and the derived active equivalent-domain groups: +// - custom_equivalent_domains: UI/client rules with id + excluded state. +// - equivalent_domains: active groups derived from non-excluded custom rules. +// - excluded_global_equivalent_domains: disabled global rule type ids. +// Do not treat equivalent_domains and custom_equivalent_domains as accidental +// duplicates without a migration and compatibility plan. type RawGlobalDomain = Partial & { Type?: unknown; Domains?: unknown; diff --git a/src/services/storage-domain-rules-repo.ts b/src/services/storage-domain-rules-repo.ts index 50aa674..be1747a 100644 --- a/src/services/storage-domain-rules-repo.ts +++ b/src/services/storage-domain-rules-repo.ts @@ -1,6 +1,12 @@ import type { UserDomainSettings } from '../types'; import { normalizeCustomEquivalentDomains, normalizeEquivalentDomains } from './domain-rules'; +// Storage adapter for the domain_settings table. +// +// CONTRACT: +// equivalent_domains is kept as the active derived groups for compatibility and +// fallback reads. custom_equivalent_domains is the full rule list that preserves +// UI/client state. Save both together through saveUserDomainSettings(). function parseJsonArray(raw: string | null | undefined, fallback: T[]): T[] { if (!raw) return fallback; try { diff --git a/src/services/storage-schema.ts b/src/services/storage-schema.ts index 4fdafd1..da996bb 100644 --- a/src/services/storage-schema.ts +++ b/src/services/storage-schema.ts @@ -1,6 +1,14 @@ // IMPORTANT: -// Keep this schema list in sync with migrations/0001_init.sql. -// Any new table/column/index must be added to both places together. +// This is the runtime D1 schema bootstrap. Keep it in sync with +// migrations/0001_init.sql. Any new table/column/index must be added to both +// places together. +// +// WHEN CHANGING THIS: +// - Bump STORAGE_SCHEMA_VERSION in src/services/storage.ts so existing installs +// rerun these idempotent statements. +// - If the new table stores persistent data, update the backup export/import +// contract in src/services/backup-archive.ts and backup-import.ts. +// - Keep statements idempotent; D1 may execute them again on later requests. const SCHEMA_STATEMENTS: readonly string[] = [ 'CREATE TABLE IF NOT EXISTS users (' + 'id TEXT PRIMARY KEY, email TEXT NOT NULL UNIQUE, name TEXT, master_password_hint TEXT, master_password_hash TEXT NOT NULL, ' + diff --git a/src/services/storage.ts b/src/services/storage.ts index fef6a65..0a8cbde 100644 --- a/src/services/storage.ts +++ b/src/services/storage.ts @@ -112,6 +112,10 @@ import { const TWO_FACTOR_REMEMBER_TTL_MS = 30 * 24 * 60 * 60 * 1000; const STORAGE_SCHEMA_VERSION_KEY = 'schema.version'; +// IMPORTANT: +// Bump this whenever src/services/storage-schema.ts or migrations/0001_init.sql +// changes. Existing D1 installs only rerun ensureStorageSchema() when this value +// differs from config.schema.version. const STORAGE_SCHEMA_VERSION = '2026-05-05-domain-rules-v2'; // D1-backed storage. diff --git a/webapp/src/lib/i18n.ts b/webapp/src/lib/i18n.ts index 49acf26..ce79041 100644 --- a/webapp/src/lib/i18n.ts +++ b/webapp/src/lib/i18n.ts @@ -1,3 +1,10 @@ +// CONTRACT: +// Locale bundles are standalone and loaded on demand. Adding a locale requires +// updating Locale, AVAILABLE_LOCALES, browser-language detection, localeLoaders, +// scripts/i18n-utils.cjs, and the locale file itself. +// +// Do not call t() at module scope for exported arrays/constants; async init can +// otherwise leave raw txt_* keys in the rendered UI. export type Locale = | 'en' | 'zh-CN'