Skip to content

Commit

Permalink
Raise an error if a secret is redacted while log is in verbose.
Browse files Browse the repository at this point in the history
  • Loading branch information
jcamiel authored and hurl-bot committed Jan 20, 2025
1 parent 42444f6 commit dc8cf59
Show file tree
Hide file tree
Showing 19 changed files with 102 additions and 32 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: Assert body value
--> tests_failed/secret.hurl:3:1
--> tests_failed/assert_secret.hurl:3:1
|
| GET http://localhost:8000/secret-failed
| ...
Expand Down
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Set-StrictMode -Version latest
$ErrorActionPreference = 'Stop'

hurl --secret name=Alice tests_failed/secret.hurl
hurl --secret name=Alice tests_failed/assert_secret.hurl
File renamed without changes.
4 changes: 4 additions & 0 deletions integration/hurl/tests_failed/assert_secret.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/bin/bash
set -Eeuo pipefail

hurl --secret name=Alice tests_failed/assert_secret.hurl
4 changes: 0 additions & 4 deletions integration/hurl/tests_failed/secret.sh

This file was deleted.

30 changes: 30 additions & 0 deletions integration/hurl/tests_failed/secret_errors.err
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
error: Readonly secret
--> tests_failed/secret_errors.hurl:6:11
|
| GET http://localhost:8000/hello
| ...
6 | variable: name=a_public_value
| ^^^^^^^^^^^^^^^^^^^ secret 'name' can't be reassigned
|

error: Invalid secret type
--> tests_failed/secret_errors.hurl:13:1
|
| GET http://localhost:8000/hello
| ...
13 | token1: header "Date" toDate "%a, %d %b %Y %H:%M:%S GMT%Z" redact
| ^^^^^^ secret must be string, actual value is <date>
|

* ------------------------------------------------------------------------------
* Executing entry 3
*
* Entry options:
* verbose: true
error: Invalid redacted secret
--> tests_failed/secret_errors.hurl:22:1
|
22 | token2: header "Date" redact
| ^^^^^^ redacted secret not authorized in verbose
|

22 changes: 22 additions & 0 deletions integration/hurl/tests_failed/secret_errors.hurl
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# We check that variable can't override secret because a secret becoming
# a public variable will leak it previous secret value.
GET http://localhost:8000/hello
x-header: a_secret_value
[Options]
variable: name=a_public_value
HTTP 200

# Secrets must be strings
GET http://localhost:8000/hello
HTTP 200
[Captures]
token1: header "Date" toDate "%a, %d %b %Y %H:%M:%S GMT%Z" redact


# Entries that created new secrets with `redact` can't be log in normal mode (vs --test mode)
GET http://localhost:8000/hello
[Options]
verbose: true
HTTP 200
[Captures]
token2: header "Date" redact
4 changes: 4 additions & 0 deletions integration/hurl/tests_failed/secret_errors.ps1
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Set-StrictMode -Version latest
$ErrorActionPreference = 'Stop'

hurl --continue-on-error --secret name=a_secret_value tests_failed/secret_errors.hurl
4 changes: 4 additions & 0 deletions integration/hurl/tests_failed/secret_errors.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/bin/bash
set -Eeuo pipefail

hurl --continue-on-error --secret name=a_secret_value tests_failed/secret_errors.hurl
9 changes: 0 additions & 9 deletions integration/hurl/tests_failed/secret_overridden.err

This file was deleted.

7 changes: 0 additions & 7 deletions integration/hurl/tests_failed/secret_overridden.hurl

This file was deleted.

4 changes: 0 additions & 4 deletions integration/hurl/tests_failed/secret_overridden.ps1

This file was deleted.

4 changes: 0 additions & 4 deletions integration/hurl/tests_failed/secret_overridden.sh

This file was deleted.

4 changes: 3 additions & 1 deletion integration/hurl/tests_ok/secret.ps1
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
Set-StrictMode -Version latest
$ErrorActionPreference = 'Stop'

Remove-Item -Recurse build/secret
if (Test-Path -Path build/secret) {
Remove-Item -Recurse build/secret
}

hurl --very-verbose `
--secret a=secret1 `
Expand Down
25 changes: 25 additions & 0 deletions packages/hurl/src/runner/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use crate::runner::result::{AssertResult, EntryResult};
use crate::runner::runner_options::RunnerOptions;
use crate::runner::{request, response, CaptureResult, RunnerErrorKind, VariableSet};
use crate::util::logger::{Logger, Verbosity};
use crate::util::term::WriteMode;

/// Runs an `entry` with `http_client` and returns one [`EntryResult`].
///
Expand All @@ -44,6 +45,29 @@ pub fn run(
let source_info = entry.source_info();
let context_dir = &runner_options.context_dir;

// We don't allow creating secrets if the logger is immediate and verbose because, in this case,
// network logs have already been written and may have leaked secrets before captures evaluation.
// Note: in `--test` mode, the logger is buffered so there is no restriction on logger level.
if let Some(response_spec) = &entry.response {
let immediate_logs =
matches!(logger.stderr.mode(), WriteMode::Immediate) && logger.verbosity.is_some();
if immediate_logs {
let redacted = response_spec.captures().iter().find(|c| c.redact);
if let Some(redacted) = redacted {
let source_info = redacted.name.source_info;
let error =
RunnerError::new(source_info, RunnerErrorKind::PossibleLoggedSecret, false);
return EntryResult {
entry_index,
source_info,
errors: vec![error],
compressed,
..Default::default()
};
}
}
}

// Evaluates our source requests given our set of variables
let http_request = match request::eval_request(&entry.request, variables, context_dir) {
Ok(r) => r,
Expand All @@ -57,6 +81,7 @@ pub fn run(
};
}
};

let client_options = ClientOptions::from(runner_options, logger.verbosity);

// Experimental features with cookie storage
Expand Down
9 changes: 8 additions & 1 deletion packages/hurl/src/runner/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ pub enum RunnerErrorKind {
message: String,
},
NoQueryResult,
PossibleLoggedSecret,
QueryHeaderNotFound,
QueryInvalidJsonpathExpression {
value: String,
Expand Down Expand Up @@ -148,6 +149,7 @@ impl DisplaySourceError for RunnerError {
RunnerErrorKind::InvalidUrl { .. } => "Invalid URL".to_string(),
RunnerErrorKind::InvalidRegex => "Invalid regex".to_string(),
RunnerErrorKind::NoQueryResult => "No query result".to_string(),
RunnerErrorKind::PossibleLoggedSecret => "Invalid redacted secret".to_string(),
RunnerErrorKind::QueryHeaderNotFound => "Header not found".to_string(),
RunnerErrorKind::QueryInvalidJson => "Invalid JSON".to_string(),
RunnerErrorKind::QueryInvalidJsonpathExpression { .. } => {
Expand Down Expand Up @@ -272,6 +274,11 @@ impl DisplaySourceError for RunnerError {
let message = error::add_carets(message, self.source_info, content);
color_red_multiline_string(&message)
}
RunnerErrorKind::PossibleLoggedSecret => {
let message = "redacted secret not authorized in verbose";
let message = error::add_carets(message, self.source_info, content);
color_red_multiline_string(&message)
}
RunnerErrorKind::QueryHeaderNotFound => {
let message = "this header has not been found in the response";
let message = error::add_carets(message, self.source_info, content);
Expand Down Expand Up @@ -321,7 +328,7 @@ impl DisplaySourceError for RunnerError {
color_red_multiline_string(&message)
}
RunnerErrorKind::UnsupportedSecretType(kind) => {
let message = &format!("secret type <{kind}> are not supported");
let message = &format!("secret must be string, actual value is <{kind}>");
let message = error::add_carets(message, self.source_info, content);
color_red_multiline_string(&message)
}
Expand Down

0 comments on commit dc8cf59

Please sign in to comment.