Skip to content
Open
1 change: 1 addition & 0 deletions src/audit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pub(crate) mod known_vulnerable_actions;
pub(crate) mod overprovisioned_secrets;
pub(crate) mod ref_confusion;
pub(crate) mod secrets_inherit;
pub(crate) mod secrets_outside_environment;
pub(crate) mod self_hosted_runner;
pub(crate) mod template_injection;
pub(crate) mod unpinned_uses;
Expand Down
80 changes: 80 additions & 0 deletions src/audit/secrets_outside_environment.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
use github_actions_models::{
common::{expr::LoE, Env, EnvValue},
workflow::job::StepBody,
};

use super::{audit_meta, Audit};
use crate::{finding::Confidence, models::Job};

pub(crate) struct SecretsOutsideEnvironment;

audit_meta!(
SecretsOutsideEnvironment,
"secrets-outside-environment",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to make new audit names a bit shorter, so some ideas here:

  1. secret-outside-env
  2. secret-without-env
  3. secret-missing-env
  4. secret-no-env (shortest, but maybe not easy to understand)

Curious if you have any other naming thoughts as well 🙂

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Picked number 2 and fixed in 96c90ac.

"secrets used without an environment to gate them"
);

impl Audit for SecretsOutsideEnvironment {
fn new(_state: super::AuditState) -> anyhow::Result<Self>
where
Self: Sized,
{
Ok(Self)
}

fn audit_raw<'w>(
&self,
input: &'w super::AuditInput,
) -> anyhow::Result<Vec<crate::finding::Finding<'w>>> {
let mut findings = vec![];

if let super::AuditInput::Workflow(w) = input {
for job in w.jobs() {
if let Job::NormalJob(j) = job {
if j.environment().is_some() {
continue;
}

for step in j.steps() {
let body = &step.body;
let eenv: &Env;

match body {
StepBody::Uses { uses: _, with } => {
eenv = with;
}
StepBody::Run {
run: _,
shell: _,
env,
working_directory: _,
} => match env {
LoE::Expr(_) => {
// TODO: Implement this.
panic!("We don't handle Expr yet!")
}
LoE::Literal(env) => eenv = env,
},
}

for v in eenv.values() {
if let EnvValue::String(s) = v {
if s.contains("secrets") {
findings.push(
Self::finding()
.add_location(step.location().primary())
.confidence(Confidence::High)
.severity(crate::finding::Severity::High)
.build(input)?,
);
}
}
}
}
}
}
}

Ok(findings)
}
}
11 changes: 6 additions & 5 deletions src/main.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use std::{
io::{Write, stdout},
io::{stdout, Write},
process::ExitCode,
str::FromStr,
};

use annotate_snippets::{Level, Renderer};
use anstream::{eprintln, stream::IsTerminal};
use anyhow::{Context, Result, anyhow};
use anyhow::{anyhow, Context, Result};
use audit::Audit;
use camino::{Utf8Path, Utf8PathBuf};
use clap::{Parser, ValueEnum};
Expand All @@ -21,9 +21,9 @@ use models::Action;
use owo_colors::OwoColorize;
use registry::{AuditRegistry, FindingRegistry, InputRegistry};
use state::AuditState;
use tracing::{Span, info_span, instrument};
use tracing_indicatif::{IndicatifLayer, span_ext::IndicatifSpanExt};
use tracing_subscriber::{EnvFilter, layer::SubscriberExt as _, util::SubscriberInitExt as _};
use tracing::{info_span, instrument, Span};
use tracing_indicatif::{span_ext::IndicatifSpanExt, IndicatifLayer};
use tracing_subscriber::{layer::SubscriberExt as _, util::SubscriberInitExt as _, EnvFilter};

mod audit;
mod config;
Expand Down Expand Up @@ -501,6 +501,7 @@ fn run() -> Result<ExitCode> {
register_audit!(audit::github_env::GitHubEnv);
register_audit!(audit::cache_poisoning::CachePoisoning);
register_audit!(audit::secrets_inherit::SecretsInherit);
register_audit!(audit::secrets_outside_environment::SecretsOutsideEnvironment);
register_audit!(audit::bot_conditions::BotConditions);
register_audit!(audit::overprovisioned_secrets::OverprovisionedSecrets);
register_audit!(audit::unredacted_secrets::UnredactedSecrets);
Expand Down
14 changes: 9 additions & 5 deletions src/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,17 @@ use std::collections::HashMap;
use std::fmt::Debug;
use std::{iter::Enumerate, ops::Deref};

use anyhow::{Context, Result, bail};
use anyhow::{bail, Context, Result};
use camino::Utf8Path;
use github_actions_models::common::Env;
use github_actions_models::common::expr::LoE;
use github_actions_models::common::Env;
use github_actions_models::workflow::event::{BareEvent, OptionalBody};
use github_actions_models::workflow::job::{RunsOn, Strategy};
use github_actions_models::workflow::{self, Trigger, job, job::StepBody};
use github_actions_models::workflow::job::{DeploymentEnvironment, RunsOn, Strategy};
use github_actions_models::workflow::{self, job, job::StepBody, Trigger};
use github_actions_models::{action, common};
use indexmap::IndexMap;
use line_index::LineIndex;
use serde_json::{Value, json};
use serde_json::{json, Value};
use terminal_link::Link;

use crate::finding::{Route, SymbolicLocation};
Expand Down Expand Up @@ -210,6 +210,10 @@ impl<'w> NormalJob<'w> {
Steps::new(self)
}

pub(crate) fn environment(&self) -> Option<&DeploymentEnvironment> {
self.inner.environment.as_ref()
}

/// Perform feats of heroism to figure of what this job's runner's
/// default shell is.
///
Expand Down
12 changes: 12 additions & 0 deletions tests/integration/test-data/secrets-outside-environment.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
name: Action
on: push
jobs:
build:
name: Job
runs-on: ubuntu-latest
steps:
- name: Docker setup
uses: actions_repo/actions/docker@main
with:
username: ${{ secrets.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_PASSWORD }}