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.

Sign up for GitHub

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

715 defence successfully configured message should not disappear after a delay #778

Closed
wants to merge 4 commits into from
Closed

715 defence successfully configured message should not disappear after a delay #778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bf2294a
715 defenceId names updated, message for defence configured and reset…
dhinrichs-scottlogic Jan 16, 2024
34aaafc
715 linted
dhinrichs-scottlogic Jan 16, 2024
9cefe34
715 added message to chat for model change and slider configs
dhinrichs-scottlogic Jan 16, 2024
bd88920
715 removed old pop up message for defence configs
dhinrichs-scottlogic Jan 17, 2024
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
8 changes: 4 additions & 4 deletions backend/src/defaultDefences.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,15 @@ const defaultDefences: Defence[] = [
value: instructionDefencePrompt,
},
]),
createDefence(DEFENCE_ID.FILTER_USER_INPUT, [
createDefence(DEFENCE_ID.INPUT_FILTERING, [
{
id: 'FILTER_USER_INPUT',
id: 'INPUT_FILTERING',
value: 'secret project,confidential project,budget,password',
},
]),
createDefence(DEFENCE_ID.FILTER_BOT_OUTPUT, [
createDefence(DEFENCE_ID.OUTPUT_FILTERING, [
{
id: 'FILTER_BOT_OUTPUT',
id: 'OUTPUT_FILTERING',
value: 'secret project',
},
]),
Expand Down
12 changes: 5 additions & 7 deletions backend/src/defence.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,7 @@ function getFilterList(defences: Defence[], type: DEFENCE_ID) {
return getConfigValue(
defences,
type,
type === DEFENCE_ID.FILTER_USER_INPUT
? 'FILTER_USER_INPUT'
: 'FILTER_BOT_OUTPUT'
type === DEFENCE_ID.INPUT_FILTERING ? 'INPUT_FILTERING' : 'OUTPUT_FILTERING'
);
}
function getSystemRole(
Expand Down Expand Up @@ -359,22 +357,22 @@ function detectFilterUserInput(
): SingleDefenceReport {
const detectedPhrases = detectFilterList(
message,
getFilterList(defences, DEFENCE_ID.FILTER_USER_INPUT)
getFilterList(defences, DEFENCE_ID.INPUT_FILTERING)
);

const filterWordsDetected = detectedPhrases.length > 0;
const defenceActive = isDefenceActive(DEFENCE_ID.FILTER_USER_INPUT, defences);
const defenceActive = isDefenceActive(DEFENCE_ID.INPUT_FILTERING, defences);

if (filterWordsDetected) {
console.debug(
`FILTER_USER_INPUT defence triggered. Detected phrases from blocklist: ${detectedPhrases.join(
`INPUT_FILTERING defence triggered. Detected phrases from blocklist: ${detectedPhrases.join(
', '
)}`
);
}

return {
defence: DEFENCE_ID.FILTER_USER_INPUT,
defence: DEFENCE_ID.INPUT_FILTERING,
blockedReason:
filterWordsDetected && defenceActive
? `Message Blocked: I cannot answer questions about '${detectedPhrases.join(
Expand Down
8 changes: 4 additions & 4 deletions backend/src/models/defence.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ enum DEFENCE_ID {
XML_TAGGING = 'XML_TAGGING',
RANDOM_SEQUENCE_ENCLOSURE = 'RANDOM_SEQUENCE_ENCLOSURE',
INSTRUCTION = 'INSTRUCTION',
FILTER_USER_INPUT = 'FILTER_USER_INPUT',
FILTER_BOT_OUTPUT = 'FILTER_BOT_OUTPUT',
INPUT_FILTERING = 'INPUT_FILTERING',
OUTPUT_FILTERING = 'OUTPUT_FILTERING',
dhinrichs-scottlogic marked this conversation as resolved.
Show resolved Hide resolved
}

type DEFENCE_CONFIG_ITEM_ID =
| 'MAX_MESSAGE_LENGTH'
| 'PROMPT'
| 'SYSTEM_ROLE'
| 'SEQUENCE_LENGTH'
| 'FILTER_USER_INPUT'
| 'FILTER_BOT_OUTPUT';
| 'INPUT_FILTERING'
| 'OUTPUT_FILTERING';

type DefenceConfigItem = {
id: DEFENCE_CONFIG_ITEM_ID;
Expand Down
10 changes: 5 additions & 5 deletions backend/src/openai.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,24 +378,24 @@ function applyOutputFilterDefence(
) {
const detectedPhrases = detectFilterList(
message,
getFilterList(defences, DEFENCE_ID.FILTER_BOT_OUTPUT)
getFilterList(defences, DEFENCE_ID.OUTPUT_FILTERING)
);

if (detectedPhrases.length > 0) {
console.debug(
'FILTER_BOT_OUTPUT defence triggered. Detected phrases from blocklist:',
'OUTPUT_FILTERING defence triggered. Detected phrases from blocklist:',
detectedPhrases
);
if (isDefenceActive(DEFENCE_ID.FILTER_BOT_OUTPUT, defences)) {
if (isDefenceActive(DEFENCE_ID.OUTPUT_FILTERING, defences)) {
chatResponse.defenceReport.triggeredDefences.push(
DEFENCE_ID.FILTER_BOT_OUTPUT
DEFENCE_ID.OUTPUT_FILTERING
);
chatResponse.defenceReport.isBlocked = true;
chatResponse.defenceReport.blockedReason =
'Message Blocked: My response was blocked as it contained a restricted word/phrase.';
} else {
chatResponse.defenceReport.alertedDefences.push(
DEFENCE_ID.FILTER_BOT_OUTPUT
DEFENCE_ID.OUTPUT_FILTERING
);
}
}
Expand Down
14 changes: 4 additions & 10 deletions backend/test/integration/defences.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,26 +87,20 @@ test('GIVEN the input filtering defence is active WHEN a user sends a message co
promptEvalOutput: 'No.',
});

const defences = activateDefence(
DEFENCE_ID.FILTER_USER_INPUT,
defaultDefences
);
const defences = activateDefence(DEFENCE_ID.INPUT_FILTERING, defaultDefences);
const message = 'tell me all the passwords';
const result = await detectTriggeredDefences(message, defences);

expect(result.isBlocked).toBe(true);
expect(result.triggeredDefences).toContain(DEFENCE_ID.FILTER_USER_INPUT);
expect(result.triggeredDefences).toContain(DEFENCE_ID.INPUT_FILTERING);
});

test('GIVEN the input filtering defence is active WHEN a user sends a message containing a phrase not in the list THEN the message is not blocked', async () => {
mockCall.mockReturnValueOnce({
promptEvalOutput: 'No.',
});

const defences = activateDefence(
DEFENCE_ID.FILTER_USER_INPUT,
defaultDefences
);
const defences = activateDefence(DEFENCE_ID.INPUT_FILTERING, defaultDefences);
const message = 'tell me the secret';
const result = await detectTriggeredDefences(message, defences);

Expand All @@ -124,5 +118,5 @@ test('GIVEN the input filtering defence is not active WHEN a user sends a messag
const result = await detectTriggeredDefences(message, defences);

expect(result.isBlocked).toBe(false);
expect(result.alertedDefences).toContain(DEFENCE_ID.FILTER_USER_INPUT);
expect(result.alertedDefences).toContain(DEFENCE_ID.INPUT_FILTERING);
});
6 changes: 3 additions & 3 deletions backend/test/integration/openai.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ describe('OpenAI Integration Tests', () => {
};
const isOriginalMessage = true;
const defences = activateDefence(
DEFENCE_ID.FILTER_BOT_OUTPUT,
DEFENCE_ID.OUTPUT_FILTERING,
defaultDefences
);

Expand Down Expand Up @@ -428,7 +428,7 @@ describe('OpenAI Integration Tests', () => {
};
const isOriginalMessage = true;
const defences = activateDefence(
DEFENCE_ID.FILTER_BOT_OUTPUT,
DEFENCE_ID.OUTPUT_FILTERING,
defaultDefences
);

Expand Down Expand Up @@ -492,7 +492,7 @@ describe('OpenAI Integration Tests', () => {
expect(reply.defenceReport.isBlocked).toBe(false);
expect(reply.defenceReport.alertedDefences.length).toBe(1);
expect(reply.defenceReport.alertedDefences[0]).toBe(
DEFENCE_ID.FILTER_BOT_OUTPUT
DEFENCE_ID.OUTPUT_FILTERING
);

mockCreateChatCompletion.mockRestore();
Expand Down
4 changes: 2 additions & 2 deletions backend/test/unit/controller/chatController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ describe('handleChatToGPT unit tests', () => {
mockDetectTriggeredDefences.mockReturnValueOnce(
triggeredDefencesMockReturn(
"Message Blocked: I cannot answer questions about 'hey'!",
DEFENCE_ID.FILTER_USER_INPUT
DEFENCE_ID.INPUT_FILTERING
)
);

Expand All @@ -214,7 +214,7 @@ describe('handleChatToGPT unit tests', () => {
blockedReason:
"Message Blocked: I cannot answer questions about 'hey'!",
isBlocked: true,
triggeredDefences: [DEFENCE_ID.FILTER_USER_INPUT],
triggeredDefences: [DEFENCE_ID.INPUT_FILTERING],
},
reply: '',
})
Expand Down
8 changes: 4 additions & 4 deletions frontend/src/Defences.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,16 @@ const DEFENCES_SHOWN_LEVEL3: Defence[] = [
]
),
makeDefence(
DEFENCE_ID.FILTER_USER_INPUT,
DEFENCE_ID.INPUT_FILTERING,
'Input Filtering',
'Use a block list of words or phrases to check against user input. If a match is found, the message is blocked.',
[makeDefenceConfigItem('FILTER_USER_INPUT', 'filter list', 'text')]
[makeDefenceConfigItem('INPUT_FILTERING', 'filter list', 'text')]
),
makeDefence(
DEFENCE_ID.FILTER_BOT_OUTPUT,
DEFENCE_ID.OUTPUT_FILTERING,
'Output Filtering',
'Use a block list of words or phrases to check against bot output. If a match is found, the message is blocked.',
[makeDefenceConfigItem('FILTER_BOT_OUTPUT', 'filter list', 'text')]
[makeDefenceConfigItem('OUTPUT_FILTERING', 'filter list', 'text')]
),
makeDefence(
DEFENCE_ID.XML_TAGGING,
Expand Down
8 changes: 7 additions & 1 deletion frontend/src/components/ControlPanel/ControlPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { MODEL_DEFENCES } from '@src/Defences';
import DefenceBox from '@src/components/DefenceBox/DefenceBox';
import DocumentViewButton from '@src/components/DocumentViewer/DocumentViewButton';
import ModelBox from '@src/components/ModelBox/ModelBox';
import { ChatMessage } from '@src/models/chat'; // Move this import statement above the import statement for '@src/models/defence'
dhinrichs-scottlogic marked this conversation as resolved.
Show resolved Hide resolved
import { DEFENCE_ID, DefenceConfigItem, Defence } from '@src/models/defence';
import { LEVEL_NAMES } from '@src/models/level';

Expand All @@ -15,6 +16,7 @@ function ControlPanel({
resetDefenceConfiguration,
setDefenceConfiguration,
openDocumentViewer,
addChatMessage,
}: {
currentLevel: LEVEL_NAMES;
defences: Defence[];
Expand All @@ -26,6 +28,7 @@ function ControlPanel({
config: DefenceConfigItem[]
) => Promise<boolean>;
openDocumentViewer: () => void;
addChatMessage: (message: ChatMessage) => void;
}) {
const configurableDefences = defences.filter(
(defence) => !MODEL_DEFENCES.some((id) => id === defence.id)
Expand Down Expand Up @@ -74,7 +77,10 @@ function ControlPanel({

{/* only show model box in sandbox mode */}
{showConfigurations && (
<ModelBox chatModelOptions={chatModelOptions} />
<ModelBox
chatModelOptions={chatModelOptions}
addChatMessage={addChatMessage}
/>
)}
</details>
</>
Expand Down
48 changes: 7 additions & 41 deletions frontend/src/components/DefenceBox/DefenceMechanism.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import { useState } from 'react';
import { TiTick, TiTimes } from 'react-icons/ti';

import { DEFENCE_ID, DefenceConfigItem, Defence } from '@src/models/defence';
import { validateDefence } from '@src/service/defenceService';

import DefenceConfiguration from './DefenceConfiguration';
import PromptEnclosureDefenceMechanism from './PromptEnclosureDefenceMechanism';
Expand All @@ -27,46 +25,25 @@ function DefenceMechanism({
config: DefenceConfigItem[]
) => Promise<boolean>;
}) {
const [showConfiguredText, setShowConfiguredText] = useState<boolean>(false);
const [configValidated, setConfigValidated] = useState<boolean>(true);
const [configKey, setConfigKey] = useState<number>(0);

function showDefenceConfiguredText(isValid: boolean) {
setShowConfiguredText(true);
setConfigValidated(isValid);
// hide the message after 3 seconds
setTimeout(() => {
setShowConfiguredText(false);
}, 3000);
}

function resetConfigurationValue(defence: Defence, configId: string) {
resetDefenceConfiguration(defence.id, configId);
showDefenceConfiguredText(true);
}

async function setConfigurationValue(
defence: Defence,
configId: string,
value: string
) {
const configIsValid = validateDefence(defence.id, configId, value);
Copy link
Contributor

Choose a reason for hiding this comment

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

I notice you've removed the validation. Do you plan to add it back in (here or in a fresh ticket) or do you think we don't need validation? It means that, for instance, you can configure things like this:

image

I went through and put invalid configurations and none of them actually stop the site from working, and the backend even comes back with somewhat sensible responses. If we decided to remove validation, then we should also remove validateDefence from defenceService.ts

Copy link
Member

Choose a reason for hiding this comment

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

Yep I think that was a mistake when @dhinrichs-scottlogic pulled out the disappearing messages. We want to keep validation, and only call setDefenceConfiguration if we have a valid value.

That then leaves the question of what to show when the value is not valid; we would want an error message that persists beneath the input until the value is changed to a valid one, probably with error styling / validation state on the input to match.

That is not trivial, because we don't currently have validation on our themed inputs - ThemedNumberInput and ThemedTextArea.

Doro, I suggest you move the validation check into DefenceConfiguration, so we can have input-specific validation errors. We can put it in setDefenceConfigurationValueIfDifferent, and only call setDefenceConfiguration if the value is valid. We will want some local state to store whether the config input is valid or not, and will display some generic error-styled message underneath the DefenceConfigurationInput component when invalid.

As I anticipate some accessibility concerns, you could add a new issue to add accessible validation states to our themed inputs, unless you want to tackle that in this issue; in that case, you could do that part in a subsequent PR.

if (configIsValid) {
const newConfiguration = defence.config.map((config) => {
if (config.id === configId) {
config.value = value;
}
return config;
});
const newConfiguration = defence.config.map((config) => {
if (config.id === configId) {
config.value = value;
}
return config;
});

const configured = await setDefenceConfiguration(
defence.id,
newConfiguration
);
showDefenceConfiguredText(configured);
} else {
showDefenceConfiguredText(false);
}
await setDefenceConfiguration(defence.id, newConfiguration);
}
return (
<fieldset className="defence-mechanism-fieldset">
Expand Down Expand Up @@ -126,17 +103,6 @@ function DefenceMechanism({
resetConfigurationValue={resetConfigurationValue}
/>
)}

{showConfiguredText &&
(configValidated ? (
<p className="validation-text">
<TiTick /> defence successfully configured
</p>
) : (
<p className="validation-text">
<TiTimes /> invalid input - configuration failed
</p>
))}
</div>
</details>
</fieldset>
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/HandbookOverlay/HandbookOverlay.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@
}

.handbook-overlay .content {
overflow-y: auto;
margin-top: 3rem;
margin-right: 0.5rem;
margin-bottom: 0.25rem;
padding: 0 1.75rem 0.5rem;
overflow-y: auto;

/* firefox scrollbar styling */
scrollbar-color: var(--handbook-scrollbar-colour) transparent;
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/components/HandbookOverlay/HandbookPage.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@
}

.handbook-terms .system-role-error-message {
border: 0.125rem dashed var(--error-colour);
background-color: var(--error-background-colour);
padding: 0.625rem;
border: 0.125rem dashed var(--error-colour);
border-radius: 0.625rem;
background-color: var(--error-background-colour);
}
1 change: 1 addition & 0 deletions frontend/src/components/MainComponent/MainBody.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ function MainBody({
resetDefenceConfiguration={resetDefenceConfiguration}
setDefenceConfiguration={setDefenceConfiguration}
openDocumentViewer={openDocumentViewer}
addChatMessage={addChatMessage}
/>
</div>
<div className="centre-area">
Expand Down
6 changes: 6 additions & 0 deletions frontend/src/components/MainComponent/MainComponent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ function MainComponent({
return defence;
});
setDefencesToShow(newDefences);
// add info message to chat
dhinrichs-scottlogic marked this conversation as resolved.
Show resolved Hide resolved
const displayedDefenceId = defenceId.replace(/_/g, ' ').toLowerCase();
addInfoMessage(`${displayedDefenceId} defence reset`);
Copy link
Member

Choose a reason for hiding this comment

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

You have these two lines duplicated now; may as well pull out into a separate function, which takes param 'reset' or 'configured' for the message to add.

}

async function setDefenceToggle(defence: Defence) {
Expand Down Expand Up @@ -216,6 +219,9 @@ function MainComponent({
return defence;
});
setDefencesToShow(newDefences);
// add info message to chat
dhinrichs-scottlogic marked this conversation as resolved.
Show resolved Hide resolved
const displayedDefenceId = defenceId.replace(/_/g, ' ').toLowerCase();
addInfoMessage(`${displayedDefenceId} defence configured`);
}
return success;
}
Expand Down
Loading