Skip to content

Restore S3 carver wiring and reject the unimplemented local carver - #1095

Merged
javuto merged 1 commit into
developfrom
s3-carvers-initialization
Oct 17, 2026
Merged

javuto merged 1 commit into
developfrom
s3-carvers-initialization

Conversation

@javuto

@javuto javuto commented Oct 17, 2026

Copy link
Copy Markdown
Collaborator

Restore S3 carver wiring and reject the unimplemented local carver

Problem

File carving has three carver types (--carver-type = db | s3 | local), but only db worked:

  • S3 was never set up. The "Single YAML configuration file" refactor (e0938b7) removed the only call to carves.CreateCarverS3.
    • In osctrl-tls, carvers3 was declared but never set, so with --carver-type=s3 every carve block failed with s3 carver not initialized. Before that, InitateBlock dereferenced the nil carver.
    • osctrl-api passed nil, so archiving or downloading an S3 carve dereferenced a nil h.Carves.S3.
  • local never worked. CreateBlock had no local case in any version, so every block failed with unknown carver, and carver.local.carvesDir was never read. History shows local was only ever added to the valid-types maps. carvesDir was osctrl-admin's archive staging folder, which went away when osctrl-admin was removed.
  • No carver: section crashed startup. The carver was built from flagParams.Carver.Type before the service config was resolved, so a YAML file without that section caused a nil-pointer crash.
  • Empty static credentials broke role-based setups. CreateCarverS3 always installed static credentials, even empty ones. That breaks the instance/task-role setup the sample config recommends.

Changes

  • New carves.CreateFileCarvesFromConfig: the single place where the carver is built from config. Both osctrl-tls and osctrl-api call it after serviceConfigMgr.Resolve, so carver settings saved in the DB apply. Startup fails with a clear error when:

    • the type is unknown, or is local;
    • the type is s3 and carver.s3.bucket or carver.s3.region is missing;
    • only one of accessKey / secretAccessKey is set.

    A missing carver: section defaults to db.

  • config.ValidateCarverType: shared type check, also used when osctrl-tls loads its YAML. The local error reads: carver type "local" is not supported; use "db" (blocks stored in the database) or "s3".

  • CreateCarverS3: uses static keys only when they're configured; otherwise it uses the default AWS credential chain (env, shared config, credential_process, IAM Roles Anywhere, web identity, instance/task role). This matches the S3 logger fix in S3 logger: use the default AWS credential chain when no static keys are configured #1094.

  • InitateBlock / Archive: return s3 carver not initialized instead of crashing when the carver is s3 but no S3 client exists. Removed the dead local branch in Archive.

  • Docs: removed local (and the undocumented none) from the sample tls.yml / api.yml and from the service-config UI help. Kept --carver-local-dir / CARVER_LOCAL_DIR and the LocalCarver struct so existing command lines and YAML still parse; the flag's help now says it's unused.

Why reject local instead of implementing it

It has never worked, and making it work means osctrl-tls and osctrl-api must share a filesystem: TLS writes the blocks and API archives and serves them. The usual deployment runs them on different hosts or containers. db already covers keeping data local, and s3 covers external storage.

Security / operational impact

  • Breaking for local configs: any deployment with carver.type: local (or CARVER_TYPE=local) now refuses to start instead of silently failing every carve. The fix is to switch to db or s3. The same applies to a local carver type saved in the service-config DB.
  • Incomplete S3 config: deployments with carver.type: s3 and missing bucket or region now fail at startup instead of failing per block.
  • Credentials: S3 carving now works without static keys. Least privilege needs s3:PutObject and s3:GetObject on the bucket's objects.
  • Known limitation (unchanged): S3 archiving uses multipart UploadPartCopy, so every block except the last must be at least 5 MiB on real S3. Environments using the S3 carver need carver_block_size of 5 MiB or more.

Validation

  • TDD: the new tests were written first and watched fail (nil-S3 panic, empty static credentials, missing wiring, local accepted).
    • pkg/carves/carver_test.go:
      • DB carver: blocks stored and archive rebuilt locally.
      • S3 carver, against an in-process fake S3 server: block upload, multipart archive, download.
      • Nil S3 client returns an error instead of crashing.
      • Table test of every config case.
      • Credential-chain fallback.
    • cmd/tls/carver_test.go, cmd/api/carver_test.go: YAML load → service parameters → carver, for db, s3, incomplete s3 and local.
    • pkg/config/validation_test.go: local rejected when osctrl-tls validates its YAML.
  • go test ./..., go vet ./..., gofmt -l, golangci-lint run ./... (0 issues), make openapi-check: all pass, on top of develop at 81d10eb.
  • Not run: frontend npm run check / Vitest (no node_modules in this worktree). The frontend change is two help strings in ServiceConfigPage.tsx.

@javuto javuto added carves File carving related issues 🚧 bugfix Fix for an existing bug labels Oct 17, 2026
@javuto
javuto merged commit 3d81324 into develop Oct 17, 2026
8 checks passed
@javuto
javuto deleted the s3-carvers-initialization branch October 17, 2026 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🚧 bugfix Fix for an existing bug carves File carving related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant