Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Scripted client nullable replies #121

Merged
merged 2 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions docs/running-the-scripted-client.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,16 @@ A prompt sequence is a simple JSON file with the following structure:
}
}
},
{
"prompt-filter": {
"constraints": {
"path": ".*/example.txt",
"permissions": [ "read" ],
"available-permissions": [ "read", "write", "execute" ]
}
},
"reply": null
}
...
]
}
Expand Down Expand Up @@ -89,7 +99,7 @@ but if you do then all prompts not matching it will be ignored by the scripted c
See the `Prompt filter fields` section below for specific details on each of the
supported fields for a filter.

### Prompts
### Prompt cases
A sequence of prompt cases: a prompt filter allowing for structural matching
against the next prompt in the sequence along with a template for build the
reply to that prompt provided the filter matches. If the filter does not match
Expand All @@ -116,7 +126,13 @@ prompts the following fields are available:
- `available-permissions`: the ordered list of available permissions seen in the prompt

### Reply templates
The only required fields for a reply template are the `action` (allow or deny)
If you wish the client to send a reply for a particular prompt then you should provide
a reply template under the `reply` field of prompt case. This is the expected default
behaviour of the client. In order to expect that a certain prompt be observed as part
of the sequence but _not_ send a reply you must explicitly provide `"reply": null`
rather than simply omitting the key.

When providing a reply template, the only required fields are the `action` (allow or deny)
and `lifespan` (single, session, forever or timespan). If `lifespan` is set to
timespan then the optional `duration` field must also be provided.

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
{
"version": 1,
"prompts": [
{
"prompt-filter": {
"snap": "testSnap",
"interface": "home",
"constraints": {
"path": "/home/foo/bar",
"requested-permissions": [ "read" ],
"available-permissions": [ "read", "write", "execute" ]
}
},
"reply": {
"action": "allow",
"lifespan": "single",
"constraints": {
"path-pattern": "/home/foo/bar",
"permissions": [ "read", "write" ]
}
}
},
{
"prompt-filter": {
"snap": "testSnap",
"interface": "home",
"constraints": {
"path": "/home/foo/bar",
"requested-permissions": [ "read" ],
"available-permissions": [ "read", "write", "execute" ]
}
},
"reply": null
}
]
}
18 changes: 13 additions & 5 deletions prompting-client/src/cli_actions/scripted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ impl ScriptedClient {
&mut self,
prompt: TypedPrompt,
prev_error: Option<String>,
) -> Result<TypedPromptReply> {
) -> Result<Option<TypedPromptReply>> {
if let Some(error) = prev_error {
return Err(Error::FailedPromptSequence {
error: MatchError::UnexpectedError { error },
Expand All @@ -169,10 +169,10 @@ impl ScriptedClient {

match prompt {
TypedPrompt::Home(inner) if inner.constraints.path == self.path => {
Ok(TypedPromptReply::Home(
Ok(Some(TypedPromptReply::Home(
// Using a timespan so our rule auto-removes
HomeInterface::prompt_to_reply(inner, Action::Allow).for_timespan("10s"),
))
)))
}

_ => match self.seq.try_match_next(prompt) {
Expand All @@ -191,7 +191,10 @@ impl ScriptedClient {
EnrichedPrompt { prompt, .. }: EnrichedPrompt,
snapd_client: &mut SnapdSocketClient,
) -> Result<()> {
let mut reply = self.reply_for_prompt(prompt.clone(), None).await?;
let mut reply = match self.reply_for_prompt(prompt.clone(), None).await? {
Some(reply) => reply,
None => return Ok(()),
};
let id = prompt.id().clone();

debug!(id=%id.0, ?reply, "replying to prompt");
Expand All @@ -212,10 +215,15 @@ impl ScriptedClient {
};

debug!(%prev_error, "error returned from snapd, retrying");
reply = self
let maybe_reply = self
.reply_for_prompt(prompt.clone(), Some(prev_error))
.await?;

reply = match maybe_reply {
Some(reply) => reply,
None => return Ok(()),
};

debug!(id=%id.0, ?reply, "replying to prompt");
}

Expand Down
60 changes: 47 additions & 13 deletions prompting-client/src/prompt_sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::snapd_client::{
},
Action, Lifespan, Prompt, PromptReply, TypedPrompt, TypedPromptReply,
};
use serde::{Deserialize, Serialize};
use serde::{Deserialize, Deserializer, Serialize};
use std::{collections::VecDeque, fs};

#[allow(dead_code)]
Expand Down Expand Up @@ -40,7 +40,10 @@ impl PromptSequence {
}
}

pub fn try_match_next(&mut self, p: TypedPrompt) -> Result<TypedPromptReply, MatchError> {
pub fn try_match_next(
&mut self,
p: TypedPrompt,
) -> Result<Option<TypedPromptReply>, MatchError> {
let case = match self.prompts.pop_front() {
Some(case) => case,
None => return Err(MatchError::NoPromptsRemaining),
Expand All @@ -50,8 +53,9 @@ impl PromptSequence {
(TypedPromptCase::Home(case), TypedPrompt::Home(p)) => {
let res = case
.into_reply_or_error(p, self.index)
.map(TypedPromptReply::Home);
.map(|res| res.map(TypedPromptReply::Home));
self.index += 1;

res
}
}
Expand Down Expand Up @@ -95,14 +99,25 @@ impl TypedPromptFilter {
}
}

/// Override the default handling for a missing key in being parsed as `None` so that only an
/// explicit value of `null` is accepted.
fn explicit_null<'de, T, D>(de: D) -> Result<Option<T>, D::Error>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is what allows us to distinguish the field being explicitly specified as null vs simply being omitted. (The default behaviour of serde when deserializing into an Option<T> field is to treat both the same).

This is adapted form the example shown here in order to emit an error in the case of a missing field rather than parsing into an Option<Option<T>> and having to handle the validation explicitly after the struct has been deserialized.

where
T: Deserialize<'de>,
D: Deserializer<'de>,
{
Deserialize::deserialize(de)
}

#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case")]
pub struct PromptCase<I>
where
I: SnapInterface,
{
prompt_filter: PromptFilter<I>,
reply: PromptReplyTemplate<I>,
#[serde(deserialize_with = "explicit_null")]
reply: Option<PromptReplyTemplate<I>>,
}

impl<I> PromptCase<I>
Expand All @@ -113,20 +128,24 @@ where
self,
p: Prompt<I>,
index: usize,
) -> Result<PromptReply<I>, MatchError> {
match self.prompt_filter.matches(&p) {
MatchAttempt::Success => {
let mut reply = I::prompt_to_reply(p, self.reply.action);
reply.lifespan = self.reply.lifespan;
reply.duration = self.reply.duration;
if let Some(constraints) = self.reply.constraints {
) -> Result<Option<PromptReply<I>>, MatchError> {
match (self.prompt_filter.matches(&p), self.reply) {
(MatchAttempt::Success, None) => Ok(None),

(MatchAttempt::Success, Some(template)) => {
let mut reply = I::prompt_to_reply(p, template.action);
reply.lifespan = template.lifespan;
reply.duration = template.duration;
if let Some(constraints) = template.constraints {
reply.constraints = constraints.apply(reply.constraints);
}

Ok(reply)
Ok(Some(reply))
}

MatchAttempt::Failure(failures) => Err(MatchError::MatchFailures { index, failures }),
(MatchAttempt::Failure(failures), _) => {
Err(MatchError::MatchFailures { index, failures })
}
}
}
}
Expand Down Expand Up @@ -257,6 +276,21 @@ mod tests {
};
use simple_test_case::{dir_cases, test_case};

#[derive(Debug, Deserialize, PartialEq, Eq)]
struct TestStruct {
#[serde(deserialize_with = "explicit_null")]
a: Option<i32>,
}

#[test_case("{}", None; "field missing")]
#[test_case(r#"{"a":null}"#, Some(TestStruct { a: None }); "explicit null")]
#[test_case(r#"{"a":41}"#, Some(TestStruct { a: Some(41) }); "value")]
#[test]
fn explicit_null_works(s: &str, expected: Option<TestStruct>) {
let res = serde_json::from_str(s).ok();
assert_eq!(res, expected);
}

#[dir_cases("resources/filter-serialize-tests")]
#[test]
fn simple_serialize_works(path: &str, data: &str) {
Expand Down
Loading