Record security audit findings
This commit is contained in:
194
docs/roadmap/security.md
Normal file
194
docs/roadmap/security.md
Normal file
@@ -0,0 +1,194 @@
|
||||
# Security Audit
|
||||
|
||||
Reviewed revision: `b30c9f167dba16a9192d9bfde4aeb2b9c597ce0e`.
|
||||
Audit date: 2026-10-25.
|
||||
|
||||
## Scope And Trust Model
|
||||
|
||||
This review examined input validation and budgets, import/discovery containment,
|
||||
link resolution, collection authority, application callbacks, secret references
|
||||
and bootstrap, historical sensitivity, diagnostics, effective rendering and
|
||||
diffs, and the runtime/development dependency boundary. Knowledge-graph searches
|
||||
and caller traces were supplemented by source inspection, existing tests, a
|
||||
bounded standalone diagnostic, and an advisory-database spot-check.
|
||||
|
||||
The [architecture](../policy/architecture.md) and
|
||||
[configuration contract](../config.md) establish these boundaries:
|
||||
|
||||
- ConfigKit is a synchronous startup library, not a network service or an
|
||||
authentication/authorization framework. Applications select inputs, roots,
|
||||
filesystem implementations, callbacks, and credential use. Applications own
|
||||
remote acquisition and authentication of that data.
|
||||
- The public facade delegates to a private engine and private document/YAML
|
||||
packages. Input composition and inspection do not execute shell commands or
|
||||
launch a background service. Ordinary loading does not change the process
|
||||
environment; explicit secrets-directory bootstrap is the narrow exception.
|
||||
- Host imports use canonical containment checks and owned rooted access under
|
||||
the documented stable-input assumption. Borrowed native filesystems retain
|
||||
their own behavior and are explicitly not a general sandbox guarantee.
|
||||
- Secret references resolve after composition and application resolution. The
|
||||
trusted returned configuration can expose credentials through `Value()`;
|
||||
inspection retains names rather than credentials. Tags, a path predicate,
|
||||
inheritance, and recorded dependencies govern redaction of other values.
|
||||
Application-supplied messages and unrecorded secret transformations remain
|
||||
application responsibilities.
|
||||
|
||||
One new availability finding warrants remediation. No new credential disclosure,
|
||||
command injection, application-authority bypass, or host containment escape was
|
||||
established. This is a source-level audit with bounded checks, not a certification
|
||||
that every input, dependency, platform, or deployment is secure.
|
||||
|
||||
## 1. Finite Symlink Reuse Can Cause Exponential Import-Resolution Work
|
||||
|
||||
Status: open. Priority: medium. Category: availability / input amplification.
|
||||
|
||||
**Relevant code.** `resolveHostImportPath` and `resolveNativeImportPath` in
|
||||
[filesystem_links.go](../../internal/engine/filesystem_links.go), lines 129–242,
|
||||
expand link-target components with a frame stack and track only currently active
|
||||
links. Completion removes a link from that set; a later use resolves its entire
|
||||
target again. No completed-target result is reused.
|
||||
|
||||
`importEntryTarget` invokes these resolvers for discovered links. Both
|
||||
[directory selection](../../internal/engine/import_selection.go) and
|
||||
[glob selection](../../internal/engine/glob.go) reach it. Directory selection
|
||||
resolves the target before deciding whether recursive descent is enabled, so
|
||||
`recursive: false` does not avoid this work. Native resolution is used when the
|
||||
borrowed filesystem provides `fs.ReadLinkFS`; host resolution uses the analogous
|
||||
rooted implementation.
|
||||
|
||||
**Plausible scenario.** An application loads a configuration tree whose imported
|
||||
assets can be supplied by a less-trusted user or configuration provider. That
|
||||
provider controls link entries inside an allowed import directory. The
|
||||
application's own main file need only select that directory with link following
|
||||
enabled, which is the current default. No concurrent filesystem mutation,
|
||||
outside-root access, executable callback, or malicious filesystem implementation
|
||||
is required. This is conditional on influence over imported filesystem data;
|
||||
it is not an anonymous remote vulnerability in ConfigKit by itself.
|
||||
|
||||
Inside an ordinary `links` directory, create this finite graph:
|
||||
|
||||
```text
|
||||
s00 -> .
|
||||
s01 -> s00/s00
|
||||
s02 -> s01/s01
|
||||
s03 -> s02/s02
|
||||
...
|
||||
```
|
||||
|
||||
Each link resolves to the same contained directory. It has no active link cycle:
|
||||
the first reference to a lower link finishes before the second begins. Resolving
|
||||
level N nevertheless repeats the level N-1 expansion twice, giving a recurrence
|
||||
of `T(N) = 2T(N-1) + O(1)` component work for this fixed-name fixture. Directory
|
||||
enumeration contains only N+1 entries, and no imported configuration nodes or
|
||||
new import edges account for those repeated expansions. Consequently, source,
|
||||
entry, input-node, and active-import-depth budgets do not bound this work by the
|
||||
size of the selected graph. This is repeated resolution of a compact graph, not
|
||||
genuinely expanding configuration output or ordinary deterministic sorting.
|
||||
|
||||
**Evidence.** A standalone public-API diagnostic created real temporary symlinks
|
||||
and exposed them through `NewFilesystem` backed by `os.DirFS`. A thin
|
||||
`fs.ReadLinkFS` wrapper counted metadata/link operations and imposed a diagnostic
|
||||
ceiling of 12,000 `Lstat` calls. The main input selected `dir: links` with link
|
||||
following enabled and default nonrecursive discovery. Successful loads produced
|
||||
these counts:
|
||||
|
||||
| Link entries | `Lstat` calls | `ReadLink` calls |
|
||||
| --- | ---: | ---: |
|
||||
| 3 | 32 | 11 |
|
||||
| 5 | 128 | 57 |
|
||||
| 7 | 512 | 247 |
|
||||
| 9 | 2,048 | 1,013 |
|
||||
| 11 | 8,192 | 4,083 |
|
||||
|
||||
A 13-entry fixture reached the diagnostic ceiling and returned a safe load
|
||||
failure when the wrapper stopped it. The same fixture with `follow: false`
|
||||
completed with 17 `Lstat` calls and zero `ReadLink` calls. The operation counts
|
||||
substantiate amplification in the native resolver; the host resolver's matching
|
||||
frame-expansion logic was verified by source inspection and caller tracing,
|
||||
not a separate host operation-count test. No timings, large attack graph, or
|
||||
external-system exploitation were used.
|
||||
|
||||
**Impact.** A small imported asset tree can monopolize startup/load execution for
|
||||
an impractically long period. Applications that expose configuration validation
|
||||
or repeated loading to less-trusted providers can also tie up their own workers.
|
||||
The normal trusted-operator-only deployment reduces the adversarial exposure,
|
||||
but accidental alias graphs can cause the same availability failure.
|
||||
|
||||
**Potential remediation.** Reuse fully resolved link-target results within the
|
||||
resolution operation, keyed by the appropriate rooted namespace and full link
|
||||
path, while retaining active-path cycle detection and containment checks.
|
||||
Equivalent dependency-based evaluation is another option. Preserve legitimate
|
||||
finite reuse, relative/absolute host-link semantics, and the stable-input model;
|
||||
do not replace the active set with a global "already visited means cycle" rule.
|
||||
Review both resolvers and directory/glob entry points. The objective is to avoid
|
||||
re-expanding completed dependencies, not to introduce a public cache or reject
|
||||
valid graphs with an arbitrary link-count cap. Consumers can already opt out of
|
||||
following discovered links where appropriate; changing that default is not part
|
||||
of this recommendation.
|
||||
|
||||
## Previously Reported Risks
|
||||
|
||||
The [correctness audit](correctness.md) already records pointer-only recursive
|
||||
schema nontermination, failed dynamic-descriptor cache state, and callback guards
|
||||
left active during panic unwinding. The [resource audit](resources.md) owns
|
||||
pre-budget typed-container allocation, incomplete retained-history accounting,
|
||||
and an import-root cleanup gap after callback panic. These can affect availability
|
||||
when their documented preconditions hold; they are not newly discovered remote
|
||||
exploits or duplicate remediation tasks here.
|
||||
|
||||
The [error-handling audit](error-handling.md) separately owns silently omitted
|
||||
directory contributions after certain enumeration/close failures. The
|
||||
[efficiency audit](efficiency.md) owns previously identified avoidable batch costs.
|
||||
This review does not reclassify every expensive operation or application
|
||||
programming error as a security vulnerability.
|
||||
|
||||
## Dependency Spot-Check
|
||||
|
||||
The ordinary package's non-standard-library dependency graph contains only
|
||||
`go.yaml.in/yaml/v4 v4.0.0-rc.6`; ReleaseKit, Goldmark, and `x/mod` are maintainer
|
||||
tool dependencies rather than ConfigKit runtime imports. Cached module integrity
|
||||
verification passed.
|
||||
|
||||
The current [Go vulnerability database module index](https://vuln.go.dev/index/modules.json)
|
||||
contained no entries for the selected YAML v4, ReleaseKit, or Goldmark v2 module
|
||||
paths. Its two `golang.org/x/mod` reports,
|
||||
[GO-2026-6179](https://pkg.go.dev/vuln/GO-2026-6179) and
|
||||
[GO-2026-6180](https://pkg.go.dev/vuln/GO-2026-6180), identify v0.40.0 as fixed,
|
||||
which is the version selected here. The related toolchain fixes also precede the
|
||||
Go 1.26.10 toolchain used for this audit. The database metadata reported a modified
|
||||
time of `2026-10-21T00:27:29Z` when checked.
|
||||
|
||||
This was a module-path/advisory spot-check, not a complete standard-library,
|
||||
toolchain, transitive-reachability, or supply-chain attestation. `govulncheck`,
|
||||
`gosec`, and `gitleaks` were not installed; no scanners were added to the project.
|
||||
Absence of an advisory is not proof of absence of vulnerabilities, and
|
||||
`go mod verify` does not audit dependency behavior or establish that upstream
|
||||
content is safe.
|
||||
The [database API documentation](https://go.dev/doc/security/vuln/database)
|
||||
describes the advisory source used for this check.
|
||||
|
||||
## Validation And Preserved Boundaries
|
||||
|
||||
The following passed with Go 1.26.10 on Linux amd64 using cached dependencies and
|
||||
disabled automatic dependency/toolchain acquisition:
|
||||
|
||||
```sh
|
||||
go test -race ./... -count=1
|
||||
go test ./internal/engine -run 'Secret|Sensitive|Sensitivity|Contain|Escap|Outside|Symlink|Nonregular|Budget|Limit|Diagnostic' -count=1
|
||||
go vet ./...
|
||||
go mod verify
|
||||
```
|
||||
|
||||
These checks include existing canary-based non-disclosure, historical and
|
||||
bilateral diff sensitivity, safe diagnostic, parser-policy, containment, and
|
||||
nonregular-file cases. The bounded link diagnostic ran outside the repository.
|
||||
No production code or repository tests were added or changed; no native
|
||||
macOS/Windows run, active external exploitation, or exhaustive fuzzing was done.
|
||||
|
||||
This report does not propose authentication in the library, arbitrary environment
|
||||
name restrictions, secret-file permission requirements, retries, memory erasure,
|
||||
or protection against privileged concurrent filesystem mutation. Those would
|
||||
need their own concrete requirement and threat analysis. It preserves borrowed
|
||||
filesystem ownership, application-owned safe callback messages, public secret
|
||||
reference names, explicit partial bootstrap semantics, and the current release
|
||||
gates.
|
||||
Reference in New Issue
Block a user