Skip to content

Commit

Permalink
feat: "raw" audit support
Browse files Browse the repository at this point in the history
  • Loading branch information
woodruffw committed Jan 23, 2025
1 parent 7d1284c commit 80d906e
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 20 deletions.
17 changes: 15 additions & 2 deletions src/audit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub(crate) mod hardcoded_container_credentials;
pub(crate) mod impostor_commit;
pub(crate) mod insecure_commands;
pub(crate) mod known_vulnerable_actions;
pub(crate) mod overprovisioned_secrets;
pub(crate) mod ref_confusion;
pub(crate) mod secrets_inherit;
pub(crate) mod self_hosted_runner;
Expand Down Expand Up @@ -147,6 +148,10 @@ pub(crate) use audit_meta;
/// 2. [`Audit::audit_composite_step`]: runs on each composite step within the
/// action (most specific)
///
/// For both:
///
/// 1. [`Audit::audit_raw`]: runs on the raw, unparsed YAML document source
///
/// Picking a higher specificity means that the lower methods are shadowed.
/// In other words, if an audit chooses to implement [`Audit::audit`], it should implement
/// **only** [`Audit::audit`] and not [`Audit::audit_normal_job`] or
Expand Down Expand Up @@ -208,15 +213,23 @@ pub(crate) trait Audit: AuditCore {
Ok(results)
}

fn audit_raw<'w>(&self, _raw: &'w str) -> Result<Vec<Finding<'w>>> {
Ok(vec![])
}

/// The top-level auditing function for both workflows and actions.
///
/// Implementors **should not** override this blanket implementation,
/// since it's marked with tracing instrumentation.
#[instrument(skip(self))]
fn audit<'w>(&self, input: &'w AuditInput) -> Result<Vec<Finding<'w>>> {
match input {
let mut results = match input {
AuditInput::Workflow(workflow) => self.audit_workflow(workflow),
AuditInput::Action(action) => self.audit_action(action),
}
}?;

results.extend(self.audit_raw(input.document().source())?);

Ok(results)
}
}
41 changes: 41 additions & 0 deletions src/audit/overprovisioned_secrets.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
use std::ops::Range;

use crate::{expr::Expr, utils::extract_expressions};

use super::{audit_meta, Audit};

pub(crate) struct OverprovisionedSecrets;

audit_meta!(
OverprovisionedSecrets,
"overprovisioned-secrets",
"detects secrets that are overprovisioned"
);

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

fn audit_raw<'w>(&self, raw: &'w str) -> anyhow::Result<Vec<super::Finding<'w>>> {
for (expr, span) in extract_expressions(raw) {
let Ok(parsed) = Expr::parse(expr.as_bare()) else {
tracing::warn!("couldn't parse expression: {expr}", expr = expr.as_bare());
continue;
};

todo!()
}

Ok(vec![])
}
}

impl OverprovisionedSecrets {
fn secrets_expansions(span: Range<usize>, expr: &Expr) -> Vec<Range<usize>> {
todo!()
}
}
2 changes: 1 addition & 1 deletion src/audit/template_injection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ impl TemplateInjection {
step: &impl StepCommon<'s>,
) -> Vec<(String, Severity, Confidence, Persona)> {
let mut bad_expressions = vec![];
for expr in extract_expressions(run) {
for (expr, _) in extract_expressions(run) {
let Ok(parsed) = Expr::parse(expr.as_bare()) else {
tracing::warn!("couldn't parse expression: {expr}", expr = expr.as_bare());
continue;
Expand Down
1 change: 1 addition & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ fn run() -> Result<ExitCode> {
register_audit!(audit::cache_poisoning::CachePoisoning);
register_audit!(audit::secrets_inherit::SecretsInherit);
register_audit!(audit::bot_conditions::BotConditions);
register_audit!(audit::overprovisioned_secrets::OverprovisionedSecrets);

let mut results = FindingRegistry::new(&app, &config);
{
Expand Down
36 changes: 19 additions & 17 deletions src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Helper routines.
use std::ops::Range;

use camino::Utf8Path;
use github_actions_models::common::{
expr::{ExplicitExpr, LoE},
Expand Down Expand Up @@ -38,14 +40,14 @@ pub(crate) fn split_patterns(patterns: &str) -> impl Iterator<Item = &str> {
}

/// Parse an expression from the given free-form text, returning the
/// expression and the next offset at which to resume parsing.
/// expression and the expression's byte span (relative to the input).
///
/// Returns `None` if no expression is found, or an index past
/// the end of the text if parsing is successful but exhausted.
///
/// Adapted roughly from GitHub's `parseScalar`:
/// See: <https://github.com/actions/languageservices/blob/3a8c29c2d/workflow-parser/src/templates/template-reader.ts#L448>
fn extract_expression(text: &str) -> Option<(ExplicitExpr, usize)> {
fn extract_expression(text: &str) -> Option<(ExplicitExpr, Range<usize>)> {
let start = text.find("${{")?;

let mut end = None;
Expand All @@ -63,23 +65,23 @@ fn extract_expression(text: &str) -> Option<(ExplicitExpr, usize)> {
end.map(|end| {
(
ExplicitExpr::from_curly(&text[start..=end]).unwrap(),
end + 1,
start..end + 1,
)
})
}

/// Extract zero or more expressions from the given free-form text.
pub(crate) fn extract_expressions(text: &str) -> Vec<ExplicitExpr> {
pub(crate) fn extract_expressions(text: &str) -> Vec<(ExplicitExpr, Range<usize>)> {
let mut exprs = vec![];
let mut view = text;

while let Some((expr, next)) = extract_expression(view) {
exprs.push(expr);
while let Some((expr, span)) = extract_expression(view) {
exprs.push((expr, (span.start..span.end)));

if next >= text.len() {
if span.end >= text.len() {
break;
} else {
view = &view[next..];
view = &view[span.end..];
}
}

Expand Down Expand Up @@ -161,21 +163,21 @@ mod tests {
#[test]
fn test_parse_expression() {
let exprs = &[
("${{ foo }}", "foo", 10),
("${{ foo }}${{ bar }}", "foo", 10),
("leading ${{ foo }} trailing", "foo", 18),
("${{ foo }}", "foo", 0..10),
("${{ foo }}${{ bar }}", "foo", 0..10),
("leading ${{ foo }} trailing", "foo", 8..18),
(
"leading ${{ '${{ quoted! }}' }} trailing",
"'${{ quoted! }}'",
31,
8..31,
),
("${{ 'es''cape' }}", "'es''cape'", 17),
("${{ 'es''cape' }}", "'es''cape'", 0..17),
];

for (text, expected_expr, expected_idx) in exprs {
let (actual_expr, actual_idx) = extract_expression(text).unwrap();
for (text, expected_expr, expected_span) in exprs {
let (actual_expr, actual_span) = extract_expression(text).unwrap();
assert_eq!(*expected_expr, actual_expr.as_bare());
assert_eq!(*expected_idx, actual_idx);
assert_eq!(*expected_span, actual_span);
}
}

Expand All @@ -184,7 +186,7 @@ mod tests {
let expressions = r#"echo "OSSL_PATH=${{ github.workspace }}/osslcache/${{ matrix.PYTHON.OPENSSL.TYPE }}-${{ matrix.PYTHON.OPENSSL.VERSION }}-${OPENSSL_HASH}" >> $GITHUB_ENV"#;
let exprs = extract_expressions(expressions)
.into_iter()
.map(|e| e.as_curly().to_string())
.map(|(e, _)| e.as_curly().to_string())
.collect::<Vec<_>>();

assert_eq!(
Expand Down

0 comments on commit 80d906e

Please sign in to comment.