Project

General

Profile

action #113312

Updated by livdywan almost 2 years ago

## Observation 

 https://openqa.opensuse.org/tests/2453477/logfile?filename=autoinst-log.txt 
 shows 

 ``` 
 susedistribution::handle_password_prompt -> lib/susedistribution.pm:70 called testapi::type_password 
 [2022-07-06T12:06:05.534656+02:00] [debug] <<< testapi::type_string(string="[masked]", max_interval=100, wait_screen_change=0, wait_still_screen=0, timeout=30, similarity_level=47) 
 [2022-07-06T12:06:05.535894+02:00] [debug] <<< consoles::video_base::type_string(max_interval=100, cmd="backend_type_string", json_cmd_token="HWQIViIt", text="nots3cr3t") 
 ``` 

 I guess it should be obvious that this is not intended this way. There had been multiple people working on hiding passwords but apparently it only gets worse. Let's be better :) 

 ## Acceptance criteria 
 * **AC1:** No log messages show the password, duh, in particular no password from consoles::video_base::type_string 

 ## Suggestions 
 * Check why we even have "testapi::type_string" and "video_base::type_string" and repeat the log message in both. Maybe we can avoid the log in consoles::video_base::type_string 
 * Check unit tests if maybe we just look at the log message of testapi::type_string but not video_base 
 * Try to reproduce locally and use `git bisect` to find the culprit introducing the regression: 
     Already clear. commit 4e66dcc0 introduced this: 
        
 ``` 
 (cfconrad/log_call_secrets) 
 Author: Clemens Famulla-Conrad <cfamullaconrad@suse.de> 
 Date:     Wed May 18 10:50:07 2022 +0200 

     bmwqemu: Hide secrets on log output with log_call() 
    
     Parse given parameters for `-masked => <string>` and replace 
     all occurrence from the given `<string>` with `[masked]` in the 
     log output. 

 diff --git a/consoles/video_base.pm b/consoles/video_base.pm 
 index 6dba68d4..549f12dc 100644 
 --- a/consoles/video_base.pm 
 +++ b/consoles/video_base.pm 
 @@ -30,6 +30,8 @@ sub _typing_limit () { $bmwqemu::vars{TYPING_LIMIT} // TYPING_LIMIT_DEFAULT || 1 
  sub send_key_event ($key, $press_release_delay) { } 
 
  sub type_string ($self, $args) { 
 +      bmwqemu::log_call(%$args, $args->{secret} ? (-masked => $args->{text}) : ()); 
 ``` 

 ## Previous PR URLs 
 * https://github.com/os-autoinst/os-autoinst/pull/2062 
 * https://github.com/os-autoinst/os-autoinst/pull/2002

Back