Skip to content

Add manifest environment access policy #666

Description

@leynos

Summary

Add manifest-level access control for environment variables read through the
Jinja env() helper. The control must be enforced at env() call time in
src/manifest/env_reader.rs, before a blocked lookup can return a process
environment value.

This reduces secret ingress into rendered manifest fields. It does not make
rendered manifest contents inherently safe to log: any value that is allowed
and rendered still needs the existing output and logging discipline.

Thanks to @benrrr56-wq for the nudge here — a little bouncer for env() seems
like a very sensible addition.

Proposed configuration and policy shape

Follow the named-policy idiom used by NetworkPolicy in
src/stdlib/network/policy/mod.rs and its CliConfig wiring. Introduce a
purpose-specific environment-access policy, constructed from the effective CLI
configuration and supplied to manifest loading; do not model this as a
PolicyDefinition<T> tri-state.

Add these appendable CliConfig fields, with the corresponding normal
configuration, environment, and CLI sources generated by the existing config
machinery:

# netsuke.toml (or another supported Netsuke configuration layer)
env_allow_var = ["CI", "PACKAGE_REGISTRY_TOKEN"]
env_block_var = ["AWS_SECRET_ACCESS_KEY", "GITHUB_TOKEN"]

The public spellings should be NETSUKE_ENV_ALLOW_VAR and
NETSUKE_ENV_BLOCK_VAR for environment configuration, and
--env-allow-var NAME and --env-block-var NAME for CLI configuration. Each
entry is one exact environment-variable name; no glob or pattern matching is
introduced by this ticket. The sample configuration and generated CLI help must
document the same syntax.

The policy must be default-allow for compatibility: when neither list is
configured, every variable retains today's env() behaviour. An allowlist
becomes restrictive only when it contains at least one entry: then only listed
variables may resolve, subject to the denylist. A denylist without an allowlist
blocks only its listed variables. This preserves existing manifests unchanged
by default, avoids a breaking migration, and lets maintainers adopt explicit
allow rules incrementally. A future default-deny migration would require a
separate, versioned compatibility plan; it is out of scope here.

When both lists apply to the same variable, the denylist wins, matching the
NetworkPolicy precedent that blocked hosts are denied even when allowed
elsewhere. Evaluate the block-list before returning a value, and keep list
contents out of user-facing diagnostics and trace fields.

Blocked-access diagnostics

A blocked env() access must fail with a new fixed-text, localised manifest
diagnostic (for example, manifest.env.blocked). Match the no-name pattern of
manifest.env.missing and manifest.env.invalid_utf8:

  • do not include the requested variable name or a value in the error;
  • do not interpolate the variable name into the log message or tracing fields;
  • emit only a bounded trace such as
    tracing::debug!(failure_kind = "blocked", "manifest env lookup failed").

The Jinja template location remains sufficient to identify the failed env()
call without turning credentials or their names into diagnostic data.

Acceptance criteria

  • A named environment-access policy is wired from CliConfig into manifest
    loading and enforced in src/manifest/env_reader.rs at every env()
    call.
  • env_allow_var and env_block_var have the documented TOML,
    NETSUKE_..., and CLI spellings; their entries are exact variable names.
  • With neither list configured, env() remains default-allow so existing
    manifests do not need migration.
  • A non-empty allowlist enables default-deny for names outside that list;
    a denylist alone blocks only its entries.
  • A matching deny rule always overrides a matching allow rule.
  • A permitted variable resolves successfully through env().
  • A denied variable is blocked with a bounded, value-free, name-free error
    and failure_kind = "blocked" trace; tests prove neither the variable
    name nor its value reaches either diagnostic surface.
  • Existing missing and invalid-UTF-8 diagnostics retain their fixed-text,
    no-name behaviour.
  • Documentation explains that this is secret-ingress reduction, not a
    guarantee that rendered manifest content is safe to log.
  • Relevant unit, integration, and documentation gates pass.

Relationship to #644 and Phases 1 to 3

This is defence in depth alongside #644,
which prevents project configuration from widening trusted fetch policy. It
addresses a different boundary: which host-environment values a manifest may
bring into rendered fields.

It does not replace, weaken, or reopen the removal of unsafe AST-dump logging
delivered by Phases 1 to 3. That remediation remains required independently;
this policy merely reduces the set of secrets that can enter rendered manifest
fields in the first place.

References

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions