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

Stop sending full defences on low levels #844

Merged
merged 43 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
7bedff7
defences undefined for levels 1 and 2
pmarsh-scottlogic Feb 23, 2024
7e86ae4
fixes tests
pmarsh-scottlogic Feb 23, 2024
b0a4823
propoerly undefines defences on level 1 and 2
pmarsh-scottlogic Feb 23, 2024
4e45d90
moves variable declarations to destructureing
pmarsh-scottlogic Feb 29, 2024
d92314c
missed one
pmarsh-scottlogic Feb 29, 2024
74d8fb4
undo error swallow
pmarsh-scottlogic Feb 29, 2024
1237cca
merge dev
pmarsh-scottlogic Feb 29, 2024
dc66ffb
remove bad type guards
pmarsh-scottlogic Feb 29, 2024
c442d04
improve validation for configureDefence
pmarsh-scottlogic Feb 29, 2024
aa8f672
last bit of validation
pmarsh-scottlogic Feb 29, 2024
aa0c576
update tests for handleConfigureDefence
pmarsh-scottlogic Feb 29, 2024
d6b6455
new folder for defence controller unit tests and wrote first handleD…
pmarsh-scottlogic Feb 29, 2024
294de45
adds test for defence activation
pmarsh-scottlogic Feb 29, 2024
4785161
adds tests for defence deactivation
pmarsh-scottlogic Feb 29, 2024
66cdd2e
moves tests all back into one file again
pmarsh-scottlogic Feb 29, 2024
e74a26f
adds tests for handleResetSingleDefence
pmarsh-scottlogic Feb 29, 2024
663b22e
improve validation on handleGetDefenceStatus
pmarsh-scottlogic Feb 29, 2024
8429bfd
removes extraneous stuff from test objects
pmarsh-scottlogic Feb 29, 2024
ebba951
adds tests to make sure defences aren't chngeable for level 1
pmarsh-scottlogic Feb 29, 2024
107e453
add loop to test levels 1 and 2 with same code
pmarsh-scottlogic Feb 29, 2024
373e0b7
adds remaining tests for defence controller integration
pmarsh-scottlogic Feb 29, 2024
8293a4a
handleResetSingleDefence takes level parameter now
pmarsh-scottlogic Feb 29, 2024
ae5e746
adds level to frontend call to reset defence config item. Types all t…
pmarsh-scottlogic Feb 29, 2024
a336617
some renamings and fill in gaps in tests
pmarsh-scottlogic Feb 29, 2024
23771d9
adds another missing test
pmarsh-scottlogic Feb 29, 2024
2aab5f7
tweaks
pmarsh-scottlogic Feb 29, 2024
a04ca87
merge dev
pmarsh-scottlogic Mar 5, 2024
24b7c04
fix test suite
pmarsh-scottlogic Mar 5, 2024
2fc5a6f
replace defence search with defences.some and replace cuurrentDefence…
pmarsh-scottlogic Mar 5, 2024
e907b04
change defenceId for generic falsy check
pmarsh-scottlogic Mar 5, 2024
f56383f
improve guards for level and config
pmarsh-scottlogic Mar 5, 2024
65b5644
improve guards for level and config
pmarsh-scottlogic Mar 5, 2024
b45cce9
correct test for configuring a config item
pmarsh-scottlogic Mar 5, 2024
d670f00
updates error message when not allowed to activate or modify defences
pmarsh-scottlogic Mar 5, 2024
095ad15
rescturecure defence controller integration tests
pmarsh-scottlogic Mar 5, 2024
0d02968
change other tests to use test.each rather than foreach
pmarsh-scottlogic Mar 5, 2024
9f9d628
uses isValidLevel
pmarsh-scottlogic Mar 5, 2024
dec9904
improve validateFilterConfig
pmarsh-scottlogic Mar 5, 2024
e98c251
just pass qa defence rather than all defences
pmarsh-scottlogic Mar 7, 2024
b0e17d0
fix tests
pmarsh-scottlogic Mar 7, 2024
360c30c
uses type intersection to define QaLlmDefence
pmarsh-scottlogic Mar 12, 2024
600dc65
Merge branch 'dev' into stop-sending-full-defences-on-low-leevels
pmarsh-scottlogic Mar 12, 2024
8ec48c7
only gedDefencesFrDTOs if it is defined
pmarsh-scottlogic Mar 12, 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
25 changes: 14 additions & 11 deletions backend/src/controller/chatController.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Response } from 'express';

import { defaultDefences } from '@src/defaultDefences';
import {
transformMessage,
detectTriggeredInputDefences,
Expand All @@ -21,7 +22,7 @@ import {
ChatInfoMessage,
chatInfoMessageTypes,
} from '@src/models/chatMessage';
import { Defence } from '@src/models/defence';
import { DEFENCE_ID, Defence, QaLlmDefence } from '@src/models/defence';
import { EmailInfo } from '@src/models/email';
import { LEVEL_NAMES } from '@src/models/level';
import { chatGptSendMessage } from '@src/openai';
Expand Down Expand Up @@ -94,8 +95,7 @@ async function handleChatWithoutDefenceDetection(
chatResponse: ChatHttpResponse,
currentLevel: LEVEL_NAMES,
chatModel: ChatModel,
chatHistory: ChatMessage[],
defences: Defence[]
chatHistory: ChatMessage[]
): Promise<LevelHandlerResponse> {
console.log(`User message: '${message}'`);

Expand All @@ -106,7 +106,6 @@ async function handleChatWithoutDefenceDetection(

const openAiReply = await chatGptSendMessage(
updatedChatHistory,
defences,
chatModel,
currentLevel
);
Expand Down Expand Up @@ -149,11 +148,15 @@ async function handleChatWithDefenceDetection(
}'`
);

const qaLlmDefence = defences.find(
(defence) => defence.id === DEFENCE_ID.QA_LLM
) as QaLlmDefence | undefined;

const openAiReplyPromise = chatGptSendMessage(
chatHistoryWithNewUserMessages,
defences,
chatModel,
currentLevel
currentLevel,
qaLlmDefence
);

// run input defence detection and chatGPT concurrently
Expand Down Expand Up @@ -240,14 +243,15 @@ async function handleChatToGPT(req: OpenAiChatRequest, res: Response) {
? req.session.chatModel
: defaultChatModel;

const defences =
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering why we want to define defences here (other than the system role) for levels 1 and 2, which leads me to wonder why handleChatWithoutDefenceDetection needs to receive the defences at all...

If you follow the passing of defences through handleChatWithoutDefenceDetection and beyond, you end up at handleAskQuestionFunction in src/openai.ts, which simply checks if the Q&A LLM defence is active. If we don't have any defences, then surely we can assume it's not active. Plus, it feels overly complex to be passing all the defences through so many calls, when all we need is the QA_LLM defence.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what do you suggest? pass undefined through the calls instead? Make new methods for the whole call chain for getting a response without defences? I vote the former

Copy link
Member

Choose a reason for hiding this comment

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

I suggest we don't pass defences into either handleChat function. The entire function call chain is self-contained, i.e. all functions are only invoked within this single call chain, so why not simply look up the defences from local storage in handleAskQuestionFunction? The currentLevel is passed all the way down so it's a trivial lookup. No need to pass defences down at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The answer is: to get something from the session, we need access to the req object

Copy link
Member

Choose a reason for hiding this comment

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

Of course, silly me! As discussed, let's try passing just the Q&A LLM defence down the call chain, but also have it optional so we don't need it for levels 1 and 2.

req.session.levelState[currentLevel].defences ?? defaultDefences;

const currentChatHistory = setSystemRoleInChatHistory(
currentLevel,
req.session.levelState[currentLevel].defences,
defences,
req.session.levelState[currentLevel].chatHistory
);

const defences = [...req.session.levelState[currentLevel].defences];

let levelResult: LevelHandlerResponse;
try {
if (currentLevel < LEVEL_NAMES.LEVEL_3) {
Expand All @@ -256,8 +260,7 @@ async function handleChatToGPT(req: OpenAiChatRequest, res: Response) {
initChatResponse,
currentLevel,
chatModel,
currentChatHistory,
defences
currentChatHistory
);
} else {
levelResult = await handleChatWithDefenceDetection(
Expand Down
231 changes: 171 additions & 60 deletions backend/src/controller/defenceController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import {
resetDefenceConfig,
} from '@src/defence';
import { DefenceActivateRequest } from '@src/models/api/DefenceActivateRequest';
import { DefenceConfigResetRequest } from '@src/models/api/DefenceConfigResetRequest';
import { DefenceConfigItemResetRequest } from '@src/models/api/DefenceConfigResetRequest';
import { DefenceConfigureRequest } from '@src/models/api/DefenceConfigureRequest';
import { DefenceConfigItem } from '@src/models/defence';
import { LEVEL_NAMES } from '@src/models/level';
import { LEVEL_NAMES, isValidLevel } from '@src/models/level';

import { sendErrorResponse } from './handleError';

Expand All @@ -24,46 +24,99 @@ function configValueExceedsCharacterLimit(config: DefenceConfigItem[]) {
}

function handleDefenceActivation(req: DefenceActivateRequest, res: Response) {
const defenceId = req.body.defenceId;
const level = req.body.level;

if (defenceId && level !== undefined) {
// activate the defence
req.session.levelState[level].defences = activateDefence(
defenceId,
req.session.levelState[level].defences
);
res.status(200).send();
} else {
res.status(400).send();
const { defenceId, level } = req.body;

if (!defenceId) {
sendErrorResponse(res, 400, 'Missing defenceId');
return;
}

if (!Number.isFinite(level) || level === undefined) {
sendErrorResponse(res, 400, 'Missing level');
return;
}

if (!isValidLevel(level)) {
sendErrorResponse(res, 400, 'Invalid level');
return;
}
pmarsh-scottlogic marked this conversation as resolved.
Show resolved Hide resolved

const currentDefences = req.session.levelState[level].defences;

if (!currentDefences) {
sendErrorResponse(res, 400, 'You cannot activate defences on this level');
return;
}

if (!currentDefences.some((defence) => defence.id === defenceId)) {
sendErrorResponse(res, 400, `Defence with id ${defenceId} not found`);
return;
}
pmarsh-scottlogic marked this conversation as resolved.
Show resolved Hide resolved

req.session.levelState[level].defences = activateDefence(
defenceId,
currentDefences
);
res.status(200).send();
}

function handleDefenceDeactivation(req: DefenceActivateRequest, res: Response) {
// id of the defence
const defenceId = req.body.defenceId;
const level = req.body.level;
if (defenceId && level !== undefined) {
// deactivate the defence
req.session.levelState[level].defences = deactivateDefence(
defenceId,
req.session.levelState[level].defences
);
res.send();
} else {
res.status(400);
res.send();
const { defenceId, level } = req.body;

if (!defenceId) {
sendErrorResponse(res, 400, 'Missing defenceId');
return;
}

if (!Number.isFinite(level) || level === undefined) {
sendErrorResponse(res, 400, 'Missing level');
return;
}

if (!isValidLevel(level)) {
sendErrorResponse(res, 400, 'Invalid level');
return;
}

const currentDefences = req.session.levelState[level].defences;

if (!currentDefences) {
sendErrorResponse(res, 400, 'You cannot deactivate defences on this level');
return;
}

if (!currentDefences.some((defence) => defence.id === defenceId)) {
sendErrorResponse(res, 400, `Defence with id ${defenceId} not found`);
return;
}

req.session.levelState[level].defences = deactivateDefence(
defenceId,
currentDefences
);
res.status(200).send();
}

function handleConfigureDefence(req: DefenceConfigureRequest, res: Response) {
// id of the defence
const defenceId = req.body.defenceId;
const config = req.body.config;
const level = req.body.level;
const { defenceId, level, config } = req.body;

if (!defenceId) {
sendErrorResponse(res, 400, 'Missing defenceId');
return;
}

if (!Number.isFinite(level) || level === undefined) {
sendErrorResponse(res, 400, 'Missing level');
return;
}

if (!defenceId || !config || level === undefined) {
sendErrorResponse(res, 400, 'Missing defenceId, config or level');
if (!isValidLevel(level)) {
sendErrorResponse(res, 400, 'Invalid level');
return;
}

if (!config) {
sendErrorResponse(res, 400, 'Missing config');
return;
}

Expand All @@ -72,48 +125,106 @@ function handleConfigureDefence(req: DefenceConfigureRequest, res: Response) {
return;
}

// configure the defence
const currentDefences = req.session.levelState[level].defences;

if (!currentDefences || level !== LEVEL_NAMES.SANDBOX) {
sendErrorResponse(res, 400, 'You cannot configure defences on this level');
return;
}

const defence = currentDefences.find((defence) => defence.id === defenceId);

if (defence === undefined) {
sendErrorResponse(res, 400, `Defence with id ${defenceId} not found`);
return;
}

req.session.levelState[level].defences = configureDefence(
defenceId,
req.session.levelState[level].defences,
currentDefences,
config
);
res.send();
}

function handleResetSingleDefence(
req: DefenceConfigResetRequest,
function handleResetDefenceConfigItem(
req: DefenceConfigItemResetRequest,
res: Response
) {
const defenceId = req.body.defenceId;
const configId = req.body.configId;
const level = LEVEL_NAMES.SANDBOX; //configuration only available in sandbox
if (defenceId && configId) {
req.session.levelState[level].defences = resetDefenceConfig(
defenceId,
configId,
req.session.levelState[level].defences
const { defenceId, configItemId, level } = req.body;

if (!defenceId) {
sendErrorResponse(res, 400, 'Missing defenceId');
return;
}

if (!configItemId) {
sendErrorResponse(res, 400, 'Missing configId');
return;
}

if (!Number.isFinite(level) || level === undefined) {
sendErrorResponse(res, 400, 'Missing level');
return;
}

if (!isValidLevel(level)) {
sendErrorResponse(res, 400, 'Invalid level');
return;
}

const currentDefences = req.session.levelState[level].defences;

if (!currentDefences || level !== LEVEL_NAMES.SANDBOX) {
sendErrorResponse(
res,
400,
'You cannot reset defence config items on this level'
);
const updatedDefenceConfig: DefenceConfigItem | undefined =
req.session.levelState[level].defences
.find((defence) => defence.id === defenceId)
?.config.find((config) => config.id === configId);

if (updatedDefenceConfig) {
res.send(updatedDefenceConfig);
} else {
res.status(400);
res.send();
}
} else {
res.status(400);
res.send();
return;
}

const defence = currentDefences.find((defence) => defence.id === defenceId);

if (defence === undefined) {
sendErrorResponse(res, 400, `Defence with id ${defenceId} not found`);
return;
}

const configItem = defence.config.find(
(configItem) => configItem.id === configItemId
);

if (configItem === undefined) {
sendErrorResponse(
res,
400,
`Config item with id ${configItemId} not found for defence with id ${defenceId}`
);
return;
}

req.session.levelState[level].defences = resetDefenceConfig(
defenceId,
configItemId,
currentDefences
);

const updatedDefences = req.session.levelState[level].defences;
if (updatedDefences === undefined) {
sendErrorResponse(res, 500, 'Something went whacky');
return;
}
const updatedDefenceConfig = updatedDefences
.find((defence) => defence.id === defenceId)
?.config.find((config) => config.id === configItemId);

res.send(updatedDefenceConfig);
}

export {
handleDefenceActivation,
handleDefenceDeactivation,
handleConfigureDefence,
handleResetSingleDefence,
handleResetDefenceConfigItem,
};
6 changes: 4 additions & 2 deletions backend/src/models/api/DefenceConfigResetRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ import {
DEFENCE_ID,
DefenceConfigItem,
} from '@src/models/defence';
import { LEVEL_NAMES } from '@src/models/level';

export type DefenceConfigResetRequest = Request<
export type DefenceConfigItemResetRequest = Request<
never,
DefenceConfigItem,
{
defenceId?: DEFENCE_ID;
configId?: DEFENCE_CONFIG_ITEM_ID;
configItemId?: DEFENCE_CONFIG_ITEM_ID;
level?: LEVEL_NAMES;
},
never
>;
11 changes: 10 additions & 1 deletion backend/src/models/defence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,20 @@ type DefenceConfigItem = {
value: string;
};

type QaLlmDefence = Defence & {
id: DEFENCE_ID.QA_LLM;
};

type Defence = {
id: DEFENCE_ID;
config: DefenceConfigItem[];
isActive: boolean;
};
pmarsh-scottlogic marked this conversation as resolved.
Show resolved Hide resolved

export { DEFENCE_ID };
export type { Defence, DefenceConfigItem, DEFENCE_CONFIG_ITEM_ID };
export type {
Defence,
DefenceConfigItem,
DEFENCE_CONFIG_ITEM_ID,
QaLlmDefence,
};
Loading
Loading