From 99a006e7a48a5c1f63fabcbb93667d975b7950ce Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Wed, 5 Aug 2026 08:37:00 -0700 Subject: [PATCH 1/3] security: publish threat model and policy (#70) --- CHANGELOG.md | 3 + MANIFEST.in | 1 + README.md | 5 ++ SECURITY.md | 72 ++++++++++++++++ docs/security-review.md | 56 ++++++++++++ docs/security-threat-model.md | 123 +++++++++++++++++++++++++++ tests/test_security_documentation.py | 64 ++++++++++++++ tests/validate.sh | 3 + 8 files changed, 327 insertions(+) create mode 100644 SECURITY.md create mode 100644 docs/security-review.md create mode 100644 docs/security-threat-model.md create mode 100644 tests/test_security_documentation.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 6be4b68..d84d020 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,9 @@ and versions are tracked in the repo-root `VERSION` file. ### Added +- Add the security policy, runtime threat model, and threat-to-control release + checklist covering secret handling, filesystem ownership, plugins, inherited + runs, concurrency, and telemetry boundaries. - Add the public API stability and deprecation policy, migration guide, and `base_cli.deprecated()` warning helper with contract-test guardrails. - Add immutable `LifecycleOptions` and `LifecycleOption` policies for enabling, diff --git a/MANIFEST.in b/MANIFEST.in index 8e07973..d722bb1 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -5,6 +5,7 @@ include CONTRIBUTING.md include LICENSE include MANIFEST.in include README.md +include SECURITY.md include VERSION include base_manifest.yaml include pyproject.toml diff --git a/README.md b/README.md index 7a65cf9..b75c4be 100644 --- a/README.md +++ b/README.md @@ -46,6 +46,11 @@ mechanism, and migration requirements are documented in [`docs/api-stability.md`](docs/api-stability.md) and [`docs/migrations.md`](docs/migrations.md). +Security reporting, runtime trust boundaries, threat assumptions, and the +release security checklist are documented in [`SECURITY.md`](SECURITY.md), +[`docs/security-threat-model.md`](docs/security-threat-model.md), and +[`docs/security-review.md`](docs/security-review.md). + Shared record renderers keep machine output stable: CSV and TSV stream one-pass iterables without headers or footers, while terminal tables account for Unicode display width and safely truncate oversized cells. See diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..f36588e --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,72 @@ +# Security policy + +`base-cli` is a library for embedding a lifecycle into Python command-line +applications. We take reports about the framework, its release artifacts, and +the security controls documented in the [runtime threat model](docs/security-threat-model.md) +seriously. + +## Reporting a vulnerability + +Please report suspected vulnerabilities privately through GitHub's +[Private Vulnerability Reporting](https://github.com/basefoundry/base-cli/security/advisories/new) +workflow. Do not open a public issue, pull request, or discussion containing +secrets, an exploitable proof of concept, or details that would enable an +unfixed attack. + +Include, when safe to share: + +- the affected base-cli version or commit and the Python/OS environment; +- a concise description of the impact and the attack preconditions; +- reproduction steps or a minimal proof of concept with secrets removed; and +- any proposed mitigation or an indication of whether exploitation is active. + +If private reporting is unavailable for a repository or account, contact the +repository maintainers through the private contact route shown on the +[repository profile](https://github.com/basefoundry/base-cli) and reference +“base-cli security report”; do not publish the details first. + +## Supported versions + +Security fixes are targeted at the latest released minor line and the current +development branch. At the time this policy was published, that means the +`0.3.x` release line and `main`. Older pre-1.0 lines are best effort only; +upgrade to the latest release before requesting a backport. A release that +changes the supported window will update this table and the changelog. + +| Version | Security support | +| --- | --- | +| `0.3.x` | Supported | +| `main` | Supported for fixes merged before the next release | +| `<0.3` | Upgrade strongly recommended; best effort only | + +The [API stability policy](docs/api-stability.md) explains the pre-1.0 +compatibility boundary. A security fix may require an emergency breaking +change when leaving a vulnerable behavior in place would expose users. + +## Response and disclosure expectations + +These are service targets rather than a guarantee: + +- acknowledge a report within **3 business days**; +- provide an initial severity and affected-version assessment within **10 + business days**; and +- provide a status update at least every **7 days** while a report is active. + +We will coordinate a fix, release notes, and (when appropriate) a GitHub +Security Advisory/CVE. We normally request a **90-day coordinated disclosure +window** from the first maintainer response, adjusted with the reporter when +the fix or downstream coordination needs more or less time. We may publish an +advisory earlier if exploitation is public or users need an urgent mitigation. + +Reporter credit is given unless anonymity is requested. Please do not include +personal data or production credentials in a report. + +## Scope and security boundaries + +The framework protects the lifecycle data it owns: argv redaction, private +runtime files, fail-closed temporary cleanup, bounded JSON contracts, and +opt-in telemetry with a safe attribute set. It does not sandbox consumer +callbacks, third-party plugins, Python dependencies, shell commands, or the +operating system. Consumers must review the [threat model](docs/security-threat-model.md) +and complete the [security review checklist](docs/security-review.md) for their +own profile, plugins, paths, history writer, and telemetry exporter. diff --git a/docs/security-review.md b/docs/security-review.md new file mode 100644 index 0000000..ffbb61e --- /dev/null +++ b/docs/security-review.md @@ -0,0 +1,56 @@ +# Security release review checklist + +Use this checklist for a base-cli release and for any change that adds a +filesystem path, input source, plugin, history field, telemetry attribute, or +concurrency boundary. Each row maps a threat to the control and the regression +tests that should be run or extended. + +## Threat-to-control map + +| Review area | Questions to answer | Control / evidence | +| --- | --- | --- | +| argv and secrets | Can a new option, argument, environment value, prompt, error, or debug line contain a secret? | Mark parameters sensitive; update the redaction plan; run `tests/test_redaction_security.py`, `tests/test_app_security_boundaries.py`, and `tests/test_invocation_parity.py`. | +| Config and serialization | Are config files regular/readable, parsed as data, schema-validated, and excluded from logs/telemetry? | Validate explicit paths and mappings; add malformed/secret cases; run config, JSON-contract, and output tests. | +| Logs and history | Does any new field cross a persistence callback or change permissions/retention? | Redact before the callback, preserve private modes, document consumer ownership; run history, logging, and run-metadata tests. | +| Filesystem and symlinks | Can a path be replaced, traversed, mounted, symlinked, or cleaned outside the owned run root? | Retain handles/identity, refuse uncertain paths, and preserve primary results; run `tests/test_cleanup_security.py` and `tests/test_adversarial_regressions.py`. | +| Permissions | Does the change create a file or directory under a custom root, on Windows, or on a network/mounted filesystem? | Verify POSIX modes and Windows ACL assumptions; update platform documentation and add a permission regression where practical. | +| Plugins and dependencies | Does an extension load earlier, discover more metadata, or gain new authority? | Keep loading explicit/lazy, preserve allowlists/disable switches, pin and audit dependencies; run `tests/test_extensions.py`, Bandit, and `pip-audit --strict`. | +| Concurrency | Can two threads/processes write, replace, retain, or prune the same artifact? | Use atomic replacement and the existing lock boundary; add a race/adversarial test; run the full concurrency suite. | +| Inherited runs | Can a child finalize, prune, or delete parent state? | Keep inherited bindings read-only/unowned; run inherited startup, metadata, and cleanup tests. | +| Telemetry | Could a new span attribute include argv, config, paths, identifiers, or secrets? | Keep the safe attribute allowlist; test broken/missing exporters and inspect provider configuration; run `tests/test_integrations.py`. | +| Release surface | Does the change alter a public export, schema, warning, or security promise? | Update `docs/api-stability.md`, `SECURITY.md`, changelog, contract tests, and migration guidance as applicable. | + +## Required release checks + +Run the repository gates appropriate to the change: + +```bash +./tests/validate.sh +python -m pytest +ruff format --check scripts examples +ruff check lib/python/base_cli scripts examples tests +python -m mypy --strict examples/typed_consumer.py +python scripts/validate_docs.py +bandit -q -r lib/python/base_cli scripts -lll -iii +pip-audit --strict +``` + +For changes involving runtime ownership, redaction, history, extensions, +telemetry, or concurrency, run the focused suites listed in the map in +addition to the full suite. CI is the final release gate; a local pass does not +override a failing cross-platform check. + +## Reviewer sign-off + +Before merging, the reviewer should be able to answer “yes” to each question: + +- [ ] The changed assets and trust boundaries are named in the threat model. +- [ ] New inputs and persistence destinations have an explicit secret and + permission decision. +- [ ] Symlink, traversal, replacement, and concurrent-operation behavior is + fail-closed or covered by a regression test. +- [ ] Plugins and telemetry remain opt-in, bounded, and consumer-auditable. +- [ ] Consumer responsibilities and residual same-account/process risks are + documented. +- [ ] Security-relevant behavior, public contracts, and release notes agree. +- [ ] Full CI, security scans, and the focused tests are green. diff --git a/docs/security-threat-model.md b/docs/security-threat-model.md new file mode 100644 index 0000000..cc7cfac --- /dev/null +++ b/docs/security-threat-model.md @@ -0,0 +1,123 @@ +# Runtime threat model + +This model describes the assets, trust boundaries, controls, and residual +risks for a `base-cli` invocation. It covers the generic framework; a consumer +must extend it for its own commands, configuration schema, plugins, network +clients, and deployment environment. + +## Security objectives and assets + +The important assets are: + +- **credentials and sensitive input:** argv, environment variables, explicit + configuration values, Click prompt values, and consumer-owned service data; +- **diagnostics and history:** logs, `run.json`, temporary files, cache entries, + and consumer history records; +- **execution integrity:** command selection, lifecycle state, cleanup + ownership, exit status, and the parent/child runtime relationship; +- **extension and dependency supply chain:** installed distributions, entry + points, optional Rich/Telemetry integrations, and Python dependencies; and +- **telemetry and machine contracts:** exported span attributes, JSON records, + command-protocol frames, and schema meanings. + +The primary security goals are to avoid accidental secret disclosure, prevent a +cleanup operation from deleting an unrelated path, preserve an accurate +command result when secondary persistence fails, and make optional integrations +unable to silently broaden the data sent or code executed by the framework. + +## Trust boundaries + +```text +Shell / OS (argv, env, cwd, identity) + | + v +Click parsing -> base-cli lifecycle -> consumer callbacks/profile + | | | + v v v +config files runtime/log/history plugins and services + | + v + telemetry exporter +``` + +The boundaries are intentionally explicit: + +1. The shell and operating system supply untrusted strings and process + authority. `base-cli` cannot distinguish a secret from an arbitrary value + unless a parameter is marked sensitive or matches its documented heuristic. +2. Click parsing and the lifecycle transform input into a `Context`; consumer + callbacks and profile policies remain application code and are not sandboxed. +3. Configuration files, project discovery, custom runtime roots, history + writers, and service factories cross from consumer policy into framework + persistence. The generic profile deliberately supplies fewer implicit + sources than a product profile. +4. Entry-point plugins cross into third-party Python code only when a consumer + creates discovery and loads a selected extension. Discovery is not a code + sandbox. +5. Telemetry crosses a process/network boundary only when the consumer opts in + and supplies a provider/exporter. The framework publishes a bounded safe + attribute set but cannot secure the exporter's endpoint or credentials. + +## Threats, controls, and residual risk + +| Threat / asset | Framework controls and tests | Residual risk and consumer action | +| --- | --- | --- | +| Secrets in argv, environment-derived values, config, or prompts leak into logs | Sensitive options/arguments, secret-name heuristics, equals/short-option handling, and redaction before history callbacks; `tests/test_redaction_security.py`, `tests/test_app_security_boundaries.py`, and `tests/test_invocation_parity.py` | A custom secret name or consumer log can still disclose data. Mark domain-specific parameters with `sensitive=True`, do not log `ctx.config`, and review custom formatters/history writers. | +| Logs, history, JSON, or run metadata expose credentials or unbounded attacker text | Redacted history boundary, bounded JSON log messages, owner-only POSIX modes, atomic metadata writes, and JSON contract tests | Consumer-owned paths and history stores may have weaker permissions. Set private ACLs, avoid copying raw logs, and treat retained diagnostics as sensitive. | +| Symlink, traversal, replacement, or mount races redirect cleanup | Exclusive runtime-leaf ownership, retained descriptors, identity checks, no-follow traversal, run-ID containment, and fail-closed cleanup; `tests/test_cleanup_security.py`, `tests/test_app_security_boundaries.py`, and adversarial regression tests | A same-account process with the same filesystem authority can race user-owned paths. Use a private cache root and avoid sharing runtime trees between mutually hostile users. | +| Insecure permissions expose runtime files | POSIX `0600`/`0700` modes; Windows uses inherited user-profile ACLs and warns when secure handle operations are unavailable | A custom Windows cache root or network filesystem may not inherit private ACLs. Consumers must provision and verify permissions. | +| Malicious or accidental config content changes execution | Explicit config paths must be readable regular files; YAML is parsed as data; generic profile has no implicit product files; consumer profiles own schema validation and precedence | The consumer still decides which files, environment variables, and values are trusted. Validate schema, reject unexpected keys, and do not treat config as a secret store. | +| A plugin executes unwanted code or changes command behavior | Lazy discovery, deterministic ordering, duplicate detection, explicit `load`/`load_all`, `allowlist`, and `disabled`; `tests/test_extensions.py` | Loaded extensions have the process's authority and are not sandboxed. Pin and review distributions, disable discovery by default where possible, and use an allowlist. | +| Concurrency corrupts logs, history, metadata, or retention state | Atomic replacement, sidecar/process locks, bounded complete-bundle retention, and concurrency/adversarial tests | The framework cannot make consumer databases or external stores transactional. Use a transactional writer and define recovery for multi-process consumers. | +| Child/inherited runs mutate or delete a parent's artifacts | Inherited runtimes do not own a new bundle, do not finalize parent metadata, and are excluded from cleanup/retention; inherited-run tests cover startup, success, and failure | Parent and child processes still share the user's filesystem authority. Consumers must validate parent provenance and avoid inheriting from untrusted paths. | +| Telemetry leaks argv, config, paths, or secrets | Telemetry is opt-in; only run ID, CLI name, environment, dry-run, outcome, exit code, and duration are attached; missing/broken exporters are no-ops; `tests/test_integrations.py` | A consumer-supplied tracer/exporter can add arbitrary attributes or send to an untrusted endpoint. Review provider configuration, use TLS/authentication, and never attach raw command data. | +| Optional integrations or dependencies broaden the attack surface | Rich, Typer, and OpenTelemetry are optional extras; import/use is lazy; quality CI runs Bandit and pip-audit | A vulnerable or malicious dependency remains a supply-chain risk. Pin/lock deployments, review updates, and install only required extras. | +| Secondary persistence failure hides the real command result or leaves false state | Transactional startup/teardown, terminal outcome finalization, best-effort history, and rollback tests | A crash can still lose diagnostics. Treat logs/history as evidence, not an authorization or billing source, and alert on persistence warnings. | + +## Secure defaults + +The generic framework defaults to: + +- no implicit product configuration files, project discovery, history writer, + plugin discovery, or telemetry; +- redaction before data reaches persistent logs or consumer history callbacks; +- owner-only runtime files/directories on POSIX and user-profile ACL inheritance + on Windows; +- fail-closed cleanup that retains uncertain paths and reports a warning; +- bounded, versioned machine contracts rather than arbitrary object + serialization; and +- best-effort secondary persistence that cannot replace the primary command + result. + +These defaults reduce accidental exposure; they are not a sandbox or encryption +boundary. + +## Non-goals + +`base-cli` does not provide: + +- encryption at rest, a secrets manager, key rotation, or secret redaction from + arbitrary consumer output; +- isolation from a same-user process, root/administrator, a malicious shell, + malicious Python package, or a loaded plugin; +- a policy for the consumer's network requests, subprocesses, project files, + or authorization model; or +- a guarantee that consumer-owned configuration, history, telemetry, or cache + paths are private when they are outside the framework's managed root. + +## Consumer responsibilities + +Before shipping a CLI built on the framework, the consumer should: + +1. inventory secret-bearing options, positional arguments, environment values, + config keys, and output fields; mark all non-obvious parameters sensitive; +2. validate configuration schemas and restrict config/cache/log/history roots + to an appropriate owner or service account; +3. pin and review plugins and optional dependencies, using discovery allowlists + or disabling discovery when extensions are not required; +4. configure telemetry exporters with TLS, authentication, retention, and an + endpoint policy, and review every custom span attribute; +5. test the command's own subprocess, network, filesystem, and authorization + behavior; and +6. run the release/security checklist in [`security-review.md`](security-review.md) + for every release and whenever a trust boundary changes. diff --git a/tests/test_security_documentation.py b/tests/test_security_documentation.py new file mode 100644 index 0000000..2f8cb1a --- /dev/null +++ b/tests/test_security_documentation.py @@ -0,0 +1,64 @@ +from __future__ import annotations + +import unittest +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] + + +class SecurityDocumentationTests(unittest.TestCase): + def test_security_policy_defines_private_reporting_support_and_disclosure(self) -> None: + policy = (ROOT / "SECURITY.md").read_text(encoding="utf-8") + + for phrase in ( + "Private Vulnerability Reporting", + "Supported versions", + "3 business days", + "90-day coordinated disclosure", + "Reporter credit", + "runtime threat model", + ): + with self.subTest(phrase=phrase): + self.assertIn(phrase, policy) + + def test_threat_model_covers_required_assets_boundaries_and_responsibilities(self) -> None: + model = (ROOT / "docs" / "security-threat-model.md").read_text(encoding="utf-8").casefold() + + for phrase in ( + "argv", + "environment", + "configuration", + "logs", + "history", + "symlink", + "permissions", + "plugins", + "concurrency", + "inherited", + "telemetry", + "non-goals", + "consumer responsibilities", + ): + with self.subTest(phrase=phrase): + self.assertIn(phrase, model) + + def test_security_review_maps_threats_to_tests_and_controls(self) -> None: + review = (ROOT / "docs" / "security-review.md").read_text(encoding="utf-8") + + for phrase in ( + "Threat-to-control map", + "tests/test_redaction_security.py", + "tests/test_cleanup_security.py", + "tests/test_extensions.py", + "tests/test_integrations.py", + "bandit", + "pip-audit --strict", + "Reviewer sign-off", + ): + with self.subTest(phrase=phrase): + self.assertIn(phrase, review) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/validate.sh b/tests/validate.sh index 9d2fac2..2ab4753 100755 --- a/tests/validate.sh +++ b/tests/validate.sh @@ -4,6 +4,7 @@ required_files=( README.md VERSION CHANGELOG.md + SECURITY.md CONTRIBUTING.md .github/pull_request_template.md .github/base-project.yml @@ -16,6 +17,8 @@ required_files=( docs/releasing.md docs/api-stability.md docs/migrations.md + docs/security-threat-model.md + docs/security-review.md MANIFEST.in scripts/validate_package_artifact.py scripts/validate_installed_package.py From 71bb05c0703fe331072570f56d4e7adfdbdfa0cb Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Wed, 5 Aug 2026 08:40:27 -0700 Subject: [PATCH 2/3] fix: allow security policy artifacts (#70) --- scripts/validate_docs.py | 7 ++++++- scripts/validate_package_artifact.py | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/validate_docs.py b/scripts/validate_docs.py index e56657c..62cd341 100644 --- a/scripts/validate_docs.py +++ b/scripts/validate_docs.py @@ -19,7 +19,12 @@ def fail(message: str) -> None: def validate_links(root: Path) -> None: - markdown_files = [root / "README.md", root / "CONTRIBUTING.md", *sorted((root / "docs").glob("*.md"))] + markdown_files = [ + root / "README.md", + root / "CONTRIBUTING.md", + root / "SECURITY.md", + *sorted((root / "docs").glob("*.md")), + ] for document in markdown_files: text = document.read_text(encoding="utf-8") for raw_target in LINK_PATTERN.findall(text): diff --git a/scripts/validate_package_artifact.py b/scripts/validate_package_artifact.py index 38f8ffd..d9d5288 100644 --- a/scripts/validate_package_artifact.py +++ b/scripts/validate_package_artifact.py @@ -26,6 +26,7 @@ "MANIFEST.in", "PKG-INFO", "README.md", + "SECURITY.md", "setup.cfg", "VERSION", "base_manifest.yaml", From 8c1577a0409351e691a132f0f18f9a4ad8c0bf49 Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Wed, 5 Aug 2026 08:44:24 -0700 Subject: [PATCH 3/3] fix: tolerate concurrent Windows history locks (#70) --- lib/python/base_cli/history.py | 9 ++++++++- tests/test_history.py | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/lib/python/base_cli/history.py b/lib/python/base_cli/history.py index 99b2a44..48be9ee 100644 --- a/lib/python/base_cli/history.py +++ b/lib/python/base_cli/history.py @@ -198,7 +198,14 @@ def append_history_line(path: Path, line: str) -> None: sidecar_path = path.with_name(f".{path.name}.lock") sidecar_fd = os.open(sidecar_path, os.O_RDWR | os.O_CREAT | binary_flag, 0o600) if os.fstat(sidecar_fd).st_size == 0: - os.write(sidecar_fd, b"0") + try: + os.write(sidecar_fd, b"0") + except PermissionError: + # Another Windows process can initialize the shared empty + # sidecar between fstat() and write(). Its byte is enough + # for the subsequent blocking msvcrt lock; do not turn + # that expected initialization race into a command error. + pass restrict_file(sidecar_path) lock_fd = sidecar_fd lock_history_file(lock_fd) diff --git a/tests/test_history.py b/tests/test_history.py index e92f5d0..bd0179e 100644 --- a/tests/test_history.py +++ b/tests/test_history.py @@ -59,3 +59,24 @@ def test_msvcrt_backend_uses_a_private_sidecar_lock(self) -> None: self.assertTrue(path.with_name(".history.jsonl.lock").is_file()) self.assertEqual(fake_msvcrt.calls, [(_FakeMsvcrt.LK_LOCK, 1), (_FakeMsvcrt.LK_UNLCK, 1)]) + + def test_msvcrt_sidecar_initialization_race_is_tolerated(self) -> None: + fake_msvcrt = _FakeMsvcrt() + original_write = history.os.write + calls = 0 + + def write_with_initialization_race(fd: int, data: bytes) -> int: + nonlocal calls + calls += 1 + if calls == 1: + raise PermissionError(13, "sidecar is being initialized") + return original_write(fd, data) + + with tempfile.TemporaryDirectory() as tmpdir: + path = Path(tmpdir) / "history.jsonl" + with mock.patch.object(history, "_fcntl", None), mock.patch.object( + history, "_msvcrt", fake_msvcrt + ), mock.patch.object(history.os, "write", side_effect=write_with_initialization_race): + history.append_history_line(path, '{"run": 1}\n') + + self.assertEqual(path.read_text(encoding="utf-8"), '{"run": 1}\n')